Skip to content

Commit

Permalink
Remove wakeUpImmediatelyOnPendingScheduledEvents from QuicEventBase
Browse files Browse the repository at this point in the history
Summary: As title.

Reviewed By: lhuang04, kvtsoy

Differential Revision: D65963823

fbshipit-source-id: 90caa7738726418b432b01febf40f4394b0333bc
  • Loading branch information
jbeshay authored and facebook-github-bot committed Nov 16, 2024
1 parent eb26c68 commit 360d4ab
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 88 deletions.
32 changes: 0 additions & 32 deletions quic/common/events/LibevQuicEventBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ void libEvTimeoutCallback(
wrapper->timeoutExpired();
}

void libEvTimeoutCallbackInternal(
struct ev_loop* /* loop */,
ev_timer* w,
int /* revents */) {
auto self = static_cast<quic::LibevQuicEventBase*>(w->data);
CHECK(self != nullptr);
self->checkCallbacks();
}

void libEvPrepareCallback(
struct ev_loop* /* loop */,
ev_prepare* w,
Expand All @@ -54,9 +45,6 @@ LibevQuicEventBase::~LibevQuicEventBase() {
// If the loop has been destroyed, skip the ev loop operations.
if (loopWeak_->get()) {
ev_prepare_stop(ev_loop_, &prepareWatcher_);
if (internalTimerInitialized_) {
ev_timer_stop(ev_loop_, &ev_timer_internal_);
}
}

struct FunctionLoopCallbackDisposer {
Expand Down Expand Up @@ -216,26 +204,6 @@ void LibevQuicEventBase::checkCallbacks() {
currentLoopWrappers.front().runLoopCallback();
}
runOnceCallbackWrappers_ = nullptr;

if (wakeUpImmediatelyOnPendingScheduledEvents_ &&
!loopCallbackWrappers_.empty()) {
// We have newly added events for the next loop. Wake up as soon as
// possible.
if (!internalTimerInitialized_) {
internalTimerInitialized_ = true;
ev_timer_init(
&ev_timer_internal_,
libEvTimeoutCallbackInternal,
0.0001 /* after */,
0. /* repeat */);
ev_timer_internal_.data = this;
ev_timer_start(ev_loop_, &ev_timer_internal_);
} else {
ev_timer_stop(ev_loop_, &ev_timer_internal_);
ev_timer_set(&ev_timer_internal_, 0.0001, 0.);
ev_timer_start(ev_loop_, &ev_timer_internal_);
}
}
}

bool LibevQuicEventBase::isInEventBaseThread() const {
Expand Down
7 changes: 0 additions & 7 deletions quic/common/events/LibevQuicEventBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,6 @@ class LibevQuicEventBase
return std::chrono::milliseconds(1);
}

void wakeUpImmediatelyOnPendingScheduledEvents() override {
wakeUpImmediatelyOnPendingScheduledEvents_ = true;
}

// MUST be called before any timers are scheduled.
void useTimerFd() {
#if defined(HAS_TIMERFD)
Expand Down Expand Up @@ -343,9 +339,6 @@ class LibevQuicEventBase
// We're using it to execute delayed work given to us via runInLoop.
ev_prepare prepareWatcher_;
std::atomic<std::thread::id> loopThreadId_;
bool wakeUpImmediatelyOnPendingScheduledEvents_{false};
ev_timer ev_timer_internal_;
bool internalTimerInitialized_{false};
bool useTimerFd_{false};
bool prioritizeTimers_{false};
};
Expand Down
2 changes: 0 additions & 2 deletions quic/common/events/QuicEventBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ class QuicEventBase {

virtual void terminateLoopSoon() = 0;

virtual void wakeUpImmediatelyOnPendingScheduledEvents() {}

[[nodiscard]] virtual std::chrono::milliseconds getTimerTickInterval()
const = 0;

Expand Down
1 change: 0 additions & 1 deletion quic/common/events/test/QuicEventBaseMock.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class QuicEventBaseMock : public QuicEventBase {
MOCK_METHOD((bool), loopIgnoreKeepAlive, ());
MOCK_METHOD((void), terminateLoopSoon, ());
MOCK_METHOD((std::chrono::milliseconds), getTimerTickInterval, (), (const));
MOCK_METHOD((void), wakeUpImmediatelyOnPendingScheduledEvents, ());
};

} // namespace quic::test
47 changes: 1 addition & 46 deletions quic/common/events/test/QuicEventBaseTestBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,50 +172,6 @@ TYPED_TEST_P(QuicEventBaseTest, NestedRunInLoopFuncInNextIteration) {
// Verify that the nested callback has been executed after the second loop.
EXPECT_EQ(cb->callbackCount_, 2);
}

TYPED_TEST_P(
QuicEventBaseTest,
NestedRunInLoopFuncInNextIterationWithTimerToWakeUp) {
// Verify that scheduling a callback within the callback itself gets executed
// in the loop not on top of the stack. The nested callback is a function
// rather than a callback class.
class TestCallback : public quic::QuicEventBaseLoopCallback {
public:
explicit TestCallback(quic::QuicEventBase* qEvb) : qEvb_(qEvb) {}
void runLoopCallback() noexcept override {
auto currenCount = ++callbackCount_;
if (currenCount == 1) {
// Schedule a function callback that increments the callback count in
// the next loop.
qEvb_->runInLoop([&]() { callbackCount_++; }, false);

// Verify that the nested callback has not been executed yet.
EXPECT_EQ(callbackCount_, currenCount);
}
}

uint8_t callbackCount_{0};

private:
quic::QuicEventBase* qEvb_;
};

auto& qEvb = this->qEvb_;
qEvb->wakeUpImmediatelyOnPendingScheduledEvents();

auto cb = std::make_unique<TestCallback>(qEvb.get());
qEvb->runInLoop(cb.get());
qEvb->loopOnce();

// Verify that only one callback has been executed in the first loop.
ASSERT_EQ(cb->callbackCount_, 1);

qEvb->loopOnce();

// Verify that the nested callback has been executed after the second loop.
EXPECT_EQ(cb->callbackCount_, 2);
}

// Tests end here

// All tests must be registered
Expand All @@ -224,7 +180,6 @@ REGISTER_TYPED_TEST_SUITE_P(
NestedRunInLoopCallbackInThisIteration,
NestedRunInLoopFuncInThisIteration,
NestedRunInLoopCallbackInNextIteration,
NestedRunInLoopFuncInNextIteration,
NestedRunInLoopFuncInNextIterationWithTimerToWakeUp
NestedRunInLoopFuncInNextIteration
// Add more tests here
);

0 comments on commit 360d4ab

Please sign in to comment.