Faster onDestroy

Summary: there is no need to call `std::function::invoke` for every DestructorGuard.

Reviewed By: yfeldblum

Differential Revision: D3229345

fb-gh-sync-id: c42f8cd05576d56b6a9b2f9d06878d9b01a36e94
fbshipit-source-id: c42f8cd05576d56b6a9b2f9d06878d9b01a36e94

folly/io/async/DelayedDestruction.h

@@ -54,7 +54,7 @@ class DelayedDestruction : public DelayedDestructionBase {
     if (getDestructorGuardCount() != 0) {
       destroyPending_ = true;
     } else {
-      onDestroy_(false);
+      onDelayedDestroy(false);
     }
   }
 
@@ -98,15 +98,6 @@ class DelayedDestruction : public DelayedDestructionBase {
 
   DelayedDestruction()
     : destroyPending_(false) {
-
-    onDestroy_ = [this] (bool delayed) {
-      // check if it is ok to destroy now
-      if (delayed && !destroyPending_) {
-        return;
-      }
-      destroyPending_ = false;
-      delete this;
-    };
   }
 
  private:
@@ -118,5 +109,14 @@ class DelayedDestruction : public DelayedDestructionBase {
    * guardCount_ drops to 0.
    */
   bool destroyPending_;
+
+  void onDelayedDestroy(bool delayed) override {
+    // check if it is ok to destroy now
+    if (delayed && !destroyPending_) {
+      return;
+    }
+    destroyPending_ = false;
+    delete this;
+  }
 };
 } // folly

folly/io/async/DelayedDestructionBase.h

@@ -37,7 +37,7 @@ namespace folly {
  *
  * Classes needing this functionality should:
  * - derive from DelayedDestructionBase directly
- * - pass a callback to onDestroy_ which'll be called before the object is
+ * - implement onDelayedDestroy which'll be called before the object is
  *   going to be destructed
  * - create a DestructorGuard object on the stack in each public method that
  *   may invoke a callback
@@ -93,7 +93,7 @@ class DelayedDestructionBase : private boost::noncopyable {
         assert(dd_->guardCount_ > 0);
         --dd_->guardCount_;
         if (dd_->guardCount_ == 0) {
-          dd_->onDestroy_(true);
+          dd_->onDelayedDestroy(true);
         }
       }
     }
@@ -213,14 +213,15 @@ class DelayedDestructionBase : private boost::noncopyable {
   }
 
   /**
-   * Implement onDestroy_ in subclasses.
-   * onDestroy_() is invoked when the object is potentially being destroyed.
+   * Implement onDelayedDestroy in subclasses.
+   * onDelayedDestroy() is invoked when the object is potentially being
+   * destroyed.
    *
    * @param delayed  This parameter is true if destruction was delayed because
-   *                 of a DestructorGuard object, or false if onDestroy_() is
-   *                 being called directly from the destructor.
+   *                 of a DestructorGuard object, or false if onDelayedDestroy()
+   *                 is being called directly from the destructor.
    */
-  std::function<void(bool)> onDestroy_;
+  virtual void onDelayedDestroy(bool delayed) = 0;
 
  private:
   /**

folly/io/async/test/DelayedDestructionBaseTest.cpp

@@ -43,11 +43,6 @@ using namespace folly;
 class DestructionOnCallback : public DelayedDestructionBase {
  public:
   DestructionOnCallback() : state_(0), deleted_(false) {
-    onDestroy_ = [this] (bool delayed) {
-      deleted_ = true;
-      delete this;
-      (void)delayed; // prevent unused variable warnings
-    };
   }
 
   void onComplete(int n, int& state) {
@@ -58,10 +53,6 @@ class DestructionOnCallback : public DelayedDestructionBase {
     state = state_;
   }
 
-  void setOnDestroy(std::function<void(bool)> onDestroy) {
-    onDestroy_ = onDestroy;
-  }
-
   int state() const { return state_; }
   bool deleted() const { return deleted_; }
 
@@ -77,6 +68,12 @@ class DestructionOnCallback : public DelayedDestructionBase {
  private:
   int state_;
   bool deleted_;
+
+  void onDelayedDestroy(bool delayed) override {
+    deleted_ = true;
+    delete this;
+    (void)delayed; // prevent unused variable warnings
+  }
 };
 
 struct DelayedDestructionBaseTest : public ::testing::Test {
@@ -89,17 +86,3 @@ TEST_F(DelayedDestructionBaseTest, basic) {
   d->onComplete(3, state);
   EXPECT_EQ(state, 10); // 10 = 6 + 3 + 1
 }
-
-TEST_F(DelayedDestructionBaseTest, destructFromContainer) {
-  std::list<DestructionOnCallback> l;
-  l.emplace_back();
-  l.back().setOnDestroy([&] (bool delayed) {
-    l.erase(l.begin());
-    (void)delayed;
-  });
-  EXPECT_NE(l.size(), 0);
-  int32_t state;
-  l.back().onComplete(3, state);
-  EXPECT_EQ(state, 10); // 10 = 6 + 3 + 1
-  EXPECT_EQ(l.size(), 0);
-}

folly/io/async/test/UndelayedDestruction.h

@@ -58,16 +58,6 @@ class UndelayedDestruction : public TDD {
   template<typename ...Args>
   explicit UndelayedDestruction(Args&& ...args)
     : TDD(std::forward<Args>(args)...) {
-      this->TDD::onDestroy_ = [&, this] (bool delayed) {
-        if (delayed && !this->TDD::getDestroyPending()) {
-          return;
-        }
-        // Do nothing.  This will always be invoked from the call to destroy
-        // inside our destructor.
-        assert(!delayed);
-        // prevent unused variable warnings when asserts are compiled out.
-        (void)delayed;
-      };
   }
 
   /**
@@ -91,6 +81,17 @@ class UndelayedDestruction : public TDD {
     this->destroy();
   }
 
+  void onDelayedDestroy(bool delayed) override {
+    if (delayed && !this->TDD::getDestroyPending()) {
+      return;
+    }
+    // Do nothing.  This will always be invoked from the call to destroy
+    // inside our destructor.
+    assert(!delayed);
+    // prevent unused variable warnings when asserts are compiled out.
+    (void)delayed;
+  }
+
  protected:
   /**
    * Override our parent's destroy() method to make it protected.