From 07b497a7ed2fc26b3000a916c92655f6e035d5df Mon Sep 17 00:00:00 2001 From: Dave Watson Date: Fri, 5 Aug 2016 12:15:19 -0700 Subject: [PATCH] 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 --- folly/io/async/HHWheelTimer.cpp | 10 +--------- folly/io/async/HHWheelTimer.h | 18 ------------------ folly/io/async/test/HHWheelTimerTest.cpp | 8 ++------ 3 files changed, 3 insertions(+), 33 deletions(-) diff --git a/folly/io/async/HHWheelTimer.cpp b/folly/io/async/HHWheelTimer.cpp index 9918f9dd..31213ba5 100644 --- 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 catchup = std::chrono::duration_cast( 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 4d255b64..33300828 100644 --- 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 e4f74211..4899842e 100644 --- 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)); -- 2.34.1