From fe12ab7241f32d43abe12ff295ffbdab877afdba Mon Sep 17 00:00:00 2001 From: Konstantin Tsoy Date: Wed, 12 Jul 2023 17:26:15 -0700 Subject: [PATCH] Hide direct timer invocations on the event base into the wrapper Summary: Needed for future libev-based event base impl. Reviewed By: hanidamlaj Differential Revision: D46670916 fbshipit-source-id: 55740ed05da9c1f54cc8639ece89d1f21aaff0d3 --- quic/api/QuicTransportBase.cpp | 28 ++++++++++--------- quic/api/QuicTransportBase.h | 18 +++++++----- quic/api/test/QuicTransportBaseTest.cpp | 2 +- quic/client/QuicClientTransport.h | 3 +- quic/client/connector/QuicConnector.cpp | 2 +- quic/client/connector/QuicConnector.h | 2 +- quic/client/state/ClientStateMachine.h | 2 +- quic/common/Events.cpp | 14 +++++++--- quic/common/Events.h | 12 +++++--- quic/common/EventsMobile.cpp | 14 +++++++--- quic/common/EventsMobile.h | 24 ++++++++++++---- quic/common/QuicEventBaseInterface.h | 11 ++++++-- .../QuicHappyEyeballsFunctions.cpp | 6 ++-- .../QuicHappyEyeballsFunctions.h | 4 +-- 14 files changed, 91 insertions(+), 51 deletions(-) diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 0c070c1c5..8a0ca5ebc 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -64,6 +64,12 @@ QuicTransportBase::QuicTransportBase( } } +void QuicTransportBase::scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::milliseconds timeout) { + qEvb_.scheduleTimeout(callback, timeout); +} + void QuicTransportBase::setPacingTimer( TimerHighRes::SharedPtr pacingTimer) noexcept { if (pacingTimer) { @@ -412,7 +418,7 @@ void QuicTransportBase::closeImpl( if (drainConnection) { // We ever drain once, and the object ever gets created once. DCHECK(!drainTimeout_.isScheduled()); - getEventBase()->timer().scheduleTimeout( + scheduleTimeout( &drainTimeout_, folly::chrono::ceil( kDrainFactor * calculatePTO(*conn_))); @@ -1965,13 +1971,12 @@ void QuicTransportBase::setIdleTimer() { auto peerIdleTimeout = conn_->peerIdleTimeout > 0ms ? conn_->peerIdleTimeout : localIdleTimeout; auto idleTimeout = timeMin(localIdleTimeout, peerIdleTimeout); - getEventBase()->timer().scheduleTimeout(&idleTimeout_, idleTimeout); + scheduleTimeout(&idleTimeout_, idleTimeout); auto idleTimeoutCount = idleTimeout.count(); if (conn_->transportSettings.enableKeepalive) { std::chrono::milliseconds keepaliveTimeout = std::chrono::milliseconds( idleTimeoutCount - static_cast(idleTimeoutCount * .15)); - getEventBase()->timer().scheduleTimeout( - &keepaliveTimeout_, keepaliveTimeout); + scheduleTimeout(&keepaliveTimeout_, keepaliveTimeout); } } @@ -2734,9 +2739,8 @@ void QuicTransportBase::scheduleLossTimeout(std::chrono::milliseconds timeout) { if (closeState_ == CloseState::CLOSED) { return; } - auto& wheelTimer = getEventBase()->timer(); - timeout = timeMax(timeout, wheelTimer.getTickInterval()); - wheelTimer.scheduleTimeout(&lossTimeout_, timeout); + timeout = timeMax(timeout, qEvb_.getTimerTickInterval()); + scheduleTimeout(&lossTimeout_, timeout); } void QuicTransportBase::scheduleAckTimeout() { @@ -2752,16 +2756,15 @@ void QuicTransportBase::scheduleAckTimeout() { if (conn_->ackStates.appDataAckState.ackFrequencySequenceNumber) { factoredRtt = conn_->ackStates.maxAckDelay; } - auto& wheelTimer = getEventBase()->timer(); auto timeout = timeMax( std::chrono::duration_cast( - wheelTimer.getTickInterval()), + qEvb_.getTimerTickInterval()), timeMin(conn_->ackStates.maxAckDelay, factoredRtt)); auto timeoutMs = folly::chrono::ceil(timeout); VLOG(10) << __func__ << " timeout=" << timeoutMs.count() << "ms" << " factoredRtt=" << factoredRtt.count() << "us" << " " << *this; - wheelTimer.scheduleTimeout(&ackTimeout_, timeoutMs); + scheduleTimeout(&ackTimeout_, timeoutMs); } } else { if (ackTimeout_.isScheduled()) { @@ -2780,8 +2783,7 @@ void QuicTransportBase::schedulePingTimeout( } pingCallback_ = pingCb; - auto& wheelTimer = getEventBase()->timer(); - wheelTimer.scheduleTimeout(&pingTimeout_, timeout); + scheduleTimeout(&pingTimeout_, timeout); } void QuicTransportBase::schedulePathValidationTimeout() { @@ -2805,7 +2807,7 @@ void QuicTransportBase::schedulePathValidationTimeout() { auto timeoutMs = folly::chrono::ceil(validationTimeout); VLOG(10) << __func__ << " timeout=" << timeoutMs.count() << "ms " << *this; - getEventBase()->timer().scheduleTimeout(&pathValidationTimeout_, timeoutMs); + scheduleTimeout(&pathValidationTimeout_, timeoutMs); } } diff --git a/quic/api/QuicTransportBase.h b/quic/api/QuicTransportBase.h index 5579acd8f..5dc796929 100644 --- a/quic/api/QuicTransportBase.h +++ b/quic/api/QuicTransportBase.h @@ -44,6 +44,10 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { ~QuicTransportBase() override; + void scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::milliseconds timeout); + void setPacingTimer(TimerHighRes::SharedPtr pacingTimer) noexcept; folly::EventBase* getEventBase() const override; @@ -447,7 +451,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { void clearBackgroundModeParameters(); // Timeout functions - class LossTimeout : public folly::HHWheelTimer::Callback { + class LossTimeout : public QuicTimerCallback { public: ~LossTimeout() override = default; @@ -468,7 +472,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { QuicTransportBase* transport_; }; - class AckTimeout : public folly::HHWheelTimer::Callback { + class AckTimeout : public QuicTimerCallback { public: ~AckTimeout() override = default; @@ -488,7 +492,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { QuicTransportBase* transport_; }; - class PingTimeout : public folly::HHWheelTimer::Callback { + class PingTimeout : public QuicTimerCallback { public: ~PingTimeout() override = default; @@ -508,7 +512,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { QuicTransportBase* transport_; }; - class PathValidationTimeout : public folly::HHWheelTimer::Callback { + class PathValidationTimeout : public QuicTimerCallback { public: ~PathValidationTimeout() override = default; @@ -529,7 +533,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { QuicTransportBase* transport_; }; - class IdleTimeout : public folly::HHWheelTimer::Callback { + class IdleTimeout : public QuicTimerCallback { public: ~IdleTimeout() override = default; @@ -550,7 +554,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { QuicTransportBase* transport_; }; - class KeepaliveTimeout : public folly::HHWheelTimer::Callback { + class KeepaliveTimeout : public QuicTimerCallback { public: ~KeepaliveTimeout() override = default; @@ -571,7 +575,7 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { // DrainTimeout is a bit different from other timeouts. It needs to hold a // shared_ptr to the transport, since if a DrainTimeout is scheduled, // transport cannot die. - class DrainTimeout : public folly::HHWheelTimer::Callback { + class DrainTimeout : public QuicTimerCallback { public: ~DrainTimeout() override = default; diff --git a/quic/api/test/QuicTransportBaseTest.cpp b/quic/api/test/QuicTransportBaseTest.cpp index 6223d56f4..bcae9b9f3 100644 --- a/quic/api/test/QuicTransportBaseTest.cpp +++ b/quic/api/test/QuicTransportBaseTest.cpp @@ -1723,7 +1723,7 @@ TEST_P(QuicTransportImplTestBase, ConnectionErrorUnhandledException) { } TEST_P(QuicTransportImplTestBase, LossTimeoutNoLessThanTickInterval) { - auto tickInterval = evb->timer().getTickInterval(); + auto tickInterval = evb->getTimerTickInterval(); transport->scheduleLossTimeout(tickInterval - 1ms); EXPECT_NEAR( tickInterval.count(), diff --git a/quic/client/QuicClientTransport.h b/quic/client/QuicClientTransport.h index c78c42916..337a98e2c 100644 --- a/quic/client/QuicClientTransport.h +++ b/quic/client/QuicClientTransport.h @@ -166,8 +166,7 @@ class QuicClientTransport clientConn_->newToken = std::move(token); } - class HappyEyeballsConnAttemptDelayTimeout - : public folly::HHWheelTimer::Callback { + class HappyEyeballsConnAttemptDelayTimeout : public QuicTimerCallback { public: explicit HappyEyeballsConnAttemptDelayTimeout( QuicClientTransport* transport) diff --git a/quic/client/connector/QuicConnector.cpp b/quic/client/connector/QuicConnector.cpp index c9947eaba..a5dc6c8cd 100644 --- a/quic/client/connector/QuicConnector.cpp +++ b/quic/client/connector/QuicConnector.cpp @@ -92,7 +92,7 @@ void QuicConnector::connect( void QuicConnector::doConnect(std::chrono::milliseconds connectTimeout) { connectStart_ = std::chrono::steady_clock::now(); - quicClient_->getEventBase()->timer().scheduleTimeout(this, connectTimeout); + quicClient_->scheduleTimeout(this, connectTimeout); quicClient_->start(this, nullptr); } diff --git a/quic/client/connector/QuicConnector.h b/quic/client/connector/QuicConnector.h index 8538557e6..718bb9402 100644 --- a/quic/client/connector/QuicConnector.h +++ b/quic/client/connector/QuicConnector.h @@ -23,7 +23,7 @@ namespace quic { * drops the connection. */ class QuicConnector : private quic::QuicSocket::ConnectionSetupCallback, - private folly::HHWheelTimer::Callback { + private QuicTimerCallback { public: /** * Callback to report success/error and first packet processed. diff --git a/quic/client/state/ClientStateMachine.h b/quic/client/state/ClientStateMachine.h index a078533d2..ecc642f9b 100644 --- a/quic/client/state/ClientStateMachine.h +++ b/quic/client/state/ClientStateMachine.h @@ -76,7 +76,7 @@ struct QuicClientConnectionState : public QuicConnectionStateBase { struct HappyEyeballsState { // Delay timer - folly::HHWheelTimer::Callback* connAttemptDelayTimeout{nullptr}; + QuicTimerCallback* connAttemptDelayTimeout{nullptr}; // IPv6 peer address folly::SocketAddress v6PeerAddress; diff --git a/quic/common/Events.cpp b/quic/common/Events.cpp index b6f03c604..549e5f33d 100644 --- a/quic/common/Events.cpp +++ b/quic/common/Events.cpp @@ -50,10 +50,6 @@ bool QuicEventBase::scheduleTimeoutHighRes( return backingEvb_->scheduleTimeoutHighRes(obj, timeout); } -QuicHHWheelTimer& QuicEventBase::timer() { - return backingEvb_->timer(); -} - bool QuicEventBase::loopOnce(int flags) { return backingEvb_->loopOnce(flags); } @@ -74,6 +70,16 @@ void QuicEventBase::terminateLoopSoon() { return backingEvb_->terminateLoopSoon(); } +void QuicEventBase::scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::milliseconds timeout) { + return backingEvb_->timer().scheduleTimeout(callback, timeout); +} + +std::chrono::milliseconds QuicEventBase::getTimerTickInterval() const { + return backingEvb_->timer().getTickInterval(); +} + } // namespace quic #endif // !MVFST_USE_LIBEV diff --git a/quic/common/Events.h b/quic/common/Events.h index b50989b84..a1dbdfe7c 100644 --- a/quic/common/Events.h +++ b/quic/common/Events.h @@ -26,14 +26,14 @@ namespace quic { using QuicEventBaseLoopCallback = folly::EventBase::LoopCallback; using QuicBackingEventBase = folly::EventBase; using QuicAsyncTimeout = folly::AsyncTimeout; -using QuicHHWheelTimer = folly::HHWheelTimer; +using QuicTimerCallback = folly::HHWheelTimer::Callback; #endif class QuicEventBase : public QuicEventBaseInterface< QuicEventBaseLoopCallback, QuicBackingEventBase, QuicAsyncTimeout, - QuicHHWheelTimer> { + QuicTimerCallback> { public: QuicEventBase() = default; explicit QuicEventBase(QuicBackingEventBase* evb) : backingEvb_(evb) {} @@ -61,8 +61,6 @@ class QuicEventBase : public QuicEventBaseInterface< QuicAsyncTimeout* obj, std::chrono::microseconds timeout) override; - QuicHHWheelTimer& timer() override; - bool loopOnce(int flags = 0) override; bool loop() override; @@ -73,6 +71,12 @@ class QuicEventBase : public QuicEventBaseInterface< void terminateLoopSoon() override; + void scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::milliseconds timeout) override; + + [[nodiscard]] std::chrono::milliseconds getTimerTickInterval() const override; + private: QuicBackingEventBase* backingEvb_{nullptr}; }; diff --git a/quic/common/EventsMobile.cpp b/quic/common/EventsMobile.cpp index 8c529321c..4753ad9f9 100644 --- a/quic/common/EventsMobile.cpp +++ b/quic/common/EventsMobile.cpp @@ -50,10 +50,6 @@ bool QuicEventBase::scheduleTimeoutHighRes( return backingEvb_->scheduleTimeoutHighRes(obj, timeout); } -QuicHHWheelTimer& QuicEventBase::timer() { - return backingEvb_->timer(); -} - bool QuicEventBase::loopOnce(int flags) { return backingEvb_->loopOnce(flags); } @@ -74,6 +70,16 @@ void QuicEventBase::terminateLoopSoon() { return backingEvb_->terminateLoopSoon(); } +void QuicEventBase::scheduleTimeout( + QuicTimerCallback* callback, + std::chrono::milliseconds timeout) { + return backingEvb_->scheduleTimeout(callback, timeout); +} + +std::chrono::milliseconds QuicEventBase::getTimerTickInterval() const { + return backingEvb_->getTimerTickInterval(); +} + } // namespace quic #endif // MVFST_USE_LIBEV diff --git a/quic/common/EventsMobile.h b/quic/common/EventsMobile.h index 26ddcde84..bf1341a68 100644 --- a/quic/common/EventsMobile.h +++ b/quic/common/EventsMobile.h @@ -27,9 +27,18 @@ class QuicAsyncTimeout { QuicAsyncTimeout() = default; }; -class QuicHHWheelTimer { +class QuicTimerCallback { public: - QuicHHWheelTimer() = default; + virtual ~QuicTimerCallback() = default; + virtual void timeoutExpired() noexcept = 0; + virtual void callbackCanceled() noexcept { + timeoutExpired(); + } + bool isScheduled() const { + return false; + } + + void cancelTimeout() {} }; class QuicBackingEventBase { @@ -52,9 +61,6 @@ class QuicBackingEventBase { std::chrono::microseconds /* timeout */) { return false; } - QuicHHWheelTimer& timer() { - return timer_; - } bool loopOnce(int /* flags */) { return false; } @@ -67,6 +73,14 @@ class QuicBackingEventBase { } void terminateLoopSoon() {} + void scheduleTimeout( + QuicTimerCallback* /* callback */, + std::chrono::milliseconds /* timeout */) {} + + std::chrono::milliseconds getTimerTickInterval() const { + return std::chrono::milliseconds(0); + } + private: QuicHHWheelTimer timer_; }; diff --git a/quic/common/QuicEventBaseInterface.h b/quic/common/QuicEventBaseInterface.h index 3011da235..1de89834a 100644 --- a/quic/common/QuicEventBaseInterface.h +++ b/quic/common/QuicEventBaseInterface.h @@ -17,7 +17,7 @@ template < class LoopCallbackT, class BackingEventBaseT, class AsyncTimeoutT, - class TimerT> + class TimerCallbackT> class QuicEventBaseInterface { public: virtual ~QuicEventBaseInterface() = default; @@ -42,8 +42,6 @@ class QuicEventBaseInterface { AsyncTimeoutT* obj, std::chrono::microseconds timeout) = 0; - virtual TimerT& timer() = 0; - virtual bool loopOnce(int flags) = 0; virtual bool loop() = 0; @@ -53,6 +51,13 @@ class QuicEventBaseInterface { virtual bool loopIgnoreKeepAlive() = 0; virtual void terminateLoopSoon() = 0; + + virtual void scheduleTimeout( + TimerCallbackT* callback, + std::chrono::milliseconds timeout) = 0; + + [[nodiscard]] virtual std::chrono::milliseconds getTimerTickInterval() + const = 0; }; } // namespace quic diff --git a/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp b/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp index 726448844..3b9caef6b 100644 --- a/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp +++ b/quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp @@ -66,7 +66,7 @@ void startHappyEyeballs( QuicClientConnectionState& connection, QuicEventBase* evb, sa_family_t cachedFamily, - folly::HHWheelTimer::Callback& connAttemptDelayTimeout, + QuicTimerCallback& connAttemptDelayTimeout, std::chrono::milliseconds connAttempDelay, QuicAsyncUDPSocketWrapper::ErrMessageCallback* errMsgCallback, QuicAsyncUDPSocketWrapper::ReadCallback* readCallback, @@ -93,7 +93,7 @@ void startHappyEyeballs( connection.happyEyeballsState.connAttemptDelayTimeout = &connAttemptDelayTimeout; - evb->timer().scheduleTimeout(&connAttemptDelayTimeout, connAttempDelay); + evb->scheduleTimeout(&connAttemptDelayTimeout, connAttempDelay); try { happyEyeballsSetUpSocket( @@ -188,7 +188,7 @@ void happyEyeballsStartSecondSocket( void happyEyeballsOnDataReceived( QuicClientConnectionState& connection, - folly::HHWheelTimer::Callback& connAttemptDelayTimeout, + QuicTimerCallback& connAttemptDelayTimeout, std::unique_ptr& socket, const folly::SocketAddress& peerAddress) { if (connection.happyEyeballsState.finished) { diff --git a/quic/happyeyeballs/QuicHappyEyeballsFunctions.h b/quic/happyeyeballs/QuicHappyEyeballsFunctions.h index d21d3dfa6..7e25e8d33 100644 --- a/quic/happyeyeballs/QuicHappyEyeballsFunctions.h +++ b/quic/happyeyeballs/QuicHappyEyeballsFunctions.h @@ -37,7 +37,7 @@ void startHappyEyeballs( QuicClientConnectionState& connection, QuicEventBase* evb, sa_family_t cachedFamily, - folly::HHWheelTimer::Callback& connAttemptDelayTimeout, + QuicTimerCallback& connAttemptDelayTimeout, std::chrono::milliseconds connAttemptDelay, QuicAsyncUDPSocketWrapper::ErrMessageCallback* errMsgCallback, QuicAsyncUDPSocketWrapper::ReadCallback* readCallback, @@ -57,7 +57,7 @@ void happyEyeballsStartSecondSocket( void happyEyeballsOnDataReceived( QuicClientConnectionState& connection, - folly::HHWheelTimer::Callback& connAttemptDelayTimeout, + QuicTimerCallback& connAttemptDelayTimeout, std::unique_ptr& socket, const folly::SocketAddress& peerAddress); } // namespace quic