Commit 8b47e569 authored by Stepan Palamarchuk's avatar Stepan Palamarchuk Committed by Facebook Github Bot

Avoid unnecessary looping over bitmap when scheduling new timeouts

Summary:
Current implementation will loop over bitmap to figure out next tick to schedule. This consumes visible amount of CPU (10% in fibers benchmark, ~4% of Thrift noop load test).

However, this looping is unnecessary, because we already have all information available - what is the earliest pre-existing timeout (expireTick_) and the new one. So we can decide what tick to schedule for by just simple conditional statement.

Reviewed By: vitaut

Differential Revision: D13709523

fbshipit-source-id: b0e3e6301cc2e759b4e8901ba5ff009587516cf5
parent 74ea5388
...@@ -102,11 +102,10 @@ HHWheelTimer::~HHWheelTimer() { ...@@ -102,11 +102,10 @@ HHWheelTimer::~HHWheelTimer() {
void HHWheelTimer::scheduleTimeoutImpl( void HHWheelTimer::scheduleTimeoutImpl(
Callback* callback, Callback* callback,
std::chrono::milliseconds timeout, int64_t dueTick,
int64_t nextTickToProcess, int64_t nextTickToProcess,
int64_t nextTick) { int64_t nextTick) {
int64_t due = timeToWheelTicks(timeout) + nextTick; int64_t diff = dueTick - nextTickToProcess;
int64_t diff = due - nextTickToProcess;
CallbackList* list; CallbackList* list;
auto bi = makeBitIterator(bitmap_.begin()); auto bi = makeBitIterator(bitmap_.begin());
...@@ -116,20 +115,20 @@ void HHWheelTimer::scheduleTimeoutImpl( ...@@ -116,20 +115,20 @@ void HHWheelTimer::scheduleTimeoutImpl(
*(bi + (nextTick & WHEEL_MASK)) = true; *(bi + (nextTick & WHEEL_MASK)) = true;
callback->bucket_ = nextTick & WHEEL_MASK; callback->bucket_ = nextTick & WHEEL_MASK;
} else if (diff < WHEEL_SIZE) { } else if (diff < WHEEL_SIZE) {
list = &buckets_[0][due & WHEEL_MASK]; list = &buckets_[0][dueTick & WHEEL_MASK];
*(bi + (due & WHEEL_MASK)) = true; *(bi + (dueTick & WHEEL_MASK)) = true;
callback->bucket_ = due & WHEEL_MASK; callback->bucket_ = dueTick & WHEEL_MASK;
} else if (diff < 1 << (2 * WHEEL_BITS)) { } else if (diff < 1 << (2 * WHEEL_BITS)) {
list = &buckets_[1][(due >> WHEEL_BITS) & WHEEL_MASK]; list = &buckets_[1][(dueTick >> WHEEL_BITS) & WHEEL_MASK];
} else if (diff < 1 << (3 * WHEEL_BITS)) { } else if (diff < 1 << (3 * WHEEL_BITS)) {
list = &buckets_[2][(due >> 2 * WHEEL_BITS) & WHEEL_MASK]; list = &buckets_[2][(dueTick >> 2 * WHEEL_BITS) & WHEEL_MASK];
} else { } else {
/* in largest slot */ /* in largest slot */
if (diff > LARGEST_SLOT) { if (diff > LARGEST_SLOT) {
diff = LARGEST_SLOT; diff = LARGEST_SLOT;
due = diff + nextTickToProcess; dueTick = diff + nextTickToProcess;
} }
list = &buckets_[3][(due >> 3 * WHEEL_BITS) & WHEEL_MASK]; list = &buckets_[3][(dueTick >> 3 * WHEEL_BITS) & WHEEL_MASK];
} }
list->push_back(*callback); list->push_back(*callback);
} }
...@@ -158,13 +157,24 @@ void HHWheelTimer::scheduleTimeout( ...@@ -158,13 +157,24 @@ void HHWheelTimer::scheduleTimeout(
if (processingCallbacksGuard_ || isScheduled()) { if (processingCallbacksGuard_ || isScheduled()) {
baseTick = std::min(expireTick_, nextTick); baseTick = std::min(expireTick_, nextTick);
} }
scheduleTimeoutImpl(callback, timeout, baseTick, nextTick); int64_t ticks = timeToWheelTicks(timeout);
int64_t due = ticks + nextTick;
scheduleTimeoutImpl(callback, due, baseTick, nextTick);
/* If we're calling callbacks, timer will be reset after all /* If we're calling callbacks, timer will be reset after all
* callbacks are called. * callbacks are called.
*/ */
if (!processingCallbacksGuard_) { if (!processingCallbacksGuard_) {
scheduleNextTimeout(nextTick); // Check if we need to reschedule the timer.
// If the wheel timeout is already scheduled, then we need to reschedule
// only if our due is earlier than the current scheduled one.
// If it's not scheduled, we need to schedule it either for the first tick
// of next wheel epoch or our due tick, whichever is earlier.
if (!isScheduled() && !inSameEpoch(nextTick - 1, due)) {
scheduleNextTimeout(nextTick, WHEEL_SIZE - ((nextTick - 1) & WHEEL_MASK));
} else if (!isScheduled() || due < expireTick_) {
scheduleNextTimeout(nextTick, ticks + 1);
}
} }
} }
...@@ -182,7 +192,11 @@ bool HHWheelTimer::cascadeTimers(int bucket, int tick) { ...@@ -182,7 +192,11 @@ bool HHWheelTimer::cascadeTimers(int bucket, int tick) {
while (!cbs.empty()) { while (!cbs.empty()) {
auto* cb = &cbs.front(); auto* cb = &cbs.front();
cbs.pop_front(); cbs.pop_front();
scheduleTimeoutImpl(cb, cb->getTimeRemaining(now), expireTick_, nextTick); scheduleTimeoutImpl(
cb,
nextTick + timeToWheelTicks(cb->getTimeRemaining(now)),
expireTick_,
nextTick);
} }
// If tick is zero, timeoutExpired will cascade the next bucket. // If tick is zero, timeoutExpired will cascade the next bucket.
...@@ -248,7 +262,11 @@ void HHWheelTimer::timeoutExpired() noexcept { ...@@ -248,7 +262,11 @@ void HHWheelTimer::timeoutExpired() noexcept {
return; return;
} }
} }
scheduleNextTimeout(expireTick_);
// We don't need to schedule a new timeout if there're nothing in the wheel.
if (count_ > 0) {
scheduleNextTimeout(expireTick_);
}
} }
size_t HHWheelTimer::cancelAll() { size_t HHWheelTimer::cancelAll() {
...@@ -299,15 +317,12 @@ void HHWheelTimer::scheduleNextTimeout(int64_t nextTick) { ...@@ -299,15 +317,12 @@ void HHWheelTimer::scheduleNextTimeout(int64_t nextTick) {
} }
} }
if (count_ > 0) { scheduleNextTimeout(nextTick, tick);
if (!this->AsyncTimeout::isScheduled() || }
(expireTick_ > tick + nextTick - 1)) {
this->AsyncTimeout::scheduleTimeout(interval_ * tick); void HHWheelTimer::scheduleNextTimeout(int64_t nextTick, int64_t ticks) {
expireTick_ = tick + nextTick - 1; this->AsyncTimeout::scheduleTimeout(interval_ * ticks);
} expireTick_ = ticks + nextTick - 1;
} else {
this->AsyncTimeout::cancelTimeout();
}
} }
size_t HHWheelTimer::cancelTimeoutsFromList(CallbackList& timeouts) { size_t HHWheelTimer::cancelTimeoutsFromList(CallbackList& timeouts) {
......
...@@ -293,24 +293,42 @@ class HHWheelTimer : private folly::AsyncTimeout, ...@@ -293,24 +293,42 @@ class HHWheelTimer : private folly::AsyncTimeout,
int64_t calcNextTick(); int64_t calcNextTick();
int64_t calcNextTick(std::chrono::steady_clock::time_point curTime); int64_t calcNextTick(std::chrono::steady_clock::time_point curTime);
static bool inSameEpoch(int64_t tickA, int64_t tickB) {
return (tickA >> WHEEL_BITS) == (tickB >> WHEEL_BITS);
}
/** /**
* Schedule a given timeout by putting it into the appropriate bucket of the * Schedule a given timeout by putting it into the appropriate bucket of the
* wheel. * wheel.
* *
* @param callback Callback to fire after `timeout` * @param callback Callback to fire after `timeout`
* @param timeout Interval after which the `callback` should be * @param dueTick Tick at which the timer is due.
* fired.
* @param nextTickToProcess next tick that was not processed by the timer * @param nextTickToProcess next tick that was not processed by the timer
* yet. Can be less than nextTick if we're lagging. * yet. Can be less than nextTick if we're lagging.
* @nextTick next tick based on the actual time * @param nextTick next tick based on the actual time
*/ */
void scheduleTimeoutImpl( void scheduleTimeoutImpl(
Callback* callback, Callback* callback,
std::chrono::milliseconds timeout, int64_t dueTick,
int64_t nextTickToProcess, int64_t nextTickToProcess,
int64_t nextTick); int64_t nextTick);
/**
* Compute next required wheel tick to fire and schedule the timeout for that
* tick.
*
* @param nextTick next tick based on the actual time
*/
void scheduleNextTimeout(int64_t nextTick); void scheduleNextTimeout(int64_t nextTick);
/**
* Schedule next wheel timeout in a fixed number of wheel ticks.
*
* @param nextTick next tick based on the actual time
* @param ticks number of ticks in which the timer should fire
*/
void scheduleNextTimeout(int64_t nextTick, int64_t ticks);
size_t cancelTimeoutsFromList(CallbackList& timeouts); size_t cancelTimeoutsFromList(CallbackList& timeouts);
bool* processingCallbacksGuard_; bool* processingCallbacksGuard_;
......
...@@ -487,3 +487,15 @@ TEST_F(HHWheelTimerTest, prematureTimeout) { ...@@ -487,3 +487,15 @@ TEST_F(HHWheelTimerTest, prematureTimeout) {
t2.timestamps[0].getTime() - start); t2.timestamps[0].getTime() - start);
EXPECT_GE(elapsedMs.count(), timeout.count()); EXPECT_GE(elapsedMs.count(), timeout.count());
} }
TEST_F(HHWheelTimerTest, Level1) {
StackWheelTimer t(&eventBase, milliseconds(1));
TestTimeout tt;
// Schedule the timeout for the tick in a next epoch.
t.scheduleTimeout(&tt, std::chrono::milliseconds(500));
TimePoint start;
eventBase.loop();
TimePoint end;
ASSERT_EQ(tt.timestamps.size(), 1);
T_CHECK_TIMEOUT(start, end, milliseconds(500));
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment