gfe-relnote: Change the default value of --quic_lumpy_pacing_size to 2. Protected by --quic_change_default_lumpy_pacing_size_to_two. PiperOrigin-RevId: 256245931 Change-Id: I8f5f1f11bb0ba98d7d9754a2664b51751e948f22
diff --git a/quic/core/congestion_control/bbr_sender_test.cc b/quic/core/congestion_control/bbr_sender_test.cc index d071ee8..d175bdf 100644 --- a/quic/core/congestion_control/bbr_sender_test.cc +++ b/quic/core/congestion_control/bbr_sender_test.cc
@@ -417,7 +417,8 @@ // Transfer 12MB. DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); - EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode); + EXPECT_TRUE(sender_->ExportDebugState().mode == BbrSender::PROBE_BW || + sender_->ExportDebugState().mode == BbrSender::PROBE_RTT); // It's possible to read a bandwidth as much as 50% too high with aggregation. EXPECT_LE(kTestLinkBandwidth * 0.99f, sender_->ExportDebugState().max_bandwidth); @@ -430,7 +431,7 @@ // The margin here is high, because the aggregation greatly increases // smoothed rtt. EXPECT_GE(kTestRtt * 4, rtt_stats_->smoothed_rtt()); - EXPECT_APPROX_EQ(kTestRtt, rtt_stats_->min_rtt(), 0.12f); + EXPECT_APPROX_EQ(kTestRtt, rtt_stats_->min_rtt(), 0.25f); } // Test a simple long data transfer with 2 rtts of aggregation. @@ -444,7 +445,8 @@ // Transfer 12MB. DoSimpleTransfer(12 * 1024 * 1024, QuicTime::Delta::FromSeconds(35)); - EXPECT_EQ(BbrSender::PROBE_BW, sender_->ExportDebugState().mode); + EXPECT_TRUE(sender_->ExportDebugState().mode == BbrSender::PROBE_BW || + sender_->ExportDebugState().mode == BbrSender::PROBE_RTT); // It's possible to read a bandwidth as much as 50% too high with aggregation. EXPECT_LE(kTestLinkBandwidth * 0.99f, sender_->ExportDebugState().max_bandwidth); @@ -457,7 +459,7 @@ // The margin here is high, because the aggregation greatly increases // smoothed rtt. EXPECT_GE(kTestRtt * 4, rtt_stats_->smoothed_rtt()); - EXPECT_APPROX_EQ(kTestRtt, rtt_stats_->min_rtt(), 0.12f); + EXPECT_APPROX_EQ(kTestRtt, rtt_stats_->min_rtt(), 0.25f); } // Test the number of losses incurred by the startup phase in a situation when
diff --git a/quic/core/congestion_control/pacing_sender.cc b/quic/core/congestion_control/pacing_sender.cc index 53cdb1c..c58e368 100644 --- a/quic/core/congestion_control/pacing_sender.cc +++ b/quic/core/congestion_control/pacing_sender.cc
@@ -26,10 +26,18 @@ initial_burst_size_(kInitialUnpacedBurst), lumpy_tokens_(0), alarm_granularity_(QuicTime::Delta::FromMilliseconds(1)), - pacing_limited_(false) { + pacing_limited_(false), + lumpy_pacing_size_((GetQuicReloadableFlag( + quic_change_default_lumpy_pacing_size_to_two) && + GetQuicFlag(FLAGS_quic_lumpy_pacing_size) == 1) + ? 2 + : GetQuicFlag(FLAGS_quic_lumpy_pacing_size)) { if (GetQuicReloadableFlag(quic_donot_reset_ideal_next_packet_send_time)) { QUIC_RELOADABLE_FLAG_COUNT(quic_donot_reset_ideal_next_packet_send_time); } + if (GetQuicReloadableFlag(quic_change_default_lumpy_pacing_size_to_two)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_change_default_lumpy_pacing_size_to_two); + } } PacingSender::~PacingSender() {} @@ -90,8 +98,7 @@ // Reset lumpy_tokens_ if either application or cwnd throttles sending or // token runs out. lumpy_tokens_ = std::max( - 1u, std::min(static_cast<uint32_t>( - GetQuicFlag(FLAGS_quic_lumpy_pacing_size)), + 1u, std::min(static_cast<uint32_t>(lumpy_pacing_size_), static_cast<uint32_t>( (sender_->GetCongestionWindow() * GetQuicFlag(FLAGS_quic_lumpy_pacing_cwnd_fraction)) / @@ -139,6 +146,9 @@ if (burst_tokens_ > 0 || bytes_in_flight == 0 || lumpy_tokens_ > 0) { // Don't pace if we have burst tokens available or leaving quiescence. + QUIC_DVLOG(1) << "Sending packet now. burst_tokens:" << burst_tokens_ + << ", bytes_in_flight:" << bytes_in_flight + << ", lumpy_tokens:" << lumpy_tokens_; return QuicTime::Delta::Zero(); }
diff --git a/quic/core/congestion_control/pacing_sender.h b/quic/core/congestion_control/pacing_sender.h index 781a921..f53f17a 100644 --- a/quic/core/congestion_control/pacing_sender.h +++ b/quic/core/congestion_control/pacing_sender.h
@@ -103,6 +103,9 @@ // Indicates whether pacing throttles the sending. If true, make up for lost // time. bool pacing_limited_; + + // Latched value of --quic_lumpy_pacing_size. + const int32_t lumpy_pacing_size_; }; } // namespace quic
diff --git a/quic/core/congestion_control/pacing_sender_test.cc b/quic/core/congestion_control/pacing_sender_test.cc index f6cf63b..066a3b5 100644 --- a/quic/core/congestion_control/pacing_sender_test.cc +++ b/quic/core/congestion_control/pacing_sender_test.cc
@@ -280,8 +280,9 @@ } TEST_F(PacingSenderTest, FastSending) { - // Ensure the pacing sender paces, even when the inter-packet spacing is less - // than the pacing granularity. + SetQuicReloadableFlag(quic_change_default_lumpy_pacing_size_to_two, true); + // Ensure the pacing sender paces, even when the inter-packet spacing(0.5ms) + // is less than the pacing granularity(1ms). InitPacingRate(10, QuicBandwidth::FromBytesAndTimeDelta( 2 * kMaxOutgoingPacketSize, QuicTime::Delta::FromMilliseconds(1))); @@ -293,12 +294,11 @@ CheckPacketIsSentImmediately(); } - // The first packet was a "make up", then we sent two packets "into the - // future", since it's 2 packets/ms, so the delay should be 1.5ms. - CheckPacketIsSentImmediately(); - CheckPacketIsSentImmediately(); - CheckPacketIsSentImmediately(); - CheckPacketIsDelayed(QuicTime::Delta::FromMicroseconds(1500)); + CheckPacketIsSentImmediately(); // Make up + CheckPacketIsSentImmediately(); // Lumpy token + CheckPacketIsSentImmediately(); // "In the future" but within granularity. + CheckPacketIsSentImmediately(); // Lumpy token + CheckPacketIsDelayed(QuicTime::Delta::FromMicroseconds(2000)); clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); CheckPacketIsSentImmediately(); @@ -312,10 +312,11 @@ // The first packet was a "make up", then we sent two packets "into the // future", so the delay should be 1.5ms. - CheckPacketIsSentImmediately(); - CheckPacketIsSentImmediately(); - CheckPacketIsSentImmediately(); - CheckPacketIsDelayed(QuicTime::Delta::FromMicroseconds(1500)); + CheckPacketIsSentImmediately(); // Make up + CheckPacketIsSentImmediately(); // Lumpy token + CheckPacketIsSentImmediately(); // "In the future" but within granularity. + CheckPacketIsSentImmediately(); // Lumpy token + CheckPacketIsDelayed(QuicTime::Delta::FromMicroseconds(2000)); } TEST_F(PacingSenderTest, NoBurstEnteringRecovery) {