Simplify the usage of ack_frequency and ack_delay in quic_received_packet_manager. flag protected by gfe2_reloadable_flag_quic_simplify_received_packet_manager_ack.
A few other no-op changes:
1) Renamed ack_frequency_before_decimation to ack_frequency_.
2) Removed a few unused getters in uber_received_packet_manager and quic_connection.
PiperOrigin-RevId: 323473745
Change-Id: I51f119d048271543b5a7ae000d3d4bcffc3679ab
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 9a74a33..f8e3f6e 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -4601,14 +4601,9 @@
new_value);
}
-size_t QuicConnection::ack_frequency_before_ack_decimation() const {
- return uber_received_packet_manager_.ack_frequency_before_ack_decimation();
-}
-
-void QuicConnection::set_ack_frequency_before_ack_decimation(size_t new_value) {
+void QuicConnection::set_ack_frequency(size_t new_value) {
DCHECK_GT(new_value, 0u);
- uber_received_packet_manager_.set_ack_frequency_before_ack_decimation(
- new_value);
+ uber_received_packet_manager_.set_ack_frequency(new_value);
}
const QuicAckFrame& QuicConnection::ack_frame() const {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 9439adf..32eabef 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -897,8 +897,7 @@
size_t min_received_before_ack_decimation() const;
void set_min_received_before_ack_decimation(size_t new_value);
- size_t ack_frequency_before_ack_decimation() const;
- void set_ack_frequency_before_ack_decimation(size_t new_value);
+ void set_ack_frequency(size_t new_value);
// If |defer| is true, configures the connection to defer sending packets in
// response to an ACK to the SendAlarm. If |defer| is false, packets may be
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index ac301c2..5db917f 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -2936,7 +2936,7 @@
}
TEST_P(QuicConnectionTest, AckSentEveryNthPacket) {
- connection_.set_ack_frequency_before_ack_decimation(3);
+ connection_.set_ack_frequency(3);
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(39);
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 324767c..9cc6c29 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -44,8 +44,7 @@
ack_mode_(ACK_DECIMATION),
num_retransmittable_packets_received_since_last_ack_sent_(0),
min_received_before_ack_decimation_(kMinReceivedBeforeAckDecimation),
- ack_frequency_before_ack_decimation_(
- kDefaultRetransmittablePacketsBeforeAck),
+ ack_frequency_(kDefaultRetransmittablePacketsBeforeAck),
ack_decimation_delay_(kAckDecimationDelay),
unlimited_ack_decimation_(false),
fast_ack_after_quiescence_(false),
@@ -216,6 +215,38 @@
ack_frame_.packets.Min() >= peer_least_packet_awaiting_ack_);
}
+QuicTime::Delta QuicReceivedPacketManager::GetMaxAckDelay(
+ QuicPacketNumber last_received_packet_number,
+ const RttStats& rtt_stats) const {
+ DCHECK(simplify_received_packet_manager_ack_);
+ if (last_received_packet_number <
+ PeerFirstSendingPacketNumber() + min_received_before_ack_decimation_) {
+ return local_max_ack_delay_;
+ }
+
+ // Wait for the minimum of the ack decimation delay or the delayed ack time
+ // before sending an ack.
+ QuicTime::Delta ack_delay = std::min(
+ local_max_ack_delay_, rtt_stats.min_rtt() * ack_decimation_delay_);
+ if (GetQuicReloadableFlag(quic_ack_delay_alarm_granularity)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_ack_delay_alarm_granularity);
+ ack_delay = std::max(ack_delay, kAlarmGranularity);
+ }
+ return ack_delay;
+}
+
+void QuicReceivedPacketManager::MaybeUpdateAckFrequency(
+ QuicPacketNumber last_received_packet_number) {
+ DCHECK(simplify_received_packet_manager_ack_);
+ if (last_received_packet_number <
+ PeerFirstSendingPacketNumber() + min_received_before_ack_decimation_) {
+ return;
+ }
+ ack_frequency_ = unlimited_ack_decimation_
+ ? std::numeric_limits<size_t>::max()
+ : kMaxRetransmittablePacketsBeforeAck;
+}
+
void QuicReceivedPacketManager::MaybeUpdateAckTimeout(
bool should_last_packet_instigate_acks,
QuicPacketNumber last_received_packet_number,
@@ -240,6 +271,26 @@
}
++num_retransmittable_packets_received_since_last_ack_sent_;
+
+ if (simplify_received_packet_manager_ack_) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_simplify_received_packet_manager_ack);
+ MaybeUpdateAckFrequency(last_received_packet_number);
+ if (num_retransmittable_packets_received_since_last_ack_sent_ >=
+ ack_frequency_) {
+ ack_timeout_ = now;
+ return;
+ }
+
+ if (HasNewMissingPackets()) {
+ ack_timeout_ = now;
+ return;
+ }
+
+ MaybeUpdateAckTimeoutTo(
+ now + GetMaxAckDelay(last_received_packet_number, *rtt_stats));
+ return;
+ }
+
if (ack_mode_ != TCP_ACKING &&
last_received_packet_number >= PeerFirstSendingPacketNumber() +
min_received_before_ack_decimation_) {
@@ -269,7 +320,7 @@
} else {
// Ack with a timer or every 2 packets by default.
if (num_retransmittable_packets_received_since_last_ack_sent_ >=
- ack_frequency_before_ack_decimation_) {
+ ack_frequency_) {
ack_timeout_ = now;
} else if (fast_ack_after_quiescence_ &&
(now - time_of_previous_received_packet_) >
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index a350462..5f5525a 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -113,15 +113,11 @@
min_received_before_ack_decimation_ = new_value;
}
- size_t ack_frequency_before_ack_decimation() const {
- return ack_frequency_before_ack_decimation_;
- }
- void set_ack_frequency_before_ack_decimation(size_t new_value) {
+ void set_ack_frequency(size_t new_value) {
DCHECK_GT(new_value, 0u);
- ack_frequency_before_ack_decimation_ = new_value;
+ ack_frequency_ = new_value;
}
- QuicTime::Delta local_max_ack_delay() const { return local_max_ack_delay_; }
void set_local_max_ack_delay(QuicTime::Delta local_max_ack_delay) {
local_max_ack_delay_ = local_max_ack_delay;
}
@@ -136,6 +132,12 @@
// Sets ack_timeout_ to |time| if ack_timeout_ is not initialized or > time.
void MaybeUpdateAckTimeoutTo(QuicTime time);
+ // Maybe update ack_frequency_ when condition meets.
+ void MaybeUpdateAckFrequency(QuicPacketNumber last_received_packet_number);
+
+ QuicTime::Delta GetMaxAckDelay(QuicPacketNumber last_received_packet_number,
+ const RttStats& rtt_stats) const;
+
// Least packet number of the the packet sent by the peer for which it
// hasn't received an ack.
QuicPacketNumber peer_least_packet_awaiting_ack_;
@@ -168,8 +170,8 @@
QuicPacketCount num_retransmittable_packets_received_since_last_ack_sent_;
// Ack decimation will start happening after this many packets are received.
size_t min_received_before_ack_decimation_;
- // Before ack decimation starts (if enabled), we ack every n-th packet.
- size_t ack_frequency_before_ack_decimation_;
+ // Ack every n-th packet.
+ size_t ack_frequency_;
// The max delay in fraction of min_rtt to use when sending decimated acks.
float ack_decimation_delay_;
// When true, removes ack decimation's max number of packets(10) before
@@ -200,6 +202,10 @@
const bool remove_unused_ack_options_ =
GetQuicReloadableFlag(quic_remove_unused_ack_options);
+ const bool simplify_received_packet_manager_ack_ =
+ remove_unused_ack_options_ &&
+ GetQuicReloadableFlag(quic_simplify_received_packet_manager_ack);
+
// Last sent largest acked, which gets updated when ACK was successfully sent.
QuicPacketNumber last_sent_largest_acked_;
};
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc
index 56b6643..ca56662 100644
--- a/quic/core/quic_received_packet_manager_test.cc
+++ b/quic/core/quic_received_packet_manager_test.cc
@@ -352,7 +352,7 @@
TEST_P(QuicReceivedPacketManagerTest, AckSentEveryNthPacket) {
EXPECT_FALSE(HasPendingAck());
- received_manager_.set_ack_frequency_before_ack_decimation(3);
+ received_manager_.set_ack_frequency(3);
// Receives packets 1 - 39.
for (size_t i = 1; i <= 39; ++i) {
diff --git a/quic/core/uber_received_packet_manager.cc b/quic/core/uber_received_packet_manager.cc
index 723ac30..b017a84 100644
--- a/quic/core/uber_received_packet_manager.cc
+++ b/quic/core/uber_received_packet_manager.cc
@@ -193,14 +193,9 @@
}
}
-size_t UberReceivedPacketManager::ack_frequency_before_ack_decimation() const {
- return received_packet_managers_[0].ack_frequency_before_ack_decimation();
-}
-
-void UberReceivedPacketManager::set_ack_frequency_before_ack_decimation(
- size_t new_value) {
+void UberReceivedPacketManager::set_ack_frequency(size_t new_value) {
for (auto& received_packet_manager : received_packet_managers_) {
- received_packet_manager.set_ack_frequency_before_ack_decimation(new_value);
+ received_packet_manager.set_ack_frequency(new_value);
}
}
@@ -221,13 +216,6 @@
}
}
-QuicTime::Delta UberReceivedPacketManager::max_ack_delay() {
- if (!supports_multiple_packet_number_spaces_) {
- return received_packet_managers_[0].local_max_ack_delay();
- }
- return received_packet_managers_[APPLICATION_DATA].local_max_ack_delay();
-}
-
void UberReceivedPacketManager::set_max_ack_delay(
QuicTime::Delta max_ack_delay) {
if (!supports_multiple_packet_number_spaces_) {
diff --git a/quic/core/uber_received_packet_manager.h b/quic/core/uber_received_packet_manager.h
index d76ce13..9cfa247 100644
--- a/quic/core/uber_received_packet_manager.h
+++ b/quic/core/uber_received_packet_manager.h
@@ -78,8 +78,7 @@
size_t min_received_before_ack_decimation() const;
void set_min_received_before_ack_decimation(size_t new_value);
- size_t ack_frequency_before_ack_decimation() const;
- void set_ack_frequency_before_ack_decimation(size_t new_value);
+ void set_ack_frequency(size_t new_value);
bool supports_multiple_packet_number_spaces() const {
return supports_multiple_packet_number_spaces_;
@@ -91,8 +90,7 @@
void set_max_ack_ranges(size_t max_ack_ranges);
- // Get and set the max ack delay to use for application data.
- QuicTime::Delta max_ack_delay();
+ // Set the max ack delay to use for application data.
void set_max_ack_delay(QuicTime::Delta max_ack_delay);
void set_save_timestamps(bool save_timestamps);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc
index 41dadcb..1b5a196 100644
--- a/quic/core/uber_received_packet_manager_test.cc
+++ b/quic/core/uber_received_packet_manager_test.cc
@@ -299,7 +299,7 @@
TEST_F(UberReceivedPacketManagerTest, AckSentEveryNthPacket) {
EXPECT_FALSE(HasPendingAck());
- manager_->set_ack_frequency_before_ack_decimation(3);
+ manager_->set_ack_frequency(3);
// Receives packets 1 - 39.
for (size_t i = 1; i <= 39; ++i) {
