In quicconnection, refactor the code to calculate release time delay into a separate function. no behavior change. PiperOrigin-RevId: 310392260 Change-Id: Ic3b63af5fa3f3bb89c75c312c1296860fa475a3a
diff --git a/quic/core/congestion_control/pacing_sender.h b/quic/core/congestion_control/pacing_sender.h index 781a921..a8436f8 100644 --- a/quic/core/congestion_control/pacing_sender.h +++ b/quic/core/congestion_control/pacing_sender.h
@@ -74,10 +74,14 @@ QuicBandwidth PacingRate(QuicByteCount bytes_in_flight) const; - QuicTime ideal_next_packet_send_time() const { - return ideal_next_packet_send_time_; + NextReleaseTimeResult GetNextReleaseTime() const { + bool allow_burst = (burst_tokens_ > 0 || lumpy_tokens_ > 0); + return {ideal_next_packet_send_time_, allow_burst}; } + protected: + uint32_t lumpy_tokens() const { return lumpy_tokens_; } + private: friend class test::QuicSentPacketManagerPeer;
diff --git a/quic/core/congestion_control/pacing_sender_test.cc b/quic/core/congestion_control/pacing_sender_test.cc index 3ca80cd..8c298d9 100644 --- a/quic/core/congestion_control/pacing_sender_test.cc +++ b/quic/core/congestion_control/pacing_sender_test.cc
@@ -27,6 +27,16 @@ const QuicByteCount kBytesInFlight = 1024; const int kInitialBurstPackets = 10; +class TestPacingSender : public PacingSender { + public: + using PacingSender::lumpy_tokens; + using PacingSender::PacingSender; + + QuicTime ideal_next_packet_send_time() const { + return GetNextReleaseTime().release_time; + } +}; + class PacingSenderTest : public QuicTest { protected: PacingSenderTest() @@ -34,7 +44,7 @@ infinite_time_(QuicTime::Delta::Infinite()), packet_number_(1), mock_sender_(new StrictMock<MockSendAlgorithm>()), - pacing_sender_(new PacingSender) { + pacing_sender_(new TestPacingSender) { pacing_sender_->set_sender(mock_sender_.get()); // Pick arbitrary time. clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(9)); @@ -44,7 +54,7 @@ void InitPacingRate(QuicPacketCount burst_size, QuicBandwidth bandwidth) { mock_sender_ = std::make_unique<StrictMock<MockSendAlgorithm>>(); - pacing_sender_ = std::make_unique<PacingSender>(); + pacing_sender_ = std::make_unique<TestPacingSender>(); pacing_sender_->set_sender(mock_sender_.get()); EXPECT_CALL(*mock_sender_, PacingRate(_)).WillRepeatedly(Return(bandwidth)); EXPECT_CALL(*mock_sender_, BandwidthEstimate()) @@ -132,7 +142,7 @@ MockClock clock_; QuicPacketNumber packet_number_; std::unique_ptr<StrictMock<MockSendAlgorithm>> mock_sender_; - std::unique_ptr<PacingSender> pacing_sender_; + std::unique_ptr<TestPacingSender> pacing_sender_; }; TEST_F(PacingSenderTest, NoSend) { @@ -466,5 +476,77 @@ } } +TEST_F(PacingSenderTest, IdealNextPacketSendTimeWithLumpyPacing) { + // Set lumpy size to be 3, and cwnd faction to 0.5 + SetQuicFlag(FLAGS_quic_lumpy_pacing_size, 3); + SetQuicFlag(FLAGS_quic_lumpy_pacing_cwnd_fraction, 0.5f); + + // Configure pacing rate of 1 packet per millisecond. + QuicTime::Delta inter_packet_delay = QuicTime::Delta::FromMilliseconds(1); + InitPacingRate(kInitialBurstPackets, + QuicBandwidth::FromBytesAndTimeDelta(kMaxOutgoingPacketSize, + inter_packet_delay)); + + // Send kInitialBurstPackets packets, and verify that they are not paced. + for (int i = 0; i < kInitialBurstPackets; ++i) { + CheckPacketIsSentImmediately(); + } + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 2u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 2 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 1u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 3 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 0u); + + CheckPacketIsDelayed(3 * inter_packet_delay); + + // Wake up on time. + clock_.AdvanceTime(3 * inter_packet_delay); + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 2u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 2 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 1u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 3 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 0u); + + CheckPacketIsDelayed(3 * inter_packet_delay); + + // Wake up late. + clock_.AdvanceTime(4.5 * inter_packet_delay); + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() - 0.5 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 2u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 0.5 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 1u); + + CheckPacketIsSentImmediately(); + EXPECT_EQ(pacing_sender_->ideal_next_packet_send_time(), + clock_.Now() + 1.5 * inter_packet_delay); + EXPECT_EQ(pacing_sender_->lumpy_tokens(), 0u); + + CheckPacketIsDelayed(1.5 * inter_packet_delay); +} + } // namespace test } // namespace quic
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 3595fb9..91b5c2b 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2196,6 +2196,23 @@ return true; } +QuicTime QuicConnection::CalculatePacketSentTime() { + const QuicTime now = clock_->Now(); + if (!supports_release_time_ || per_packet_options_ == nullptr) { + // Don't change the release delay. + return now; + } + + auto next_release_time_result = sent_packet_manager_.GetNextReleaseTime(); + + // Release before |now| is impossible. + QuicTime next_release_time = + std::max(now, next_release_time_result.release_time); + per_packet_options_->release_time_delay = next_release_time - now; + per_packet_options_->allow_burst = next_release_time_result.allow_burst; + return next_release_time; +} + bool QuicConnection::WritePacket(SerializedPacket* packet) { if (ShouldDiscardPacket(*packet)) { ++stats_.packets_discarded; @@ -2250,18 +2267,7 @@ // Measure the RTT from before the write begins to avoid underestimating the // min_rtt_, especially in cases where the thread blocks or gets swapped out // during the WritePacket below. - QuicTime packet_send_time = clock_->Now(); - if (supports_release_time_ && per_packet_options_ != nullptr) { - QuicTime next_release_time = sent_packet_manager_.GetNextReleaseTime(); - QuicTime::Delta release_time_delay = QuicTime::Delta::Zero(); - QuicTime now = packet_send_time; - if (next_release_time > now) { - release_time_delay = next_release_time - now; - // Set packet_send_time to the future to make the RTT estimation accurate. - packet_send_time = next_release_time; - } - per_packet_options_->release_time_delay = release_time_delay; - } + QuicTime packet_send_time = CalculatePacketSentTime(); WriteResult result(WRITE_STATUS_OK, encrypted_length); switch (fate) { case COALESCE:
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index a9e1d7b..1bfacee 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1259,6 +1259,13 @@ // Whether connection is limited by amplification factor. bool LimitedByAmplificationFactor() const; + // Called before sending a packet to get packet send time and to set the + // release time delay in |per_packet_options_|. Return the time when the + // packet is scheduled to be released(a.k.a send time), which is NOW + delay. + // Returns Now() and does not update release time delay if + // |supports_release_time_| is false. + QuicTime CalculatePacketSentTime(); + // We've got a packet write error, should we ignore it? // NOTE: This is not a const function - if return true, the max packet size is // reverted to a previous(smaller) value to avoid write errors in the future.
diff --git a/quic/core/quic_packet_writer.h b/quic/core/quic_packet_writer.h index bd10523..ab29e15 100644 --- a/quic/core/quic_packet_writer.h +++ b/quic/core/quic_packet_writer.h
@@ -28,8 +28,10 @@ // would not forget to override it. virtual std::unique_ptr<PerPacketOptions> Clone() const = 0; - // Specifies release time delay for this packet. + // Specifies ideal release time delay for this packet. QuicTime::Delta release_time_delay = QuicTime::Delta::Zero(); + // Whether it is allowed to send this packet without |release_time_delay|. + bool allow_burst = false; }; // An interface between writers and the entity managing the
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 108fbf3..30779bd 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -1404,9 +1404,12 @@ } } -QuicTime QuicSentPacketManager::GetNextReleaseTime() const { - return using_pacing_ ? pacing_sender_.ideal_next_packet_send_time() - : QuicTime::Zero(); +NextReleaseTimeResult QuicSentPacketManager::GetNextReleaseTime() const { + if (!using_pacing_) { + return {QuicTime::Zero(), false}; + } + + return pacing_sender_.GetNextReleaseTime(); } void QuicSentPacketManager::SetInitialRtt(QuicTime::Delta rtt) {
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 8407fbb..371fa81 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -341,7 +341,7 @@ unacked_packets_.SetSessionNotifier(session_notifier); } - QuicTime GetNextReleaseTime() const; + NextReleaseTimeResult GetNextReleaseTime() const; QuicPacketCount initial_congestion_window() const { return initial_congestion_window_;
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h index 8a73361..7f50df7 100644 --- a/quic/core/quic_types.h +++ b/quic/core/quic_types.h
@@ -723,6 +723,13 @@ HANDSHAKE_CONFIRMED, }; +struct QUIC_NO_EXPORT NextReleaseTimeResult { + // The ideal release time of the packet being sent. + QuicTime release_time; + // Whether it is allowed to send the packet before release_time. + bool allow_burst; +}; + } // namespace quic #endif // QUICHE_QUIC_CORE_QUIC_TYPES_H_