Commit 32236ec4 authored by Stepan Palamarchuk's avatar Stepan Palamarchuk Committed by Facebook Github Bot

Remove HHWheelTimer::Callback::getCurTime and its overrides

Summary:
This function is being overridden in only one class (with a duplicate) that is used in only a single test with no clear semantics of what is being tested (the only effect it has is that cascading logic will see 0, which can be achieved with a simple sleep).

Removing this customization point allows us to avoid doing additional call to steady_clock::now per each timeout (stacked diff) and thus improving performance by ~2x (measured as part of Fibers benchmark after migrating FiberManager to use HHWheelTimer, also stacked).

Reviewed By: jmswen

Differential Revision: D13624362

fbshipit-source-id: ef3abd066d2a4f46b369f65d64dd58399ebf5e46
parent 136b6536
...@@ -56,8 +56,7 @@ void HHWheelTimer::Callback::setScheduled( ...@@ -56,8 +56,7 @@ void HHWheelTimer::Callback::setScheduled(
assert(expiration_ == decltype(expiration_){}); assert(expiration_ == decltype(expiration_){});
wheel_ = wheel; wheel_ = wheel;
expiration_ = std::chrono::steady_clock::now() + timeout;
expiration_ = getCurTime() + timeout;
} }
void HHWheelTimer::Callback::cancelTimeoutImpl() { void HHWheelTimer::Callback::cancelTimeoutImpl() {
......
...@@ -105,20 +105,11 @@ class HHWheelTimer : private folly::AsyncTimeout, ...@@ -105,20 +105,11 @@ class HHWheelTimer : private folly::AsyncTimeout,
/** /**
* Get the time remaining until this timeout expires. Return 0 if this * Get the time remaining until this timeout expires. Return 0 if this
* timeout is not scheduled or expired. Otherwise, return expiration time * timeout is not scheduled or expired. Otherwise, return expiration
* minus getCurTime(). * time minus current time.
*/ */
std::chrono::milliseconds getTimeRemaining() { std::chrono::milliseconds getTimeRemaining() {
return getTimeRemaining(getCurTime()); return getTimeRemaining(std::chrono::steady_clock::now());
}
protected:
/**
* Don't override this unless you're doing a test. This is mainly here so
* that we can override it to simulate lag in steady_clock.
*/
virtual std::chrono::steady_clock::time_point getCurTime() {
return std::chrono::steady_clock::now();
} }
private: private:
......
...@@ -54,13 +54,6 @@ class TestTimeout : public HHWheelTimer::Callback { ...@@ -54,13 +54,6 @@ class TestTimeout : public HHWheelTimer::Callback {
std::function<void()> fn; std::function<void()> fn;
}; };
class TestTimeoutDelayed : public TestTimeout {
protected:
std::chrono::steady_clock::time_point getCurTime() override {
return std::chrono::steady_clock::now() - milliseconds(5);
}
};
struct HHWheelTimerTest : public ::testing::Test { struct HHWheelTimerTest : public ::testing::Test {
EventBase eventBase; EventBase eventBase;
}; };
......
...@@ -51,13 +51,6 @@ class TestTimeout : public HHWheelTimer::Callback { ...@@ -51,13 +51,6 @@ class TestTimeout : public HHWheelTimer::Callback {
std::function<void()> fn; std::function<void()> fn;
}; };
class TestTimeoutDelayed : public TestTimeout {
protected:
std::chrono::steady_clock::time_point getCurTime() override {
return std::chrono::steady_clock::now() - milliseconds(5);
}
};
struct HHWheelTimerTest : public ::testing::Test { struct HHWheelTimerTest : public ::testing::Test {
EventBase eventBase; EventBase eventBase;
}; };
...@@ -105,12 +98,13 @@ TEST_F(HHWheelTimerTest, FireOnce) { ...@@ -105,12 +98,13 @@ TEST_F(HHWheelTimerTest, FireOnce) {
TEST_F(HHWheelTimerTest, TestSchedulingWithinCallback) { TEST_F(HHWheelTimerTest, TestSchedulingWithinCallback) {
HHWheelTimer& t = eventBase.timer(); HHWheelTimer& t = eventBase.timer();
TestTimeout t1; TestTimeout t1, t2;
// Delayed to simulate the steady_clock counter lagging
TestTimeoutDelayed t2;
t.scheduleTimeout(&t1, milliseconds(500)); t.scheduleTimeout(&t1, milliseconds(500));
t1.fn = [&] { t.scheduleTimeout(&t2, milliseconds(1)); }; t1.fn = [&] {
t.scheduleTimeout(&t2, milliseconds(1));
std::this_thread::sleep_for(std::chrono::milliseconds(5));
};
// If t is in an inconsistent state, detachEventBase should fail. // If t is in an inconsistent state, detachEventBase should fail.
t2.fn = [&] { t.detachEventBase(); }; t2.fn = [&] { t.detachEventBase(); };
......
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