Fall back to evb timer if no pacer timer set

Reviewed By: kvtsoy Differential Revision: D65848549 fbshipit-source-id: 38c1e9b8c820dbe2848cb132d7e49079c45eb67d
facebook · Nov 14, 2024 · 95898ef · 95898ef
1 parent a5fd828
commit 95898ef
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 39 deletions.
diff --git a/quic/api/QuicTransportBaseLite.cpp b/quic/api/QuicTransportBaseLite.cpp
@@ -2751,23 +2751,18 @@ void QuicTransportBaseLite::setTransportSettings(
       }
       return 0us;
     });
-    if (writeLooper_->hasPacingTimer()) {
-      bool usingBbr =
-          (conn_->transportSettings.defaultCongestionController ==
-               CongestionControlType::BBR ||
-           conn_->transportSettings.defaultCongestionController ==
-               CongestionControlType::BBRTesting ||
-           conn_->transportSettings.defaultCongestionController ==
-               CongestionControlType::BBR2);
-      auto minCwnd = usingBbr ? kMinCwndInMssForBbr
-                              : conn_->transportSettings.minCwndInMss;
-      conn_->pacer = std::make_unique<TokenlessPacer>(*conn_, minCwnd);
-      conn_->pacer->setExperimental(conn_->transportSettings.experimentalPacer);
-      conn_->canBePaced = conn_->transportSettings.pacingEnabledFirstFlight;
-    } else {
-      LOG(ERROR) << "Pacing cannot be enabled without a timer";
-      conn_->transportSettings.pacingEnabled = false;
-    }
+    bool usingBbr =
+        (conn_->transportSettings.defaultCongestionController ==
+             CongestionControlType::BBR ||
+         conn_->transportSettings.defaultCongestionController ==
+             CongestionControlType::BBRTesting ||
+         conn_->transportSettings.defaultCongestionController ==
+             CongestionControlType::BBR2);
+    auto minCwnd =
+        usingBbr ? kMinCwndInMssForBbr : conn_->transportSettings.minCwndInMss;
+    conn_->pacer = std::make_unique<TokenlessPacer>(*conn_, minCwnd);
+    conn_->pacer->setExperimental(conn_->transportSettings.experimentalPacer);
+    conn_->canBePaced = conn_->transportSettings.pacingEnabledFirstFlight;
   }
   setCongestionControl(conn_->transportSettings.defaultCongestionController);
   if (conn_->transportSettings.datagramConfig.enabled) {
@@ -2840,8 +2835,7 @@ void QuicTransportBaseLite::validateCongestionAndPacing(
   if ((type == CongestionControlType::BBR ||
        type == CongestionControlType::BBRTesting ||
        type == CongestionControlType::BBR2) &&
-      (!conn_->transportSettings.pacingEnabled ||
-       !writeLooper_->hasPacingTimer())) {
+      !conn_->transportSettings.pacingEnabled) {
     LOG(ERROR) << "Unpaced BBR isn't supported";
     type = CongestionControlType::Cubic;
   }

diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp
@@ -3916,12 +3916,12 @@ TEST_F(QuicTransportTest, NotifyPendingWriteConnBufferFreeUpSpace) {
       NetworkData(ReceivedUdpPacket(IOBuf::copyBuffer("fake data"))));
 }
 
-TEST_F(QuicTransportTest, NoPacingTimerNoPacing) {
+TEST_F(QuicTransportTest, NoPacingTimerStillPaced) {
   TransportSettings transportSettings;
   transportSettings.pacingEnabled = true;
   transport_->setTransportSettings(transportSettings);
   transport_->getConnectionState().canBePaced = true;
-  EXPECT_FALSE(isConnectionPaced(transport_->getConnectionState()));
+  EXPECT_TRUE(isConnectionPaced(transport_->getConnectionState()));
 }
 
 TEST_F(QuicTransportTest, SetPacingTimerThenEnablesPacing) {

diff --git a/quic/common/FunctionLooper.cpp b/quic/common/FunctionLooper.cpp
@@ -29,10 +29,6 @@ void FunctionLooper::setPacingTimer(QuicTimer::SharedPtr pacingTimer) noexcept {
   pacingTimer_ = std::move(pacingTimer);
 }
 
-bool FunctionLooper::hasPacingTimer() const noexcept {
-  return pacingTimer_ != nullptr;
-}
-
 void FunctionLooper::setPacingFunction(
     folly::Function<std::chrono::microseconds()>&& pacingFunc) {
   CHECK(pacingFunc);
@@ -59,12 +55,18 @@ void FunctionLooper::commonLoopBody() noexcept {
 }
 
 bool FunctionLooper::schedulePacingTimeout() noexcept {
-  if (pacingFunc_ && pacingTimer_ && !isTimerCallbackScheduled()) {
+  if (pacingFunc_ && (pacingTimer_ || evb_) && !isTimerCallbackScheduled()) {
     auto timeUntilWrite = pacingFunc_();
     if (timeUntilWrite != 0us) {
       nextPacingTime_ = Clock::now() + timeUntilWrite;
-      pacingTimer_->scheduleTimeout(this, timeUntilWrite);
-      return true;
+      if (pacingTimer_) {
+        pacingTimer_->scheduleTimeout(this, timeUntilWrite);
+        return true;
+      }
+      if (evb_) {
+        evb_->scheduleTimeoutHighRes(this, timeUntilWrite);
+        return true;
+      }
     }
   }
   return false;
@@ -93,13 +95,13 @@ void FunctionLooper::run(bool thisIteration, bool runInline) noexcept {
     return;
   }
   if (isLoopCallbackScheduled() ||
-      (!fireLoopEarly_ && pacingTimer_ && isTimerCallbackScheduled())) {
+      (!fireLoopEarly_ && pacingFunc_ && isTimerCallbackScheduled())) {
     VLOG(10) << __func__ << ": " << type_ << " already scheduled";
     return;
   }
   // If we are pacing, we're about to write again, if it's close, just write
   // now.
-  if (pacingTimer_ && isTimerCallbackScheduled()) {
+  if (pacingFunc_ && isTimerCallbackScheduled()) {
     auto n = Clock::now();
     auto timeUntilWrite = nextPacingTime_ < n
         ? 0us
@@ -122,17 +124,15 @@ void FunctionLooper::stop() noexcept {
   if (evb_) {
     cancelLoopCallback();
   }
-  if (pacingTimer_) {
-    cancelTimerCallback();
-  }
+  cancelTimerCallback();
 }
 
 bool FunctionLooper::isRunning() const {
   return running_;
 }
 
 bool FunctionLooper::isPacingScheduled() {
-  return pacingTimer_ && isTimerCallbackScheduled();
+  return pacingFunc_ && isTimerCallbackScheduled();
 }
 
 bool FunctionLooper::isLoopCallbackScheduled() {
@@ -150,9 +150,7 @@ void FunctionLooper::detachEventBase() {
   VLOG(10) << __func__ << ": " << type_;
   DCHECK(evb_ && evb_->isInEventBaseThread());
   stop();
-  if (pacingTimer_) {
-    cancelTimerCallback();
-  }
+  cancelTimerCallback();
   evb_ = nullptr;
 }
 

diff --git a/quic/common/FunctionLooper.h b/quic/common/FunctionLooper.h
@@ -43,8 +43,6 @@ class FunctionLooper : public QuicEventBaseLoopCallback,
 
   void setPacingTimer(QuicTimer::SharedPtr pacingTimer) noexcept;
 
-  bool hasPacingTimer() const noexcept;
-
   void runLoopCallback() noexcept override;
 
   /**