From 360d4abeb8ae9675e1c12ef5f7693c761c7fd817 Mon Sep 17 00:00:00 2001 From: Joseph Beshay Date: Fri, 15 Nov 2024 18:41:23 -0800 Subject: [PATCH] Remove wakeUpImmediatelyOnPendingScheduledEvents from QuicEventBase Summary: As title. Reviewed By: lhuang04, kvtsoy Differential Revision: D65963823 fbshipit-source-id: 90caa7738726418b432b01febf40f4394b0333bc --- quic/common/events/LibevQuicEventBase.cpp | 32 ------------- quic/common/events/LibevQuicEventBase.h | 7 --- quic/common/events/QuicEventBase.h | 2 - quic/common/events/test/QuicEventBaseMock.h | 1 - .../events/test/QuicEventBaseTestBase.h | 47 +------------------ 5 files changed, 1 insertion(+), 88 deletions(-) diff --git a/quic/common/events/LibevQuicEventBase.cpp b/quic/common/events/LibevQuicEventBase.cpp index 25da00777..2f2d6a153 100644 --- a/quic/common/events/LibevQuicEventBase.cpp +++ b/quic/common/events/LibevQuicEventBase.cpp @@ -21,15 +21,6 @@ void libEvTimeoutCallback( wrapper->timeoutExpired(); } -void libEvTimeoutCallbackInternal( - struct ev_loop* /* loop */, - ev_timer* w, - int /* revents */) { - auto self = static_cast(w->data); - CHECK(self != nullptr); - self->checkCallbacks(); -} - void libEvPrepareCallback( struct ev_loop* /* loop */, ev_prepare* w, @@ -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 { @@ -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 { diff --git a/quic/common/events/LibevQuicEventBase.h b/quic/common/events/LibevQuicEventBase.h index cabe1aca5..39cfaca99 100644 --- a/quic/common/events/LibevQuicEventBase.h +++ b/quic/common/events/LibevQuicEventBase.h @@ -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) @@ -343,9 +339,6 @@ class LibevQuicEventBase // We're using it to execute delayed work given to us via runInLoop. ev_prepare prepareWatcher_; std::atomic loopThreadId_; - bool wakeUpImmediatelyOnPendingScheduledEvents_{false}; - ev_timer ev_timer_internal_; - bool internalTimerInitialized_{false}; bool useTimerFd_{false}; bool prioritizeTimers_{false}; }; diff --git a/quic/common/events/QuicEventBase.h b/quic/common/events/QuicEventBase.h index 85e720c28..4152fe18c 100644 --- a/quic/common/events/QuicEventBase.h +++ b/quic/common/events/QuicEventBase.h @@ -144,8 +144,6 @@ class QuicEventBase { virtual void terminateLoopSoon() = 0; - virtual void wakeUpImmediatelyOnPendingScheduledEvents() {} - [[nodiscard]] virtual std::chrono::milliseconds getTimerTickInterval() const = 0; diff --git a/quic/common/events/test/QuicEventBaseMock.h b/quic/common/events/test/QuicEventBaseMock.h index 08d34cef5..e89b96984 100644 --- a/quic/common/events/test/QuicEventBaseMock.h +++ b/quic/common/events/test/QuicEventBaseMock.h @@ -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 diff --git a/quic/common/events/test/QuicEventBaseTestBase.h b/quic/common/events/test/QuicEventBaseTestBase.h index 9b1aadd53..15811114b 100644 --- a/quic/common/events/test/QuicEventBaseTestBase.h +++ b/quic/common/events/test/QuicEventBaseTestBase.h @@ -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(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 @@ -224,7 +180,6 @@ REGISTER_TYPED_TEST_SUITE_P( NestedRunInLoopCallbackInThisIteration, NestedRunInLoopFuncInThisIteration, NestedRunInLoopCallbackInNextIteration, - NestedRunInLoopFuncInNextIteration, - NestedRunInLoopFuncInNextIterationWithTimerToWakeUp + NestedRunInLoopFuncInNextIteration // Add more tests here );