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;
}
}
