gfe-relnote: Deprecate gfe2_reloadable_flag_quic_rpm_decides_when_to_send_acks. PiperOrigin-RevId: 254188898 Change-Id: Ied75fc45a0dd45457928664648ff96d41738d0c4
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index bca1ca3..b9c74ec 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -249,7 +249,6 @@ current_packet_data_(nullptr), last_decrypted_packet_level_(ENCRYPTION_INITIAL), should_last_packet_instigate_acks_(false), - was_last_packet_missing_(false), max_undecryptable_packets_(0), max_tracked_packets_(kMaxTrackedPackets), pending_version_negotiation_packet_(false), @@ -259,15 +258,7 @@ close_connection_after_five_rtos_(false), received_packet_manager_(&stats_), uber_received_packet_manager_(&stats_), - num_retransmittable_packets_received_since_last_ack_sent_(0), - num_packets_received_since_last_ack_sent_(0), stop_waiting_count_(0), - ack_mode_(GetQuicReloadableFlag(quic_enable_ack_decimation) - ? ACK_DECIMATION - : TCP_ACKING), - ack_decimation_delay_(kAckDecimationDelay), - unlimited_ack_decimation_(false), - fast_ack_after_quiescence_(false), pending_retransmission_alarm_(false), defer_send_in_response_to_packets_(false), ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)), @@ -306,7 +297,6 @@ handshake_timeout_(QuicTime::Delta::Infinite()), time_of_first_packet_sent_after_receiving_(QuicTime::Zero()), time_of_last_received_packet_(clock_->ApproximateNow()), - time_of_previous_received_packet_(QuicTime::Zero()), sent_packet_manager_( perspective, clock_, @@ -329,9 +319,6 @@ consecutive_num_packets_with_no_retransmittable_frames_(0), max_consecutive_num_packets_with_no_retransmittable_frames_( kMaxConsecutiveNonRetransmittablePackets), - min_received_before_ack_decimation_(kMinReceivedBeforeAckDecimation), - ack_frequency_before_ack_decimation_( - kDefaultRetransmittablePacketsBeforeAck), fill_up_link_during_probing_(false), probing_retransmission_pending_(false), stateless_reset_token_received_(false), @@ -342,24 +329,16 @@ supports_release_time_(false), release_time_into_future_(QuicTime::Delta::Zero()), no_version_negotiation_(supported_versions.size() == 1), - send_ack_when_on_can_write_(false), retry_has_been_parsed_(false), validate_packet_number_post_decryption_( GetQuicReloadableFlag(quic_validate_packet_number_post_decryption)), use_uber_received_packet_manager_( - received_packet_manager_.decide_when_to_send_acks() && validate_packet_number_post_decryption_ && GetQuicReloadableFlag(quic_use_uber_received_packet_manager)) { - if (ack_mode_ == ACK_DECIMATION) { - QUIC_RELOADABLE_FLAG_COUNT(quic_enable_ack_decimation); - } if (perspective_ == Perspective::IS_SERVER && supported_versions.size() == 1) { QUIC_RESTART_FLAG_COUNT(quic_no_server_conn_ver_negotiation2); } - if (received_packet_manager_.decide_when_to_send_acks()) { - QUIC_RELOADABLE_FLAG_COUNT(quic_rpm_decides_when_to_send_acks); - } if (validate_packet_number_post_decryption_) { QUIC_RELOADABLE_FLAG_COUNT(quic_validate_packet_number_post_decryption); } @@ -464,37 +443,10 @@ if (debug_visitor_ != nullptr) { debug_visitor_->OnSetFromConfig(config); } - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - uber_received_packet_manager_.SetFromConfig(config, perspective_); - } else { - received_packet_manager_.SetFromConfig(config, perspective_); - } + if (use_uber_received_packet_manager_) { + uber_received_packet_manager_.SetFromConfig(config, perspective_); } else { - if (GetQuicReloadableFlag(quic_enable_ack_decimation) && - config.HasClientSentConnectionOption(kACD0, perspective_)) { - ack_mode_ = TCP_ACKING; - } - if (config.HasClientSentConnectionOption(kACKD, perspective_)) { - ack_mode_ = ACK_DECIMATION; - } - if (config.HasClientSentConnectionOption(kAKD2, perspective_)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; - } - if (config.HasClientSentConnectionOption(kAKD3, perspective_)) { - ack_mode_ = ACK_DECIMATION; - ack_decimation_delay_ = kShortAckDecimationDelay; - } - if (config.HasClientSentConnectionOption(kAKD4, perspective_)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; - ack_decimation_delay_ = kShortAckDecimationDelay; - } - if (config.HasClientSentConnectionOption(kAKDU, perspective_)) { - unlimited_ack_decimation_ = true; - } - if (config.HasClientSentConnectionOption(kACKQ, perspective_)) { - fast_ack_after_quiescence_ = true; - } + received_packet_manager_.SetFromConfig(config, perspective_); } if (config.HasClientSentConnectionOption(k5RTO, perspective_)) { close_connection_after_five_rtos_ = true; @@ -1025,11 +977,6 @@ --stats_.packets_dropped; QUIC_DVLOG(1) << ENDPOINT << "Received packet header: " << header; last_header_ = header; - // An ack will be sent if a missing retransmittable packet was received; - if (!use_uber_received_packet_manager_) { - was_last_packet_missing_ = - received_packet_manager_.IsMissing(last_header_.packet_number); - } // Record packet receipt to populate ack info before processing stream // frames, since the processing may result in sending a bundled ack. @@ -1572,48 +1519,38 @@ current_effective_peer_migration_type_ = NO_CHANGE; - // An ack will be sent if a missing retransmittable packet was received; - const bool was_missing = - should_last_packet_instigate_acks_ && was_last_packet_missing_; - - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - // Some encryption levels share a packet number space, it is therefore - // possible for us to want to ack some packets even though we do not yet - // have the appropriate keys to encrypt the acks. In this scenario we - // do not update the ACK timeout. This can happen for example with - // IETF QUIC on the server when we receive 0-RTT packets and do not yet - // have 1-RTT keys (0-RTT packets are acked at the 1-RTT level). - // Note that this could cause slight performance degradations in the edge - // case where one packet is received, then the encrypter is installed, - // then a second packet is received; as that could cause the ACK for the - // second packet to be delayed instead of immediate. This is currently - // considered to be small enough of an edge case to not be optimized for. - if (!SupportsMultiplePacketNumberSpaces() || - framer_.HasEncrypterOfEncryptionLevel(QuicUtils::GetEncryptionLevel( - QuicUtils::GetPacketNumberSpace(last_decrypted_packet_level_)))) { - uber_received_packet_manager_.MaybeUpdateAckTimeout( - should_last_packet_instigate_acks_, last_decrypted_packet_level_, - last_header_.packet_number, time_of_last_received_packet_, - clock_->ApproximateNow(), sent_packet_manager_.GetRttStats(), - sent_packet_manager_.delayed_ack_time()); - } else { - QUIC_DLOG(INFO) << ENDPOINT << "Not updating ACK timeout for " - << QuicUtils::EncryptionLevelToString( - last_decrypted_packet_level_) - << " as we do not have the corresponding encrypter"; - } - } else { - received_packet_manager_.MaybeUpdateAckTimeout( - should_last_packet_instigate_acks_, last_header_.packet_number, - time_of_last_received_packet_, clock_->ApproximateNow(), - sent_packet_manager_.GetRttStats(), + if (use_uber_received_packet_manager_) { + // Some encryption levels share a packet number space, it is therefore + // possible for us to want to ack some packets even though we do not yet + // have the appropriate keys to encrypt the acks. In this scenario we + // do not update the ACK timeout. This can happen for example with + // IETF QUIC on the server when we receive 0-RTT packets and do not yet + // have 1-RTT keys (0-RTT packets are acked at the 1-RTT level). + // Note that this could cause slight performance degradations in the edge + // case where one packet is received, then the encrypter is installed, + // then a second packet is received; as that could cause the ACK for the + // second packet to be delayed instead of immediate. This is currently + // considered to be small enough of an edge case to not be optimized for. + if (!SupportsMultiplePacketNumberSpaces() || + framer_.HasEncrypterOfEncryptionLevel(QuicUtils::GetEncryptionLevel( + QuicUtils::GetPacketNumberSpace(last_decrypted_packet_level_)))) { + uber_received_packet_manager_.MaybeUpdateAckTimeout( + should_last_packet_instigate_acks_, last_decrypted_packet_level_, + last_header_.packet_number, time_of_last_received_packet_, + clock_->ApproximateNow(), sent_packet_manager_.GetRttStats(), sent_packet_manager_.delayed_ack_time()); + } else { + QUIC_DLOG(INFO) << ENDPOINT << "Not updating ACK timeout for " + << QuicUtils::EncryptionLevelToString( + last_decrypted_packet_level_) + << " as we do not have the corresponding encrypter"; } - } else if (ack_frame_updated()) { - // It's possible the ack frame was sent along with response data, so it - // no longer needs to be sent. - MaybeQueueAck(was_missing); + } else { + received_packet_manager_.MaybeUpdateAckTimeout( + should_last_packet_instigate_acks_, last_header_.packet_number, + time_of_last_received_packet_, clock_->ApproximateNow(), + sent_packet_manager_.GetRttStats(), + sent_packet_manager_.delayed_ack_time()); } ClearLastFrames(); @@ -1635,94 +1572,6 @@ ConnectionCloseSource::FROM_PEER); } -void QuicConnection::MaybeQueueAck(bool was_missing) { - DCHECK(!received_packet_manager_.decide_when_to_send_acks()); - ++num_packets_received_since_last_ack_sent_; - // Determine whether the newly received packet was missing before recording - // the received packet. - if (was_missing) { - // Only ack immediately if an ACK frame was sent with a larger - // largest acked than the newly received packet number. - const QuicPacketNumber largest_sent_largest_acked = - sent_packet_manager_.unacked_packets().largest_sent_largest_acked(); - if (largest_sent_largest_acked.IsInitialized() && - last_header_.packet_number < largest_sent_largest_acked) { - MaybeSetAckAlarmTo(clock_->ApproximateNow()); - } - } - - if (should_last_packet_instigate_acks_) { - ++num_retransmittable_packets_received_since_last_ack_sent_; - if (ack_mode_ != TCP_ACKING && - last_header_.packet_number >= - received_packet_manager_.PeerFirstSendingPacketNumber() + - min_received_before_ack_decimation_) { - // Ack up to 10 packets at once unless ack decimation is unlimited. - if (!unlimited_ack_decimation_ && - num_retransmittable_packets_received_since_last_ack_sent_ >= - kMaxRetransmittablePacketsBeforeAck) { - MaybeSetAckAlarmTo(clock_->ApproximateNow()); - } else if (!ack_alarm_->IsSet()) { - // Wait for the minimum of the ack decimation delay or the delayed ack - // time before sending an ack. - QuicTime::Delta ack_delay = - std::min(sent_packet_manager_.delayed_ack_time(), - sent_packet_manager_.GetRttStats()->min_rtt() * - ack_decimation_delay_); - const QuicTime approximate_now = clock_->ApproximateNow(); - if (fast_ack_after_quiescence_ && - (approximate_now - time_of_previous_received_packet_) > - sent_packet_manager_.GetRttStats()->SmoothedOrInitialRtt()) { - // Ack the first packet out of queiscence faster, because QUIC does - // not pace the first few packets and commonly these may be handshake - // or TLP packets, which we'd like to acknowledge quickly. - ack_delay = QuicTime::Delta::FromMilliseconds(1); - } - ack_alarm_->Set(approximate_now + ack_delay); - } - } 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_) { - MaybeSetAckAlarmTo(clock_->ApproximateNow()); - } else if (!ack_alarm_->IsSet()) { - const QuicTime approximate_now = clock_->ApproximateNow(); - if (fast_ack_after_quiescence_ && - (approximate_now - time_of_previous_received_packet_) > - sent_packet_manager_.GetRttStats()->SmoothedOrInitialRtt()) { - // Ack the first packet out of queiscence faster, because QUIC does - // not pace the first few packets and commonly these may be handshake - // or TLP packets, which we'd like to acknowledge quickly. - ack_alarm_->Set(approximate_now + - QuicTime::Delta::FromMilliseconds(1)); - } else { - ack_alarm_->Set(approximate_now + - sent_packet_manager_.delayed_ack_time()); - } - } - } - - // If there are new missing packets to report, send an ack immediately. - if (received_packet_manager_.HasNewMissingPackets()) { - if (ack_mode_ == ACK_DECIMATION_WITH_REORDERING) { - // Wait the minimum of an eighth min_rtt and the existing ack time. - QuicTime ack_time = - clock_->ApproximateNow() + - 0.125 * sent_packet_manager_.GetRttStats()->min_rtt(); - if (!ack_alarm_->IsSet() || ack_alarm_->deadline() > ack_time) { - ack_alarm_->Update(ack_time, QuicTime::Delta::Zero()); - } - } else { - MaybeSetAckAlarmTo(clock_->ApproximateNow()); - } - } - - if (fast_ack_after_quiescence_) { - time_of_previous_received_packet_ = time_of_last_received_packet_; - } - } -} - void QuicConnection::ClearLastFrames() { should_last_packet_instigate_acks_ = false; } @@ -2071,27 +1920,19 @@ ScopedPacketFlusher flusher(this); WriteQueuedPackets(); - if (received_packet_manager_.decide_when_to_send_acks()) { - const QuicTime ack_timeout = - use_uber_received_packet_manager_ - ? uber_received_packet_manager_.GetEarliestAckTimeout() - : received_packet_manager_.ack_timeout(); - if (ack_timeout.IsInitialized() && - ack_timeout <= clock_->ApproximateNow()) { - // Send an ACK now because either 1) we were write blocked when we last - // tried to send an ACK, or 2) both ack alarm and send alarm were set to - // go off together. - if (SupportsMultiplePacketNumberSpaces()) { - SendAllPendingAcks(); - } else { - SendAck(); - } - } - } else if (send_ack_when_on_can_write_) { + const QuicTime ack_timeout = + use_uber_received_packet_manager_ + ? uber_received_packet_manager_.GetEarliestAckTimeout() + : received_packet_manager_.ack_timeout(); + if (ack_timeout.IsInitialized() && ack_timeout <= clock_->ApproximateNow()) { // Send an ACK now because either 1) we were write blocked when we last - // tried to send an ACK, or 2) both ack alarm and send alarm were set to go - // off together. - SendAck(); + // tried to send an ACK, or 2) both ack alarm and send alarm were set to + // go off together. + if (SupportsMultiplePacketNumberSpaces()) { + SendAllPendingAcks(); + } else { + SendAck(); + } } if (!session_decides_what_to_write()) { WritePendingRetransmissions(); @@ -2359,17 +2200,13 @@ const QuicFrames QuicConnection::MaybeBundleAckOpportunistically() { QuicFrames frames; bool has_pending_ack = false; - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - has_pending_ack = - uber_received_packet_manager_ - .GetAckTimeout(QuicUtils::GetPacketNumberSpace(encryption_level_)) - .IsInitialized(); - } else { - has_pending_ack = received_packet_manager_.ack_timeout().IsInitialized(); - } + if (use_uber_received_packet_manager_) { + has_pending_ack = + uber_received_packet_manager_ + .GetAckTimeout(QuicUtils::GetPacketNumberSpace(encryption_level_)) + .IsInitialized(); } else { - has_pending_ack = ack_alarm_->IsSet(); + has_pending_ack = received_packet_manager_.ack_timeout().IsInitialized(); } if (!has_pending_ack && stop_waiting_count_ <= 1) { // No need to send an ACK. @@ -2794,12 +2631,6 @@ void QuicConnection::SendAck() { DCHECK(!SupportsMultiplePacketNumberSpaces()); - if (!received_packet_manager_.decide_when_to_send_acks()) { - // When received_packet_manager decides when to send ack, delaying - // ResetAckStates until ACK is successfully flushed. - ResetAckStates(); - } - QUIC_DVLOG(1) << ENDPOINT << "Sending an ACK proactively"; QuicFrames frames; frames.push_back(GetUpdatedAckFrame()); @@ -2808,14 +2639,10 @@ PopulateStopWaitingFrame(&stop_waiting); frames.push_back(QuicFrame(stop_waiting)); } - if (received_packet_manager_.decide_when_to_send_acks()) { - if (!packet_generator_.FlushAckFrame(frames)) { - return; - } - ResetAckStates(); - } else { - send_ack_when_on_can_write_ = !packet_generator_.FlushAckFrame(frames); + if (!packet_generator_.FlushAckFrame(frames)) { + return; } + ResetAckStates(); if (consecutive_num_packets_with_no_retransmittable_frames_ < max_consecutive_num_packets_with_no_retransmittable_frames_) { return; @@ -3320,21 +3147,18 @@ } if (flush_and_set_pending_retransmission_alarm_on_delete_) { - if (connection_->received_packet_manager_.decide_when_to_send_acks()) { - const QuicTime ack_timeout = - connection_->use_uber_received_packet_manager_ - ? connection_->uber_received_packet_manager_ - .GetEarliestAckTimeout() - : connection_->received_packet_manager_.ack_timeout(); - if (ack_timeout.IsInitialized()) { - if (ack_timeout <= connection_->clock_->ApproximateNow() && - !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) { - // Cancel ACK alarm if connection is write blocked, and ACK will be - // sent when connection gets unblocked. - connection_->ack_alarm_->Cancel(); - } else { - connection_->MaybeSetAckAlarmTo(ack_timeout); - } + const QuicTime ack_timeout = + connection_->use_uber_received_packet_manager_ + ? connection_->uber_received_packet_manager_.GetEarliestAckTimeout() + : connection_->received_packet_manager_.ack_timeout(); + if (ack_timeout.IsInitialized()) { + if (ack_timeout <= connection_->clock_->ApproximateNow() && + !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) { + // Cancel ACK alarm if connection is write blocked, and ACK will be + // sent when connection gets unblocked. + connection_->ack_alarm_->Cancel(); + } else { + connection_->MaybeSetAckAlarmTo(ack_timeout); } } if (connection_->ack_alarm_->IsSet() && @@ -3348,9 +3172,6 @@ connection_->clock_->ApproximateNow()) { // If send alarm will go off soon, let send alarm send the ACK. connection_->ack_alarm_->Cancel(); - if (!connection_->received_packet_manager_.decide_when_to_send_acks()) { - connection_->send_ack_when_on_can_write_ = true; - } } else if (connection_->SupportsMultiplePacketNumberSpaces()) { connection_->SendAllPendingAcks(); } else { @@ -3867,14 +3688,10 @@ void QuicConnection::ResetAckStates() { ack_alarm_->Cancel(); stop_waiting_count_ = 0; - num_retransmittable_packets_received_since_last_ack_sent_ = 0; - num_packets_received_since_last_ack_sent_ = 0; - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - uber_received_packet_manager_.ResetAckStates(encryption_level_); - } else { - received_packet_manager_.ResetAckStates(); - } + if (use_uber_received_packet_manager_) { + uber_received_packet_manager_.ResetAckStates(encryption_level_); + } else { + received_packet_manager_.ResetAckStates(); } } @@ -4058,52 +3875,35 @@ } size_t QuicConnection::min_received_before_ack_decimation() const { - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - return uber_received_packet_manager_.min_received_before_ack_decimation(); - } - return received_packet_manager_.min_received_before_ack_decimation(); + if (use_uber_received_packet_manager_) { + return uber_received_packet_manager_.min_received_before_ack_decimation(); } - return min_received_before_ack_decimation_; + return received_packet_manager_.min_received_before_ack_decimation(); } void QuicConnection::set_min_received_before_ack_decimation(size_t new_value) { - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - uber_received_packet_manager_.set_min_received_before_ack_decimation( - new_value); - } else { - received_packet_manager_.set_min_received_before_ack_decimation( - new_value); - } + if (use_uber_received_packet_manager_) { + uber_received_packet_manager_.set_min_received_before_ack_decimation( + new_value); } else { - min_received_before_ack_decimation_ = new_value; + received_packet_manager_.set_min_received_before_ack_decimation(new_value); } } size_t QuicConnection::ack_frequency_before_ack_decimation() const { - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - return uber_received_packet_manager_ - .ack_frequency_before_ack_decimation(); - } - return received_packet_manager_.ack_frequency_before_ack_decimation(); + if (use_uber_received_packet_manager_) { + return uber_received_packet_manager_.ack_frequency_before_ack_decimation(); } - return ack_frequency_before_ack_decimation_; + return received_packet_manager_.ack_frequency_before_ack_decimation(); } void QuicConnection::set_ack_frequency_before_ack_decimation(size_t new_value) { DCHECK_GT(new_value, 0u); - if (received_packet_manager_.decide_when_to_send_acks()) { - if (use_uber_received_packet_manager_) { - uber_received_packet_manager_.set_ack_frequency_before_ack_decimation( - new_value); - } else { - received_packet_manager_.set_ack_frequency_before_ack_decimation( - new_value); - } + if (use_uber_received_packet_manager_) { + uber_received_packet_manager_.set_ack_frequency_before_ack_decimation( + new_value); } else { - ack_frequency_before_ack_decimation_ = new_value; + received_packet_manager_.set_ack_frequency_before_ack_decimation(new_value); } }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index a0299a2..af673e9 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -1008,10 +1008,6 @@ // acks and pending writes if an ack opened the congestion window. void MaybeSendInResponseToPacket(); - // Queue an ack or set the ack alarm if needed. |was_missing| is true if - // the most recently received packet was formerly missing. - void MaybeQueueAck(bool was_missing); - // Gets the least unacked packet number, which is the next packet number to be // sent if there are no outstanding packets. QuicPacketNumber GetLeastUnacked() const; @@ -1187,10 +1183,6 @@ EncryptionLevel last_decrypted_packet_level_; QuicPacketHeader last_header_; bool should_last_packet_instigate_acks_; - // Whether the most recent packet was missing before it was received. - // TODO(fayang): Remove was_last_packet_missing_ when deprecating - // quic_rpm_decides_when_to_send_acks. - bool was_last_packet_missing_; // Track some peer state so we can do less bookkeeping // Largest sequence sent by the peer which had an ack frame (latest ack info). @@ -1254,30 +1246,10 @@ // Used when use_uber_received_packet_manager_ is true. UberReceivedPacketManager uber_received_packet_manager_; - // How many retransmittable packets have arrived without sending an ack. - // TODO(fayang): Remove - // num_retransmittable_packets_received_since_last_ack_sent_ when deprecating - // quic_rpm_decides_when_to_send_acks. - QuicPacketCount num_retransmittable_packets_received_since_last_ack_sent_; - // How many consecutive packets have arrived without sending an ack. - QuicPacketCount num_packets_received_since_last_ack_sent_; // Indicates how many consecutive times an ack has arrived which indicates // the peer needs to stop waiting for some packets. // TODO(fayang): remove this when deprecating quic_simplify_stop_waiting. int stop_waiting_count_; - // TODO(fayang): Remove ack_mode_, ack_decimation_delay_, - // unlimited_ack_decimation_, fast_ack_after_quiescence_ when deprecating - // quic_rpm_decides_when_to_send_acks. - // Indicates the current ack mode, defaults to acking every 2 packets. - AckMode ack_mode_; - // 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 - // sending an ack. - bool unlimited_ack_decimation_; - // When true, use a 1ms delayed ack timer if it's been an SRTT since a packet - // was received. - bool fast_ack_after_quiescence_; // Indicates the retransmission alarm needs to be set. bool pending_retransmission_alarm_; @@ -1338,11 +1310,6 @@ // This is used for timeouts, and does not indicate the packet was processed. QuicTime time_of_last_received_packet_; - // The time the previous ack-instigating packet was received and processed. - // TODO(fayang): Remove time_of_previous_received_packet_ when deprecating - // quic_rpm_decides_when_to_send_acks. - QuicTime time_of_previous_received_packet_; - // Sent packet manager which tracks the status of packets sent by this // connection and contains the send and receive algorithms to determine when // to send packets. @@ -1423,16 +1390,6 @@ // from the peer. Default to kMaxConsecutiveNonRetransmittablePackets. size_t max_consecutive_num_packets_with_no_retransmittable_frames_; - // Ack decimation will start happening after this many packets are received. - // TODO(fayang): Remove min_received_before_ack_decimation_ when deprecating - // quic_rpm_decides_when_to_send_acks. - size_t min_received_before_ack_decimation_; - - // Before ack decimation starts (if enabled), we ack every n-th packet. - // TODO(fayang): Remove ack_frequency_before_ack_decimation_ when deprecating - // quic_rpm_decides_when_to_send_acks. - size_t ack_frequency_before_ack_decimation_; - // If true, the connection will fill up the pipe with extra data whenever the // congestion controller needs it in order to make a bandwidth estimate. This // is useful if the application pesistently underutilizes the link, but still @@ -1488,20 +1445,13 @@ // vector to improve performance since it is expected to be very small. std::vector<QuicConnectionId> incoming_connection_ids_; - // Indicates whether an ACK needs to be sent in OnCanWrite(). - // TODO(fayang): Remove this when ACK sending logic is moved to received - // packet manager, and an ACK timeout would be used to record when an ACK - // needs to be sent. - bool send_ack_when_on_can_write_; - // Indicates whether a RETRY packet has been parsed. bool retry_has_been_parsed_; // Latched value of quic_validate_packet_number_post_decryption. const bool validate_packet_number_post_decryption_; - // Latched value of quic_rpm_decides_when_to_send_acks and - // quic_use_uber_received_packet_manager. + // Latched value of quic_use_uber_received_packet_manager. const bool use_uber_received_packet_manager_; };
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc index e324c38..39e4a92 100644 --- a/quic/core/quic_received_packet_manager.cc +++ b/quic/core/quic_received_packet_manager.cc
@@ -60,15 +60,16 @@ fast_ack_after_quiescence_(false), ack_timeout_(QuicTime::Zero()), time_of_previous_received_packet_(QuicTime::Zero()), - was_last_packet_missing_(false), - decide_when_to_send_acks_( - GetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks)) {} + was_last_packet_missing_(false) { + if (ack_mode_ == ACK_DECIMATION) { + QUIC_RELOADABLE_FLAG_COUNT(quic_enable_ack_decimation); + } +} QuicReceivedPacketManager::~QuicReceivedPacketManager() {} void QuicReceivedPacketManager::SetFromConfig(const QuicConfig& config, Perspective perspective) { - DCHECK(decide_when_to_send_acks_); if (GetQuicReloadableFlag(quic_enable_ack_decimation) && config.HasClientSentConnectionOption(kACD0, perspective)) { ack_mode_ = TCP_ACKING; @@ -100,9 +101,7 @@ QuicTime receipt_time) { const QuicPacketNumber packet_number = header.packet_number; DCHECK(IsAwaitingPacket(packet_number)) << " packet_number:" << packet_number; - if (decide_when_to_send_acks_) { - was_last_packet_missing_ = IsMissing(packet_number); - } + was_last_packet_missing_ = IsMissing(packet_number); if (!ack_frame_updated_) { ack_frame_.received_packet_times.clear(); } @@ -163,9 +162,6 @@ const QuicFrame QuicReceivedPacketManager::GetUpdatedAckFrame( QuicTime approximate_now) { - if (!decide_when_to_send_acks_) { - ack_frame_updated_ = false; - } if (time_largest_observed_ == QuicTime::Zero()) { // We have received no packets. ack_frame_.ack_delay_time = QuicTime::Delta::Infinite(); @@ -224,7 +220,6 @@ QuicTime now, const RttStats* rtt_stats, QuicTime::Delta delayed_ack_time) { - DCHECK(decide_when_to_send_acks_); if (!ack_frame_updated_) { // ACK frame has not been updated, nothing to do. return; @@ -299,7 +294,6 @@ } void QuicReceivedPacketManager::ResetAckStates() { - DCHECK(decide_when_to_send_acks_); ack_frame_updated_ = false; ack_timeout_ = QuicTime::Zero(); num_retransmittable_packets_received_since_last_ack_sent_ = 0; @@ -307,7 +301,6 @@ } void QuicReceivedPacketManager::MaybeUpdateAckTimeoutTo(QuicTime time) { - DCHECK(decide_when_to_send_acks_); if (!ack_timeout_.IsInitialized() || ack_timeout_ > time) { ack_timeout_ = time; }
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h index 0a04f63..c84054c 100644 --- a/quic/core/quic_received_packet_manager.h +++ b/quic/core/quic_received_packet_manager.h
@@ -121,8 +121,6 @@ QuicTime ack_timeout() const { return ack_timeout_; } - bool decide_when_to_send_acks() const { return decide_when_to_send_acks_; } - private: friend class test::QuicConnectionPeer; friend class test::QuicReceivedPacketManagerPeer; @@ -185,9 +183,6 @@ // Last sent largest acked, which gets updated when ACK was successfully sent. QuicPacketNumber last_sent_largest_acked_; - - // Latched value of quic_rpm_decides_when_to_send_acks. - const bool decide_when_to_send_acks_; }; } // namespace quic
diff --git a/quic/core/quic_received_packet_manager_test.cc b/quic/core/quic_received_packet_manager_test.cc index a9cc387..86ac933 100644 --- a/quic/core/quic_received_packet_manager_test.cc +++ b/quic/core/quic_received_packet_manager_test.cc
@@ -82,13 +82,11 @@ } bool HasPendingAck() { - DCHECK(received_manager_.decide_when_to_send_acks()); return received_manager_.ack_timeout().IsInitialized(); } void MaybeUpdateAckTimeout(bool should_last_packet_instigate_acks, uint64_t last_received_packet_number) { - DCHECK(received_manager_.decide_when_to_send_acks()); received_manager_.MaybeUpdateAckTimeout( should_last_packet_instigate_acks, QuicPacketNumber(last_received_packet_number), clock_.ApproximateNow(), @@ -136,9 +134,7 @@ EXPECT_TRUE(received_manager_.ack_frame_updated()); QuicFrame ack = received_manager_.GetUpdatedAckFrame(QuicTime::Zero()); - if (received_manager_.decide_when_to_send_acks()) { - received_manager_.ResetAckStates(); - } + received_manager_.ResetAckStates(); EXPECT_FALSE(received_manager_.ack_frame_updated()); // When UpdateReceivedPacketInfo with a time earlier than the time of the // largest observed packet, make sure that the delta is 0, not negative. @@ -147,9 +143,7 @@ QuicTime four_ms = QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(4); ack = received_manager_.GetUpdatedAckFrame(four_ms); - if (received_manager_.decide_when_to_send_acks()) { - received_manager_.ResetAckStates(); - } + received_manager_.ResetAckStates(); EXPECT_FALSE(received_manager_.ack_frame_updated()); // When UpdateReceivedPacketInfo after not having received a new packet, // the delta should still be accurate. @@ -166,9 +160,7 @@ received_manager_.RecordPacketReceived(header, two_ms); EXPECT_TRUE(received_manager_.ack_frame_updated()); ack = received_manager_.GetUpdatedAckFrame(two_ms); - if (received_manager_.decide_when_to_send_acks()) { - received_manager_.ResetAckStates(); - } + received_manager_.ResetAckStates(); EXPECT_FALSE(received_manager_.ack_frame_updated()); // UpdateReceivedPacketInfo should discard any times which can't be // expressed on the wire. @@ -244,9 +236,6 @@ } TEST_P(QuicReceivedPacketManagerTest, OutOfOrderReceiptCausesAckSent) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); RecordPacketReceipt(3, clock_.ApproximateNow()); @@ -270,9 +259,6 @@ } TEST_P(QuicReceivedPacketManagerTest, OutOfOrderAckReceiptCausesNoAck) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); RecordPacketReceipt(2, clock_.ApproximateNow()); @@ -285,9 +271,6 @@ } TEST_P(QuicReceivedPacketManagerTest, AckReceiptCausesAckSend) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); RecordPacketReceipt(1, clock_.ApproximateNow()); @@ -315,9 +298,6 @@ } TEST_P(QuicReceivedPacketManagerTest, AckSentEveryNthPacket) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); received_manager_.set_ack_frequency_before_ack_decimation(3); @@ -334,9 +314,6 @@ } TEST_P(QuicReceivedPacketManagerTest, AckDecimationReducesAcks) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -372,9 +349,6 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAfterQuiescence) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, true); @@ -408,9 +382,6 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimation) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); // The ack time should be based on min_rtt * 1/4, since it's less than the @@ -444,9 +415,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckAckDecimationAfterQuiescence) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, @@ -514,9 +482,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationUnlimitedAggregation) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicConfig config; QuicTagVector connection_options; @@ -557,9 +522,6 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationEighthRtt) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); QuicReceivedPacketManagerPeer::SetAckDecimationDelay(&received_manager_, @@ -595,9 +557,6 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithReordering) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -641,9 +600,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReordering) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -689,9 +645,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithReorderingEighthRtt) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -733,9 +686,6 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReorderingEighthRtt) { - if (!received_manager_.decide_when_to_send_acks()) { - return; - } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING);
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc index e2252a6..061e2cd 100644 --- a/quic/core/quic_versions.cc +++ b/quic/core/quic_versions.cc
@@ -430,7 +430,6 @@ // Enable necessary flags. SetQuicFlag(FLAGS_quic_supports_tls_handshake, true); SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 60); - SetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks, true); SetQuicReloadableFlag(quic_use_uber_loss_algorithm, true); SetQuicReloadableFlag(quic_use_uber_received_packet_manager, true); SetQuicReloadableFlag(quic_validate_packet_number_post_decryption, true);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc index 931db1f..64dbb17 100644 --- a/quic/core/uber_received_packet_manager_test.cc +++ b/quic/core/uber_received_packet_manager_test.cc
@@ -49,7 +49,6 @@ class UberReceivedPacketManagerTest : public QuicTest { protected: UberReceivedPacketManagerTest() { - SetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks, true); manager_ = QuicMakeUnique<UberReceivedPacketManager>(&stats_); clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1)); rtt_stats_.UpdateRtt(kMinRttMs, QuicTime::Delta::Zero(), QuicTime::Zero());
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc index 89e9466..ab62c23 100644 --- a/quic/test_tools/quic_connection_peer.cc +++ b/quic/test_tools/quic_connection_peer.cc
@@ -244,18 +244,13 @@ // static void QuicConnectionPeer::SetAckMode(QuicConnection* connection, AckMode ack_mode) { - if (connection->received_packet_manager_.decide_when_to_send_acks()) { - if (connection->use_uber_received_packet_manager_) { - for (auto& received_packet_manager : - connection->uber_received_packet_manager_ - .received_packet_managers_) { - received_packet_manager.ack_mode_ = ack_mode; - } - } else { - connection->received_packet_manager_.ack_mode_ = ack_mode; + if (connection->use_uber_received_packet_manager_) { + for (auto& received_packet_manager : + connection->uber_received_packet_manager_.received_packet_managers_) { + received_packet_manager.ack_mode_ = ack_mode; } } else { - connection->ack_mode_ = ack_mode; + connection->received_packet_manager_.ack_mode_ = ack_mode; } } @@ -263,39 +258,29 @@ void QuicConnectionPeer::SetFastAckAfterQuiescence( QuicConnection* connection, bool fast_ack_after_quiescence) { - if (connection->received_packet_manager_.decide_when_to_send_acks()) { - if (connection->use_uber_received_packet_manager_) { - for (auto& received_packet_manager : - connection->uber_received_packet_manager_ - .received_packet_managers_) { - received_packet_manager.fast_ack_after_quiescence_ = - fast_ack_after_quiescence; - } - } else { - connection->received_packet_manager_.fast_ack_after_quiescence_ = + if (connection->use_uber_received_packet_manager_) { + for (auto& received_packet_manager : + connection->uber_received_packet_manager_.received_packet_managers_) { + received_packet_manager.fast_ack_after_quiescence_ = fast_ack_after_quiescence; } } else { - connection->fast_ack_after_quiescence_ = fast_ack_after_quiescence; + connection->received_packet_manager_.fast_ack_after_quiescence_ = + fast_ack_after_quiescence; } } // static void QuicConnectionPeer::SetAckDecimationDelay(QuicConnection* connection, float ack_decimation_delay) { - if (connection->received_packet_manager_.decide_when_to_send_acks()) { - if (connection->use_uber_received_packet_manager_) { - for (auto& received_packet_manager : - connection->uber_received_packet_manager_ - .received_packet_managers_) { - received_packet_manager.ack_decimation_delay_ = ack_decimation_delay; - } - } else { - connection->received_packet_manager_.ack_decimation_delay_ = - ack_decimation_delay; + if (connection->use_uber_received_packet_manager_) { + for (auto& received_packet_manager : + connection->uber_received_packet_manager_.received_packet_managers_) { + received_packet_manager.ack_decimation_delay_ = ack_decimation_delay; } } else { - connection->ack_decimation_delay_ = ack_decimation_delay; + connection->received_packet_manager_.ack_decimation_delay_ = + ack_decimation_delay; } }