From eb10f2e36dee5eca29cb2a06d5fcddf99b4dc49c Mon Sep 17 00:00:00 2001 From: Yang Chi Date: Mon, 19 Aug 2019 09:11:23 -0700 Subject: [PATCH] refactor calculatePacingRate function Summary: No need to pass the minimal interval any more since everywhere we just pass the transportSettings value. Just that inside the function directly Reviewed By: mjoras Differential Revision: D16773023 fbshipit-source-id: 22ada4f25d565e97e7fce27371a0e2240bbfe8c0 --- quic/congestion_control/Bbr.cpp | 8 ++----- .../CongestionControlFunctions.cpp | 11 +++++----- .../CongestionControlFunctions.h | 2 -- quic/congestion_control/Copa.cpp | 1 - quic/congestion_control/QuicCubic.cpp | 1 - .../test/CongestionControlFunctionsTest.cpp | 21 ++++++++++++------- 6 files changed, 21 insertions(+), 23 deletions(-) diff --git a/quic/congestion_control/Bbr.cpp b/quic/congestion_control/Bbr.cpp index 16682282d..1b8632e38 100644 --- a/quic/congestion_control/Bbr.cpp +++ b/quic/congestion_control/Bbr.cpp @@ -273,12 +273,8 @@ void BbrCongestionController::updatePacing() noexcept { pacingWindow_ = std::max(pacingWindow_, targetPacingWindow); } // TODO: slower pacing if we are in STARTUP and loss has happened - std::tie(pacingInterval_, pacingBurstSize_) = calculatePacingRate( - conn_, - pacingWindow_, - kMinCwndInMssForBbr, - conn_.transportSettings.pacingTimerTickInterval, - mrtt); + std::tie(pacingInterval_, pacingBurstSize_) = + calculatePacingRate(conn_, pacingWindow_, kMinCwndInMssForBbr, mrtt); if (conn_.transportSettings.pacingEnabled && conn_.qLogger) { conn_.qLogger->addPacingMetricUpdate(pacingBurstSize_, pacingInterval_); diff --git a/quic/congestion_control/CongestionControlFunctions.cpp b/quic/congestion_control/CongestionControlFunctions.cpp index b868c03ea..8cb202ffc 100644 --- a/quic/congestion_control/CongestionControlFunctions.cpp +++ b/quic/congestion_control/CongestionControlFunctions.cpp @@ -28,9 +28,8 @@ std::pair calculatePacingRate( const QuicConnectionStateBase& conn, uint64_t cwnd, uint64_t minCwndInMss, - std::chrono::microseconds minimalInterval, std::chrono::microseconds rtt) { - if (minimalInterval > rtt) { + if (conn.transportSettings.pacingTimerTickInterval > rtt) { // We cannot really pace in this case. return std::make_pair( 0us, conn.transportSettings.writeConnectionDataPacketsLimit); @@ -42,10 +41,12 @@ std::pair calculatePacingRate( conn.transportSettings.maxBurstPackets, static_cast(std::ceil( static_cast(cwndInPackets) * - static_cast(minimalInterval.count()) / + static_cast( + conn.transportSettings.pacingTimerTickInterval.count()) / static_cast(rtt.count())))); - auto interval = - timeMax(minimalInterval, rtt * burstPerInterval / cwndInPackets); + auto interval = timeMax( + conn.transportSettings.pacingTimerTickInterval, + rtt * burstPerInterval / cwndInPackets); return std::make_pair(interval, burstPerInterval); } } // namespace quic diff --git a/quic/congestion_control/CongestionControlFunctions.h b/quic/congestion_control/CongestionControlFunctions.h index 63688e138..909986979 100644 --- a/quic/congestion_control/CongestionControlFunctions.h +++ b/quic/congestion_control/CongestionControlFunctions.h @@ -21,12 +21,10 @@ uint64_t boundedCwnd( uint64_t maxCwndInMss, uint64_t minCwndInMss) noexcept; -// TODO: remove minimalInterval, use transportSettings from conn std::pair calculatePacingRate( const QuicConnectionStateBase& conn, uint64_t cwnd, uint64_t minCwndInMss, - std::chrono::microseconds minimalInterval, std::chrono::microseconds rtt); template diff --git a/quic/congestion_control/Copa.cpp b/quic/congestion_control/Copa.cpp index 0c48de325..15baabe16 100644 --- a/quic/congestion_control/Copa.cpp +++ b/quic/congestion_control/Copa.cpp @@ -316,7 +316,6 @@ void Copa::updatePacing() noexcept { conn_, cwndBytes_ * 2, conn_.transportSettings.minCwndInMss, - conn_.transportSettings.pacingTimerTickInterval, conn_.lossState.srtt); if (pacingInterval_ == std::chrono::milliseconds::zero()) { return; diff --git a/quic/congestion_control/QuicCubic.cpp b/quic/congestion_control/QuicCubic.cpp index 848adb97c..706ba2ee8 100644 --- a/quic/congestion_control/QuicCubic.cpp +++ b/quic/congestion_control/QuicCubic.cpp @@ -541,7 +541,6 @@ void Cubic::updatePacing() noexcept { conn_, cwndBytes_ * pacingGain(), conn_.transportSettings.minCwndInMss, - conn_.transportSettings.pacingTimerTickInterval, conn_.lossState.srtt); if (pacingInterval_ == std::chrono::milliseconds::zero()) { return; diff --git a/quic/congestion_control/test/CongestionControlFunctionsTest.cpp b/quic/congestion_control/test/CongestionControlFunctionsTest.cpp index 0407ac4fb..104fab89d 100644 --- a/quic/congestion_control/test/CongestionControlFunctionsTest.cpp +++ b/quic/congestion_control/test/CongestionControlFunctionsTest.cpp @@ -23,14 +23,16 @@ class CongestionControlFunctionsTest : public Test {}; TEST_F(CongestionControlFunctionsTest, CalculatePacingRate) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; + conn.transportSettings.pacingTimerTickInterval = 10ms; std::chrono::microseconds rtt(1000 * 100); - auto result = calculatePacingRate( - conn, 50, conn.transportSettings.minCwndInMss, 10ms, rtt); + auto result = + calculatePacingRate(conn, 50, conn.transportSettings.minCwndInMss, rtt); EXPECT_EQ(10ms, result.first); EXPECT_EQ(5, result.second); - auto result2 = calculatePacingRate( - conn, 300, conn.transportSettings.minCwndInMss, 1ms, rtt); + conn.transportSettings.pacingTimerTickInterval = 1ms; + auto result2 = + calculatePacingRate(conn, 300, conn.transportSettings.minCwndInMss, rtt); EXPECT_EQ(1ms, result2.first); EXPECT_EQ(3, result2.second); } @@ -38,8 +40,9 @@ TEST_F(CongestionControlFunctionsTest, CalculatePacingRate) { TEST_F(CongestionControlFunctionsTest, MinPacingRate) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; + conn.transportSettings.pacingTimerTickInterval = 1ms; auto result = calculatePacingRate( - conn, 100, conn.transportSettings.minCwndInMss, 1ms, 100000us); + conn, 100, conn.transportSettings.minCwndInMss, 100000us); EXPECT_EQ(1ms, result.first); EXPECT_EQ(1, result.second); } @@ -47,8 +50,9 @@ TEST_F(CongestionControlFunctionsTest, MinPacingRate) { TEST_F(CongestionControlFunctionsTest, SmallCwnd) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; + conn.transportSettings.pacingTimerTickInterval = 1ms; auto result = calculatePacingRate( - conn, 10, conn.transportSettings.minCwndInMss, 1ms, 100000us); + conn, 10, conn.transportSettings.minCwndInMss, 100000us); EXPECT_EQ(10ms, result.first); EXPECT_EQ(1, result.second); } @@ -56,8 +60,9 @@ TEST_F(CongestionControlFunctionsTest, SmallCwnd) { TEST_F(CongestionControlFunctionsTest, RttSmallerThanInterval) { QuicConnectionStateBase conn(QuicNodeType::Client); conn.udpSendPacketLen = 1; - auto result = calculatePacingRate( - conn, 10, conn.transportSettings.minCwndInMss, 10ms, 1ms); + conn.transportSettings.pacingTimerTickInterval = 10ms; + auto result = + calculatePacingRate(conn, 10, conn.transportSettings.minCwndInMss, 1ms); EXPECT_EQ(std::chrono::milliseconds::zero(), result.first); EXPECT_EQ( conn.transportSettings.writeConnectionDataPacketsLimit, result.second);