Move ignore_max_ack_delay logic from RttStats to SentPacketManager, such that the manipulation of received ack_delay is in the same place.
PiperOrigin-RevId: 376179248
diff --git a/quic/core/congestion_control/rtt_stats.cc b/quic/core/congestion_control/rtt_stats.cc
index 2be6fa4..81f4205 100644
--- a/quic/core/congestion_control/rtt_stats.cc
+++ b/quic/core/congestion_control/rtt_stats.cc
@@ -29,8 +29,7 @@
mean_deviation_(QuicTime::Delta::Zero()),
calculate_standard_deviation_(false),
initial_rtt_(QuicTime::Delta::FromMilliseconds(kInitialRttMs)),
- last_update_time_(QuicTime::Zero()),
- ignore_max_ack_delay_(false) {}
+ last_update_time_(QuicTime::Zero()) {}
void RttStats::ExpireSmoothedMetrics() {
mean_deviation_ = std::max(
@@ -63,10 +62,6 @@
QuicTime::Delta rtt_sample(send_delta);
previous_srtt_ = smoothed_rtt_;
-
- if (ignore_max_ack_delay_) {
- ack_delay = QuicTime::Delta::Zero();
- }
// Correct for ack_delay if information received from the peer results in a
// an RTT sample at least as large as min_rtt. Otherwise, only use the
// send_delta.
@@ -138,7 +133,6 @@
calculate_standard_deviation_ = stats.calculate_standard_deviation_;
initial_rtt_ = stats.initial_rtt_;
last_update_time_ = stats.last_update_time_;
- ignore_max_ack_delay_ = stats.ignore_max_ack_delay_;
}
} // namespace quic
diff --git a/quic/core/congestion_control/rtt_stats.h b/quic/core/congestion_control/rtt_stats.h
index 0047b0f..cb591db 100644
--- a/quic/core/congestion_control/rtt_stats.h
+++ b/quic/core/congestion_control/rtt_stats.h
@@ -101,12 +101,6 @@
QuicTime last_update_time() const { return last_update_time_; }
- bool ignore_max_ack_delay() const { return ignore_max_ack_delay_; }
-
- void set_ignore_max_ack_delay(bool ignore_max_ack_delay) {
- ignore_max_ack_delay_ = ignore_max_ack_delay;
- }
-
void EnableStandardDeviationCalculation() {
calculate_standard_deviation_ = true;
}
@@ -130,8 +124,6 @@
bool calculate_standard_deviation_;
QuicTime::Delta initial_rtt_;
QuicTime last_update_time_;
- // Whether to ignore the peer's max ack delay.
- bool ignore_max_ack_delay_;
};
} // namespace quic
diff --git a/quic/core/congestion_control/rtt_stats_test.cc b/quic/core/congestion_control/rtt_stats_test.cc
index bdf54c6..1df509f 100644
--- a/quic/core/congestion_control/rtt_stats_test.cc
+++ b/quic/core/congestion_control/rtt_stats_test.cc
@@ -56,34 +56,6 @@
rtt_stats_.smoothed_rtt());
}
-TEST_F(RttStatsTest, SmoothedRttIgnoreAckDelay) {
- rtt_stats_.set_ignore_max_ack_delay(true);
- // Verify that ack_delay is ignored in the first measurement.
- rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300),
- QuicTime::Delta::FromMilliseconds(100),
- QuicTime::Zero());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt());
- // Verify that a plausible ack delay increases the max ack delay.
- rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300),
- QuicTime::Delta::FromMilliseconds(100),
- QuicTime::Zero());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt());
- // Verify that Smoothed RTT includes max ack delay if it's reasonable.
- rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300),
- QuicTime::Delta::FromMilliseconds(50), QuicTime::Zero());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.latest_rtt());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.smoothed_rtt());
- // Verify that large erroneous ack_delay does not change Smoothed RTT.
- rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(200),
- QuicTime::Delta::FromMilliseconds(300),
- QuicTime::Zero());
- EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt());
- EXPECT_EQ(QuicTime::Delta::FromMicroseconds(287500),
- rtt_stats_.smoothed_rtt());
-}
-
// Ensure that the potential rounding artifacts in EWMA calculation do not cause
// the SRTT to drift too far from the exact value.
TEST_F(RttStatsTest, SmoothedRttStability) {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 53d9f0e..7d600d0 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -117,7 +117,8 @@
use_standard_deviation_for_pto_(false),
pto_multiplier_without_rtt_samples_(3),
num_ptos_for_path_degrading_(0),
- ignore_pings_(false) {
+ ignore_pings_(false),
+ ignore_ack_delay_(false) {
SetSendAlgorithm(congestion_control_type);
if (pto_enabled_) {
QUIC_RELOADABLE_FLAG_COUNT_N(quic_default_on_pto, 1, 2);
@@ -158,7 +159,7 @@
}
}
if (config.HasClientSentConnectionOption(kMAD0, perspective)) {
- rtt_stats_.set_ignore_max_ack_delay(true);
+ ignore_ack_delay_ = true;
}
if (config.HasClientSentConnectionOption(kMAD2, perspective)) {
// Set the minimum to the alarm granularity.
@@ -1529,6 +1530,9 @@
if (ack_delay_time > peer_max_ack_delay()) {
ack_delay_time = peer_max_ack_delay();
}
+ if (ignore_ack_delay_) {
+ ack_delay_time = QuicTime::Delta::Zero();
+ }
rtt_updated_ =
MaybeUpdateRTT(largest_acked, ack_delay_time, ack_receive_time);
last_ack_frame_.ack_delay_time = ack_delay_time;
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 781f6f9..eeb474f 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -745,6 +745,9 @@
// If true, do not use PING only packets for RTT measurement or congestion
// control.
bool ignore_pings_;
+
+ // Whether to ignore the ack_delay in received ACKs.
+ bool ignore_ack_delay_;
};
} // namespace quic
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 388a662..549dd7d 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -4684,6 +4684,84 @@
EXPECT_EQ(frame.packet_tolerance, 10u);
}
+TEST_F(QuicSentPacketManagerTest, SmoothedRttIgnoreAckDelay) {
+ QuicConfig config;
+ QuicTagVector options;
+ options.push_back(kMAD0);
+ QuicConfigPeer::SetReceivedConnectionOptions(&config, options);
+ EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange());
+ EXPECT_CALL(*send_algorithm_, CanSend(_)).WillRepeatedly(Return(true));
+ EXPECT_CALL(*send_algorithm_, GetCongestionWindow())
+ .WillRepeatedly(Return(10 * kDefaultTCPMSS));
+ manager_.SetFromConfig(config);
+
+ SendDataPacket(1);
+ // Ack 1.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(300));
+ ExpectAck(1);
+ manager_.OnAckFrameStart(QuicPacketNumber(1),
+ QuicTime::Delta::FromMilliseconds(100),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_INITIAL));
+ // Verify that ack_delay is ignored in the first measurement.
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->latest_rtt());
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->smoothed_rtt());
+
+ SendDataPacket(2);
+ // Ack 2.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(300));
+ ExpectAck(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(2),
+ QuicTime::Delta::FromMilliseconds(100),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(3));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
+ ENCRYPTION_INITIAL));
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->latest_rtt());
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->smoothed_rtt());
+
+ SendDataPacket(3);
+ // Ack 3.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(300));
+ ExpectAck(3);
+ manager_.OnAckFrameStart(QuicPacketNumber(3),
+ QuicTime::Delta::FromMilliseconds(50), clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(4));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3),
+ ENCRYPTION_INITIAL));
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->latest_rtt());
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300),
+ manager_.GetRttStats()->smoothed_rtt());
+
+ SendDataPacket(4);
+ // Ack 4.
+ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(200));
+ ExpectAck(4);
+ manager_.OnAckFrameStart(QuicPacketNumber(4),
+ QuicTime::Delta::FromMilliseconds(300),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(4), QuicPacketNumber(5));
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(4),
+ ENCRYPTION_INITIAL));
+ // Verify that large erroneous ack_delay does not change Smoothed RTT.
+ EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200),
+ manager_.GetRttStats()->latest_rtt());
+ EXPECT_EQ(QuicTime::Delta::FromMicroseconds(287500),
+ manager_.GetRttStats()->smoothed_rtt());
+}
+
TEST_F(QuicSentPacketManagerTest, BuildAckFrequencyFrameWithSRTT) {
SetQuicReloadableFlag(quic_can_send_ack_frequency, true);
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
