Remove non-initial burst from QUIC PacingSender. Protected by FLAGS_quic_reloadable_flag_quic_pacing_remove_non_initial_burst. PiperOrigin-RevId: 540276891
diff --git a/quiche/quic/core/congestion_control/bbr2_simulator_test.cc b/quiche/quic/core/congestion_control/bbr2_simulator_test.cc index d65adfe..7804df4 100644 --- a/quiche/quic/core/congestion_control/bbr2_simulator_test.cc +++ b/quiche/quic/core/congestion_control/bbr2_simulator_test.cc
@@ -1593,6 +1593,12 @@ sender_->ExportDebugState().bandwidth_hi, 0.02f); } + if (GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst)) { + QuicSentPacketManagerPeer::GetPacingSender( + &sender_connection()->sent_packet_manager()) + ->SetBurstTokens(10); + } + // Now that in-flight is almost zero and the pacing gain is still above 1, // send approximately 1.4 BDPs worth of data. This should cause the PROBE_BW // mode to enter low gain cycle(PROBE_DOWN), and exit it earlier than one
diff --git a/quiche/quic/core/congestion_control/pacing_sender.cc b/quiche/quic/core/congestion_control/pacing_sender.cc index b4b6105..b7ef73c 100644 --- a/quiche/quic/core/congestion_control/pacing_sender.cc +++ b/quiche/quic/core/congestion_control/pacing_sender.cc
@@ -56,26 +56,37 @@ QuicPacketNumber packet_number, QuicByteCount bytes, HasRetransmittableData has_retransmittable_data) { QUICHE_DCHECK(sender_ != nullptr); + QUIC_DVLOG(3) << "Packet " << packet_number << " with " << bytes + << " bytes sent at " << sent_time + << ". bytes_in_flight: " << bytes_in_flight; sender_->OnPacketSent(sent_time, bytes_in_flight, packet_number, bytes, has_retransmittable_data); if (has_retransmittable_data != HAS_RETRANSMITTABLE_DATA) { return; } - // If in recovery, the connection is not coming out of quiescence. - if (bytes_in_flight == 0 && !sender_->InRecovery()) { - // Add more burst tokens anytime the connection is leaving quiescence, but - // limit it to the equivalent of a single bulk write, not exceeding the - // current CWND in packets. - burst_tokens_ = std::min( - initial_burst_size_, - static_cast<uint32_t>(sender_->GetCongestionWindow() / kDefaultTCPMSS)); + + if (remove_non_initial_burst_) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_pacing_remove_non_initial_burst, 1, 2); + } else { + // If in recovery, the connection is not coming out of quiescence. + if (bytes_in_flight == 0 && !sender_->InRecovery()) { + // Add more burst tokens anytime the connection is leaving quiescence, but + // limit it to the equivalent of a single bulk write, not exceeding the + // current CWND in packets. + burst_tokens_ = + std::min(initial_burst_size_, + static_cast<uint32_t>(sender_->GetCongestionWindow() / + kDefaultTCPMSS)); + } } + if (burst_tokens_ > 0) { --burst_tokens_; ideal_next_packet_send_time_ = QuicTime::Zero(); pacing_limited_ = false; return; } + // The next packet should be sent as soon as the current packet has been // transferred. PacingRate is based on bytes in flight including this packet. QuicTime::Delta delay = @@ -134,12 +145,22 @@ return QuicTime::Delta::Infinite(); } - 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(); + if (remove_non_initial_burst_) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_pacing_remove_non_initial_burst, 2, 2); + if (burst_tokens_ > 0 || lumpy_tokens_ > 0) { + // Don't pace if we have burst or lumpy tokens available. + QUIC_DVLOG(1) << "Can send packet now. burst_tokens:" << burst_tokens_ + << ", lumpy_tokens:" << lumpy_tokens_; + return QuicTime::Delta::Zero(); + } + } else { + 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(); + } } // If the next send time is within the alarm granularity, send immediately. @@ -149,7 +170,7 @@ return ideal_next_packet_send_time_ - now; } - QUIC_DVLOG(1) << "Sending packet now. ideal_next_packet_send_time: " + QUIC_DVLOG(1) << "Can send packet now. ideal_next_packet_send_time: " << ideal_next_packet_send_time_ << ", now: " << now; return QuicTime::Delta::Zero(); }
diff --git a/quiche/quic/core/congestion_control/pacing_sender.h b/quiche/quic/core/congestion_control/pacing_sender.h index 0e3de00..73c08d4 100644 --- a/quiche/quic/core/congestion_control/pacing_sender.h +++ b/quiche/quic/core/congestion_control/pacing_sender.h
@@ -107,6 +107,9 @@ // Indicates whether pacing throttles the sending. If true, make up for lost // time. bool pacing_limited_; + + const bool remove_non_initial_burst_ = + GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst); }; } // namespace quic
diff --git a/quiche/quic/core/congestion_control/pacing_sender_test.cc b/quiche/quic/core/congestion_control/pacing_sender_test.cc index 108b12e..ad82b9b 100644 --- a/quiche/quic/core/congestion_control/pacing_sender_test.cc +++ b/quiche/quic/core/congestion_control/pacing_sender_test.cc
@@ -84,11 +84,13 @@ .WillOnce(Return(true)); // Verify that the packet can be sent immediately. EXPECT_EQ(zero_time_, - pacing_sender_->TimeUntilSend(clock_.Now(), prior_in_flight)); + pacing_sender_->TimeUntilSend(clock_.Now(), prior_in_flight)) + << "Next packet to send is " << packet_number_; } // Actually send the packet. - if (prior_in_flight == 0) { + if (prior_in_flight == 0 && + !GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst)) { EXPECT_CALL(*mock_sender_, InRecovery()).WillOnce(Return(in_recovery)); } EXPECT_CALL(*mock_sender_, @@ -236,8 +238,17 @@ CheckPacketIsDelayed(QuicTime::Delta::FromMilliseconds(2)); clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); - CheckPacketIsSentImmediately(); + if (GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst)) { + // Can send some packets immediately to make up for 5ms of lost time. + for (int i = 0; i < 6; ++i) { + CheckPacketIsSentImmediately(); + } + CheckPacketIsDelayed(QuicTime::Delta::FromMilliseconds(3)); + return; + } + + CheckPacketIsSentImmediately(); // Next time TimeUntilSend is called with no bytes in flight, pacing should // allow a packet to be sent, and when it's sent, the tokens are refilled. CheckPacketIsSentImmediately(HAS_RETRANSMITTABLE_DATA, 0, false, 10); @@ -270,6 +281,16 @@ CheckPacketIsDelayed(QuicTime::Delta::FromMilliseconds(2)); clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); + + if (GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst)) { + // Can send some packets immediately to make up for 5ms of lost time. + for (int i = 0; i < 6; ++i) { + CheckPacketIsSentImmediately(); + } + CheckPacketIsDelayed(QuicTime::Delta::FromMilliseconds(3)); + return; + } + CheckPacketIsSentImmediately(); // Next time TimeUntilSend is called with no bytes in flight, the tokens @@ -308,6 +329,16 @@ CheckPacketIsDelayed(QuicTime::Delta::FromMicroseconds(2000)); clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5)); + + if (GetQuicReloadableFlag(quic_pacing_remove_non_initial_burst)) { + // Can send some packets immediately to make up for 5ms of lost time. + for (int i = 0; i < 10; ++i) { + CheckPacketIsSentImmediately(); + } + CheckPacketIsDelayed(QuicTime::Delta::FromMilliseconds(2)); + return; + } + CheckPacketIsSentImmediately(); // Next time TimeUntilSend is called with no bytes in flight, the tokens
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 65c7b55..7c21d75 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -53,6 +53,8 @@ QUIC_FLAG(quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true) // If true, reject or send error response code upon receiving invalid request or response headers. QUIC_FLAG(quic_reloadable_flag_quic_act_upon_invalid_header, false) +// If true, remove the non-initial burst in QUIC PacingSender. +QUIC_FLAG(quic_reloadable_flag_quic_pacing_remove_non_initial_burst, false) // If true, require handshake confirmation for QUIC connections, functionally disabling 0-rtt handshakes. QUIC_FLAG(quic_reloadable_flag_quic_require_handshake_confirmation, false) // If true, respect the incremental parameter of each stream in QuicWriteBlockedList.
diff --git a/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc b/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc index d6256fe..a5a8cbd 100644 --- a/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc +++ b/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc
@@ -9,6 +9,7 @@ #include "quiche/quic/core/quic_packets.h" #include "quiche/quic/core/quic_sent_packet_manager.h" #include "quiche/quic/test_tools/quic_unacked_packet_map_peer.h" +#include "quiche/common/platform/api/quiche_logging.h" namespace quic { namespace test { @@ -111,6 +112,13 @@ } // static +PacingSender* QuicSentPacketManagerPeer::GetPacingSender( + QuicSentPacketManager* sent_packet_manager) { + QUICHE_DCHECK(UsingPacing(sent_packet_manager)); + return &sent_packet_manager->pacing_sender_; +} + +// static bool QuicSentPacketManagerPeer::HasRetransmittableFrames( QuicSentPacketManager* sent_packet_manager, uint64_t packet_number) { return sent_packet_manager->unacked_packets_.HasRetransmittableFrames(
diff --git a/quiche/quic/test_tools/quic_sent_packet_manager_peer.h b/quiche/quic/test_tools/quic_sent_packet_manager_peer.h index c0ea673..a2b39da 100644 --- a/quiche/quic/test_tools/quic_sent_packet_manager_peer.h +++ b/quiche/quic/test_tools/quic_sent_packet_manager_peer.h
@@ -5,6 +5,7 @@ #ifndef QUICHE_QUIC_TEST_TOOLS_QUIC_SENT_PACKET_MANAGER_PEER_H_ #define QUICHE_QUIC_TEST_TOOLS_QUIC_SENT_PACKET_MANAGER_PEER_H_ +#include "quiche/quic/core/congestion_control/pacing_sender.h" #include "quiche/quic/core/quic_packets.h" #include "quiche/quic/core/quic_sent_packet_manager.h" @@ -58,6 +59,9 @@ static bool UsingPacing(const QuicSentPacketManager* sent_packet_manager); + static PacingSender* GetPacingSender( + QuicSentPacketManager* sent_packet_manager); + static bool HasRetransmittableFrames( QuicSentPacketManager* sent_packet_manager, uint64_t packet_number);