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

diff --git a/folly/io/async/DelayedDestruction.h b/folly/io/async/DelayedDestruction.h
index 71cd69156f1d5689d8fd4b2739a0136d397c9dc6..5e179d530f50d191e065fdd620976b4b2f0183f2 100644 (file)
--- a/folly/io/async/DelayedDestruction.h
+++ b/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

diff --git a/folly/io/async/DelayedDestructionBase.h b/folly/io/async/DelayedDestructionBase.h
index bd68c1cb89daaccfd6c164ae76ae94633ff0384c..52505a064c892c69adcc4fb63524f0f73ace560e 100644 (file)
--- a/folly/io/async/DelayedDestructionBase.h
+++ b/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:
   /**

diff --git a/folly/io/async/test/DelayedDestructionBaseTest.cpp b/folly/io/async/test/DelayedDestructionBaseTest.cpp
index c9497de921ead062394539de095ff18669c68bae..1d7ab3fc20129d14108df8192fd50da225f386b2 100644 (file)
--- a/folly/io/async/test/DelayedDestructionBaseTest.cpp
+++ b/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);
-}

diff --git a/folly/io/async/test/UndelayedDestruction.h b/folly/io/async/test/UndelayedDestruction.h
index a6594e0ee75065e4ff514c97b8df2c16b4bed034..47a3dba8eb05b77ef4addeb91444ec0aa5425b66 100644 (file)
--- a/folly/io/async/test/UndelayedDestruction.h
+++ b/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.