Combine HighResQuicTimer and TimerFDQuicTimer into one type

Summary: Breaking these two types apart in D51413481 and using them separately for the client and server resulted in the TimerFD not being used anymore for non-mobile clients even when it's available. The combines them back into one type and restore the compile-time selection to use the TimerFD when not on mobile. Reviewed By: sharmafb Differential Revision: D52570625 fbshipit-source-id: 3f6117d67558f4506f9fe9efd9d0ce0ad2efbbbc
facebook · Jan 9, 2024 · dd24931 · dd24931
1 parent 18ea0d6
commit dd24931
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 125 deletions.
diff --git a/quic/common/CMakeLists.txt b/quic/common/CMakeLists.txt
@@ -31,7 +31,6 @@ add_library(
   mvfst_looper
   FunctionLooper.cpp
   events/HighResQuicTimer.cpp
-  events/TimerFDQuicTimer.cpp
 )
 
 set_property(TARGET mvfst_looper PROPERTY VERSION ${PACKAGE_VERSION})

diff --git a/quic/common/events/HighResQuicTimer.cpp b/quic/common/events/HighResQuicTimer.cpp
@@ -8,12 +8,23 @@
 #include <quic/common/events/HighResQuicTimer.h>
 
 namespace quic {
+
+#ifdef QUIC_USE_TIMERFD_TIMEOUT_MGR
+HighResQuicTimer::HighResQuicTimer(
+    folly::EventBase* eventBase,
+    std::chrono::microseconds intervalDuration)
+    : timeoutMgr_(eventBase) {
+  wheelTimer_ =
+      folly::HHWheelTimerHighRes::newTimer(&timeoutMgr_, intervalDuration);
+}
+#else
 HighResQuicTimer::HighResQuicTimer(
     folly::EventBase* eventBase,
     std::chrono::microseconds intervalDuration) {
   wheelTimer_ =
       folly::HHWheelTimerHighRes::newTimer(eventBase, intervalDuration);
 }
+#endif
 
 std::chrono::microseconds HighResQuicTimer::getTickInterval() const {
   return wheelTimer_->getTickInterval();

diff --git a/quic/common/events/HighResQuicTimer.h b/quic/common/events/HighResQuicTimer.h
@@ -12,6 +12,11 @@
 #include <folly/io/async/EventBase.h>
 #include <folly/io/async/HHWheelTimer.h>
 
+#if !FOLLY_MOBILE
+#define QUIC_USE_TIMERFD_TIMEOUT_MGR
+#include <folly/experimental/STTimerFDTimeoutManager.h>
+#endif
+
 namespace quic {
 
 class HighResQuicTimer : public QuicTimer {
@@ -66,6 +71,9 @@ class HighResQuicTimer : public QuicTimer {
   };
 
   folly::HHWheelTimerHighRes::UniquePtr wheelTimer_;
+#ifdef QUIC_USE_TIMERFD_TIMEOUT_MGR
+  folly::STTimerFDTimeoutManager timeoutMgr_;
+#endif
 };
 
 } // namespace quic
diff --git a/quic/common/events/TimerFDQuicTimer.cpp b/quic/common/events/TimerFDQuicTimer.cpp
diff --git a/quic/common/events/TimerFDQuicTimer.h b/quic/common/events/TimerFDQuicTimer.h
diff --git a/quic/server/QuicServerWorker.cpp b/quic/server/QuicServerWorker.cpp
@@ -180,7 +180,7 @@ void QuicServerWorker::setUnfinishedHandshakeLimit(
 void QuicServerWorker::start() {
   CHECK(socket_);
   if (!pacingTimer_) {
-    pacingTimer_ = std::make_unique<TimerFDQuicTimer>(
+    pacingTimer_ = std::make_unique<HighResQuicTimer>(
         evb_.get(), transportSettings_.pacingTimerResolution);
   }
   socket_->resumeRead(this);
@@ -618,7 +618,7 @@ void QuicServerWorker::forwardNetworkData(
 }
 
 void QuicServerWorker::setPacingTimer(
-    TimerFDQuicTimer::SharedPtr pacingTimer) noexcept {
+    QuicTimer::SharedPtr pacingTimer) noexcept {
   pacingTimer_ = std::move(pacingTimer);
 }
 

diff --git a/quic/server/QuicServerWorker.h b/quic/server/QuicServerWorker.h
@@ -20,7 +20,7 @@
 #include <quic/codec/ConnectionIdAlgo.h>
 #include <quic/codec/QuicConnectionId.h>
 #include <quic/common/BufAccessor.h>
-#include <quic/common/events/TimerFDQuicTimer.h>
+#include <quic/common/events/HighResQuicTimer.h>
 #include <quic/congestion_control/CongestionControllerFactory.h>
 #include <quic/server/QuicServerPacketRouter.h>
 #include <quic/server/QuicServerTransportFactory.h>
@@ -180,7 +180,7 @@ class QuicServerWorker : public FollyAsyncUDPSocketAlias::ReadCallback,
 
   folly::EventBase* getEventBase() const;
 
-  void setPacingTimer(TimerFDQuicTimer::SharedPtr pacingTimer) noexcept;
+  void setPacingTimer(QuicTimer::SharedPtr pacingTimer) noexcept;
 
   /*
    * Take in a function to supply overrides for transport parameters, given
@@ -666,7 +666,7 @@ class QuicServerWorker : public FollyAsyncUDPSocketAlias::ReadCallback,
   enum ProcessId processId_ { ProcessId::ZERO };
   TakeoverPacketHandler takeoverPktHandler_;
   bool packetForwardingEnabled_{false};
-  TimerFDQuicTimer::SharedPtr pacingTimer_;
+  QuicTimer::SharedPtr pacingTimer_;
 
   // Used to override certain transport parameters, given the client address
   TransportSettingsOverrideFn transportSettingsOverrideFn_;