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) {
