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_