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) {