remove catchupEveryN

Summary: This feature doesn't make sense when wheeltimer doesn't constantly tick - we always have to get the current clock time.  On the plus side, we'll only be grabbing the clock on timer schedule or timeout, never for individual ticks.

Reviewed By: yfeldblum

Differential Revision: D3637088

fbshipit-source-id: ed8fe52419259332a14b6dc1d357979dcf258a20

diff --git a/folly/io/async/HHWheelTimer.cpp b/folly/io/async/HHWheelTimer.cpp
index 9918f9dd9e43a0d43dace3853788415560182326..31213ba5206daaa5b01acda2047b1035204faa80 100644 (file)
--- a/folly/io/async/HHWheelTimer.cpp
+++ b/folly/io/async/HHWheelTimer.cpp
@@ -87,8 +87,6 @@ HHWheelTimer::HHWheelTimer(
       defaultTimeout_(defaultTimeoutMS),
       nextTick_(1),
       count_(0),
-      catchupEveryN_(DEFAULT_CATCHUP_EVERY_N),
-      expirationsSinceCatchup_(0),
       processingCallbacksGuard_(nullptr) {}
 
 HHWheelTimer::~HHWheelTimer() {
@@ -181,14 +179,8 @@ void HHWheelTimer::timeoutExpired() noexcept {
   // timeoutExpired() can only be invoked directly from the event base loop.
   // It should never be invoked recursively.
   //
-  milliseconds catchup = now_ + interval_;
-  // If catchup is enabled, we may have missed multiple intervals, use
-  // currentTime() to check exactly.
-  if (++expirationsSinceCatchup_ >= catchupEveryN_) {
-    catchup = std::chrono::duration_cast<milliseconds>(
+  milliseconds catchup = std::chrono::duration_cast<milliseconds>(
       std::chrono::steady_clock::now().time_since_epoch());
-    expirationsSinceCatchup_ = 0;
-  }
   while (now_ < catchup) {
     now_ += interval_;
 

diff --git a/folly/io/async/HHWheelTimer.h b/folly/io/async/HHWheelTimer.h
index 4d255b6472b68b214722c7a7a2504d0fc2bbcf5f..3330082817570f64c0fa4413864ecb0a8ec58f87 100644 (file)
--- a/folly/io/async/HHWheelTimer.h
+++ b/folly/io/async/HHWheelTimer.h
@@ -250,20 +250,6 @@ class HHWheelTimer : private folly::AsyncTimeout,
     return count_;
   }
 
-  /**
-   * This turns on more exact timing.  By default the wheel timer
-   * increments its cached time only once everyN (default) ticks.
-   *
-   * With catchupEveryN at 1, timeouts will only be delayed until the
-   * next tick, at which point all overdue timeouts are called.  The
-   * wheel timer is approximately 2x slower with this set to 1.
-   *
-   * Load testing in opt mode showed skew was about 1% with no catchup.
-   */
-  void setCatchupEveryN(uint32_t everyN) {
-    catchupEveryN_ = everyN;
-  }
-
   bool isDetachable() const {
     return !folly::AsyncTimeout::isScheduled();
   }
@@ -310,10 +296,6 @@ class HHWheelTimer : private folly::AsyncTimeout,
   uint64_t count_;
   std::chrono::milliseconds now_;
 
-  static constexpr uint32_t DEFAULT_CATCHUP_EVERY_N = 10;
-
-  uint32_t catchupEveryN_;
-  uint32_t expirationsSinceCatchup_;
   bool* processingCallbacksGuard_;
 };
 

diff --git a/folly/io/async/test/HHWheelTimerTest.cpp b/folly/io/async/test/HHWheelTimerTest.cpp
index e4f74211a9ba0d34e4999053657f9dafa2f1cb3d..4899842e5669f1e90ec71c0f25498c19025ad804 100644 (file)
--- a/folly/io/async/test/HHWheelTimerTest.cpp
+++ b/folly/io/async/test/HHWheelTimerTest.cpp
@@ -262,7 +262,6 @@ TEST_F(HHWheelTimerTest, AtMostEveryN) {
   milliseconds interval(10);
   milliseconds atMostEveryN(3);
   StackWheelTimer t(&eventBase, atMostEveryN);
-  t.setCatchupEveryN(70);
 
   // Create 60 timeouts to be added to ts1 at 1ms intervals.
   uint32_t numTimeouts = 60;
@@ -367,11 +366,8 @@ TEST_F(HHWheelTimerTest, SlowLoop) {
   ASSERT_EQ(t.count(), 0);
 
   // Check that the timeout was delayed by sleep
-  T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(15), milliseconds(1));
-  T_CHECK_TIMEOUT(start, end, milliseconds(15), milliseconds(1));
-
-  // Try it again, this time with catchup timing every loop
-  t.setCatchupEveryN(1);
+  T_CHECK_TIMEOUT(start, t1.timestamps[0], milliseconds(10), milliseconds(1));
+  T_CHECK_TIMEOUT(start, end, milliseconds(10), milliseconds(1));
 
   eventBase.runInLoop([](){usleep(10000);});
   t.scheduleTimeout(&t2, milliseconds(5));