diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index b6f71498d..42efb3bda 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -377,11 +377,14 @@ constexpr auto kPacketToSendForPTO = 2; constexpr uint64_t kDefaultWriteConnectionDataPacketLimit = 5; // Minimum number of packets to write per burst in pacing constexpr uint64_t kDefaultMinBurstPackets = 5; -// Default timer tick interval for pacing timer -// the microsecond timers are accurate to about 5 usec -// but the notifications can get delayed if the event loop is busy -// this is subject to testing but I would suggest a value >= 200usec -constexpr std::chrono::microseconds kDefaultPacingTimerTickInterval{1000}; + +// Default tick interval for pacing timer. This is the smallest interval the +// pacer will use as its interval. +constexpr std::chrono::microseconds kDefaultPacingTickInterval{1000}; +// Default pacing timer resolution. This is the size of the buckets in the timer +// triggering the pacing callbacks. For pacing to work accurately, this should +// be reasonably smaller than kDefaultPacingTickInterval. +constexpr std::chrono::microseconds kDefaultPacingTimerResolution{100}; // Fraction of RTT that is used to limit how long a write function can loop constexpr DurationRep kDefaultWriteLimitRttFraction = 25; diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 049f5370a..ffc119b7e 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -3290,8 +3290,10 @@ void QuicTransportBase::updateCongestionControlSettings( conn_->transportSettings.limitedCwndInMss = transportSettings.limitedCwndInMss; conn_->transportSettings.pacingEnabled = transportSettings.pacingEnabled; - conn_->transportSettings.pacingTimerTickInterval = - transportSettings.pacingTimerTickInterval; + conn_->transportSettings.pacingTickInterval = + transportSettings.pacingTickInterval; + conn_->transportSettings.pacingTimerResolution = + transportSettings.pacingTimerResolution; conn_->transportSettings.minBurstPackets = transportSettings.minBurstPackets; conn_->transportSettings.copaDeltaParam = transportSettings.copaDeltaParam; conn_->transportSettings.copaUseRttStanding = diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 8a907b73e..6456fdfbb 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -3992,7 +3992,7 @@ TEST_F(QuicTransportTest, SetPacingTimerThenEnablesPacing) { TransportSettings transportSettings; transportSettings.pacingEnabled = true; transport_->setPacingTimer( - TimerHighRes::newTimer(&evb_, transportSettings.pacingTimerTickInterval)); + TimerHighRes::newTimer(&evb_, transportSettings.pacingTimerResolution)); transport_->setTransportSettings(transportSettings); transport_->getConnectionState().canBePaced = true; EXPECT_TRUE(isConnectionPaced(transport_->getConnectionState())); @@ -4881,7 +4881,7 @@ TEST_F(QuicTransportTest, SetMaxPacingRateWithAndWithoutPacing) { EXPECT_EQ(LocalErrorCode::PACER_NOT_AVAILABLE, res1.error()); settings.pacingEnabled = true; transport_->setPacingTimer( - TimerHighRes::newTimer(&evb_, settings.pacingTimerTickInterval)); + TimerHighRes::newTimer(&evb_, settings.pacingTimerResolution)); transport_->setTransportSettings(settings); auto res2 = transport_->setMaxPacingRate(125000); EXPECT_FALSE(res2.hasError()); diff --git a/quic/congestion_control/CongestionControlFunctions.cpp b/quic/congestion_control/CongestionControlFunctions.cpp index 0c9462ace..24ad67990 100644 --- a/quic/congestion_control/CongestionControlFunctions.cpp +++ b/quic/congestion_control/CongestionControlFunctions.cpp @@ -28,7 +28,7 @@ PacingRate calculatePacingRate( uint64_t cwnd, uint64_t minCwndInMss, std::chrono::microseconds rtt) { - if (conn.transportSettings.pacingTimerTickInterval > rtt) { + if (conn.transportSettings.pacingTickInterval > rtt) { // We cannot really pace in this case. return PacingRate::Builder() .setInterval(0us) @@ -43,10 +43,10 @@ PacingRate calculatePacingRate( static_cast(std::ceil( static_cast(cwndInPackets) * static_cast( - conn.transportSettings.pacingTimerTickInterval.count()) / + conn.transportSettings.pacingTickInterval.count()) / static_cast(rtt.count())))); auto interval = timeMax( - conn.transportSettings.pacingTimerTickInterval, + conn.transportSettings.pacingTickInterval, rtt * burstPerInterval / cwndInPackets); return PacingRate::Builder() .setInterval(interval) diff --git a/quic/congestion_control/TokenlessPacer.cpp b/quic/congestion_control/TokenlessPacer.cpp index 43b080174..0483afbfe 100644 --- a/quic/congestion_control/TokenlessPacer.cpp +++ b/quic/congestion_control/TokenlessPacer.cpp @@ -31,7 +31,7 @@ void TokenlessPacer::refreshPacingRate( : cwndBytes * 1s / rtt; if (targetRateBytesPerSec > maxPacingRateBytesPerSec_) { return setPacingRate(maxPacingRateBytesPerSec_); - } else if (rtt < conn_.transportSettings.pacingTimerTickInterval) { + } else if (rtt < conn_.transportSettings.pacingTickInterval) { writeInterval_ = 0us; batchSize_ = conn_.transportSettings.writeConnectionDataPacketsLimit; } else { @@ -58,14 +58,14 @@ void TokenlessPacer::setPacingRate(uint64_t rateBps) { if (rateBps == 0) { batchSize_ = 0; - writeInterval_ = conn_.transportSettings.pacingTimerTickInterval; + writeInterval_ = conn_.transportSettings.pacingTickInterval; } else { batchSize_ = conn_.transportSettings.writeConnectionDataPacketsLimit; uint64_t interval = (batchSize_ * conn_.udpSendPacketLen * 1000000) / rateBps; writeInterval_ = std::max( std::chrono::microseconds(interval), - conn_.transportSettings.pacingTimerTickInterval); + conn_.transportSettings.pacingTickInterval); } if (conn_.qLogger) { @@ -116,7 +116,7 @@ std::chrono::microseconds TokenlessPacer::getTimeUntilNextWrite( } return std::max( writeInterval_ - timeSinceLastWrite, - conn_.transportSettings.pacingTimerTickInterval); + conn_.transportSettings.pacingTickInterval); } uint64_t TokenlessPacer::updateAndGetWriteBatchSize(TimePoint currentTime) { diff --git a/quic/congestion_control/test/CongestionControlFunctionsTest.cpp b/quic/congestion_control/test/CongestionControlFunctionsTest.cpp index 293805cfa..98804933c 100644 --- a/quic/congestion_control/test/CongestionControlFunctionsTest.cpp +++ b/quic/congestion_control/test/CongestionControlFunctionsTest.cpp @@ -22,14 +22,14 @@ TEST_F(CongestionControlFunctionsTest, CalculatePacingRate) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; conn.transportSettings.minBurstPackets = 1; - conn.transportSettings.pacingTimerTickInterval = 10ms; + conn.transportSettings.pacingTickInterval = 10ms; std::chrono::microseconds rtt(1000 * 100); auto result = calculatePacingRate(conn, 50, conn.transportSettings.minCwndInMss, rtt); EXPECT_EQ(10ms, result.interval); EXPECT_EQ(5, result.burstSize); - conn.transportSettings.pacingTimerTickInterval = 1ms; + conn.transportSettings.pacingTickInterval = 1ms; auto result2 = calculatePacingRate(conn, 300, conn.transportSettings.minCwndInMss, rtt); EXPECT_EQ(1ms, result2.interval); @@ -39,7 +39,7 @@ TEST_F(CongestionControlFunctionsTest, CalculatePacingRate) { TEST_F(CongestionControlFunctionsTest, MinPacingRate) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; - conn.transportSettings.pacingTimerTickInterval = 1ms; + conn.transportSettings.pacingTickInterval = 1ms; auto result = calculatePacingRate( conn, 100, conn.transportSettings.minCwndInMss, 100ms); // 100 ms rtt, 1ms tick interval, 100 mss cwnd, 5 mss min burst -> 5 mss every @@ -52,7 +52,7 @@ TEST_F(CongestionControlFunctionsTest, SmallCwnd) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; conn.transportSettings.minBurstPackets = 1; - conn.transportSettings.pacingTimerTickInterval = 1ms; + conn.transportSettings.pacingTickInterval = 1ms; auto result = calculatePacingRate( conn, 10, conn.transportSettings.minCwndInMss, 100000us); EXPECT_EQ(10ms, result.interval); @@ -63,7 +63,7 @@ TEST_F(CongestionControlFunctionsTest, RttSmallerThanInterval) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; conn.transportSettings.minBurstPackets = 1; - conn.transportSettings.pacingTimerTickInterval = 10ms; + conn.transportSettings.pacingTickInterval = 10ms; auto result = calculatePacingRate(conn, 10, conn.transportSettings.minCwndInMss, 1ms); EXPECT_EQ(std::chrono::milliseconds::zero(), result.interval); diff --git a/quic/congestion_control/test/CopaTest.cpp b/quic/congestion_control/test/CopaTest.cpp index c2c93ccb2..78b7e11d7 100644 --- a/quic/congestion_control/test/CopaTest.cpp +++ b/quic/congestion_control/test/CopaTest.cpp @@ -435,7 +435,7 @@ TEST_F(CopaTest, TestVelocity) { FizzServerQuicHandshakeContext::Builder().build()); conn.transportSettings.copaDeltaParam = 0.5; conn.transportSettings.copaUseRttStanding = true; - conn.transportSettings.pacingTimerTickInterval = 10ms; + conn.transportSettings.pacingTickInterval = 10ms; conn.transportSettings.initCwndInMss = 11; Copa copa(conn); auto qLogger = std::make_shared(VantagePoint::Client); diff --git a/quic/congestion_control/test/CubicTest.cpp b/quic/congestion_control/test/CubicTest.cpp index 36a5b18fe..0b647b644 100644 --- a/quic/congestion_control/test/CubicTest.cpp +++ b/quic/congestion_control/test/CubicTest.cpp @@ -244,7 +244,7 @@ TEST_F(CubicTest, AppIdle) { TEST_F(CubicTest, PacingGain) { QuicConnectionStateBase conn(QuicNodeType::Client); - conn.transportSettings.pacingTimerTickInterval = 1ms; + conn.transportSettings.pacingTickInterval = 1ms; auto mockPacer = std::make_unique(); auto rawPacer = mockPacer.get(); conn.pacer = std::move(mockPacer); diff --git a/quic/congestion_control/test/PacerTest.cpp b/quic/congestion_control/test/PacerTest.cpp index 5dd64d69a..1163ba4cb 100644 --- a/quic/congestion_control/test/PacerTest.cpp +++ b/quic/congestion_control/test/PacerTest.cpp @@ -20,7 +20,7 @@ namespace test { class TokenlessPacerTest : public Test { public: void SetUp() override { - conn.transportSettings.pacingTimerTickInterval = 1us; + conn.transportSettings.pacingTickInterval = 1us; } protected: @@ -122,7 +122,7 @@ TEST_F(TokenlessPacerTest, RttFactor) { } TEST_F(TokenlessPacerTest, ImpossibleToPace) { - conn.transportSettings.pacingTimerTickInterval = 1ms; + conn.transportSettings.pacingTickInterval = 1ms; pacer.setPacingRateCalculator([](const QuicConnectionStateBase& conn, uint64_t cwndBytes, uint64_t, @@ -230,7 +230,7 @@ TEST_F(TokenlessPacerTest, SetMaxPacingRateOnUnlimitedPacer) { TEST_F(TokenlessPacerTest, SetZeroPacingRate) { auto timestamp = Clock::now(); // A Zero pacing rate should not result in a divide-by-zero - conn.transportSettings.pacingTimerTickInterval = 1000us; + conn.transportSettings.pacingTickInterval = 1000us; pacer.setPacingRate(0); EXPECT_EQ(0, pacer.updateAndGetWriteBatchSize(timestamp)); EXPECT_EQ(1000, pacer.getTimeUntilNextWrite(timestamp).count()); @@ -239,7 +239,7 @@ TEST_F(TokenlessPacerTest, SetZeroPacingRate) { TEST_F(TokenlessPacerTest, RefreshPacingRateWhenRTTIsZero) { auto timestamp = Clock::now(); // rtt=0 should not result in a divide-by-zero - conn.transportSettings.pacingTimerTickInterval = 1000us; + conn.transportSettings.pacingTickInterval = 1000us; pacer.refreshPacingRate(100, 0us); // Verify burst is writeConnectionDataPacketsLimit and interval is // 0us right after writing @@ -258,7 +258,7 @@ TEST_F(TokenlessPacerTest, RefreshPacingRateWhenRTTIsDefault) { std::chrono::microseconds rtt) { return PacingRate::Builder().setInterval(rtt).setBurstSize(cwnd).build(); }); - conn.transportSettings.pacingTimerTickInterval = tick; + conn.transportSettings.pacingTickInterval = tick; // rtt=kDefaultMinRTT should result in this update being skipped // There should be no pacing. Interval and Burst should use the defaults diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index d8552759b..e1888776f 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -1034,7 +1034,7 @@ void QuicServerTransport::registerAllTransportKnobParamHandlers() { CHECK(serverTransport); auto val = std::get(value); auto serverConn = serverTransport->serverConn_; - serverConn->transportSettings.pacingTimerTickInterval = + serverConn->transportSettings.pacingTickInterval = std::chrono::microseconds(val); VLOG(3) << "PACING_TIMER_TICK KnobParam received: " << val; }); diff --git a/quic/server/QuicServerWorker.cpp b/quic/server/QuicServerWorker.cpp index de3b96f03..6c97e0e7c 100644 --- a/quic/server/QuicServerWorker.cpp +++ b/quic/server/QuicServerWorker.cpp @@ -176,7 +176,7 @@ void QuicServerWorker::start() { CHECK(socket_); if (!pacingTimer_) { pacingTimer_ = TimerHighRes::newTimer( - evb_.get(), transportSettings_.pacingTimerTickInterval); + evb_.get(), transportSettings_.pacingTimerResolution); } socket_->resumeRead(this); VLOG(10) << fmt::format( diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index 254d3c596..0f7fd498b 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -124,9 +124,13 @@ struct TransportSettings { bool pacingEnabledFirstFlight{false}; // The minimum number of packets to burst out during pacing uint64_t minBurstPackets{kDefaultMinBurstPackets}; - // Pacing timer tick interval - std::chrono::microseconds pacingTimerTickInterval{ - kDefaultPacingTimerTickInterval}; + // This is the smallest interval the pacer will use as its interval. + std::chrono::microseconds pacingTickInterval{kDefaultPacingTickInterval}; + // This is the size of the buckets in the timer triggering the pacing + // callbacks. For pacing to work accurately, this should be reasonably smaller + // than kDefaultPacingTickInterval. + std::chrono::microseconds pacingTimerResolution{ + kDefaultPacingTimerResolution}; ZeroRttSourceTokenMatchingPolicy zeroRttSourceTokenMatchingPolicy{ ZeroRttSourceTokenMatchingPolicy::REJECT_IF_NO_EXACT_MATCH}; // Scale pacing rate for CC, non-empty indicates override via transport knobs diff --git a/quic/tools/tperf/tperf.cpp b/quic/tools/tperf/tperf.cpp index ce5c2b270..16618a4e2 100644 --- a/quic/tools/tperf/tperf.cpp +++ b/quic/tools/tperf/tperf.cpp @@ -460,7 +460,7 @@ class TPerfServer { settings.defaultCongestionController = congestionControlType; settings.pacingEnabled = pacing; if (pacing) { - settings.pacingTimerTickInterval = 200us; + settings.pacingTickInterval = 200us; settings.writeLimitRttFraction = 0; } if (gso) { @@ -672,7 +672,7 @@ class TPerfClient : public quic::QuicSocket::ConnectionSetupCallback, if (congestionControlType_ == quic::CongestionControlType::BBR || congestionControlType_ == CongestionControlType::BBRTesting) { settings.pacingEnabled = true; - settings.pacingTimerTickInterval = 200us; + settings.pacingTickInterval = 200us; settings.writeLimitRttFraction = 0; } if (gso_) {