Remove ack_decimation_with_reordering mode, fast_ack_after_quiescence option, and related connection options in quic received packet manager. protected by gfe2_reloadable_flag_quic_remove_unused_ack_options. This is to simplify ack frequency related code before we can use AckFrequencyFrame. PiperOrigin-RevId: 322820013 Change-Id: I9635c5ef1bcf0789c3180b2c93a1ef69f50aac9f
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 060a971..bf61ade 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -6220,6 +6220,9 @@ } TEST_P(QuicConnectionTest, SendDelayedAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } QuicConnectionPeer::SetFastAckAfterQuiescence(&connection_, true); // The beginning of the connection counts as quiescence. @@ -6352,6 +6355,9 @@ } TEST_P(QuicConnectionTest, SendDelayedAckAckDecimationAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION); QuicConnectionPeer::SetFastAckAfterQuiescence(&connection_, true); @@ -6603,6 +6609,9 @@ } TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); @@ -6668,6 +6677,9 @@ } TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithLargeReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); @@ -6751,6 +6763,9 @@ } TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); QuicConnectionPeer::SetAckDecimationDelay(&connection_, 0.125); @@ -6820,6 +6835,9 @@ TEST_P(QuicConnectionTest, SendDelayedAckDecimationWithLargeReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_CALL(visitor_, OnAckNeedsRetransmittableFrame()).Times(AnyNumber()); QuicConnectionPeer::SetAckMode(&connection_, ACK_DECIMATION_WITH_REORDERING); QuicConnectionPeer::SetAckDecimationDelay(&connection_, 0.125);
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc index 942c8b5..324767c 100644 --- a/quic/core/quic_received_packet_manager.cc +++ b/quic/core/quic_received_packet_manager.cc
@@ -60,28 +60,39 @@ void QuicReceivedPacketManager::SetFromConfig(const QuicConfig& config, Perspective perspective) { - if (config.HasClientSentConnectionOption(kACD0, perspective)) { - ack_mode_ = TCP_ACKING; + if (remove_unused_ack_options_) { + QUIC_RELOADABLE_FLAG_COUNT(quic_remove_unused_ack_options); } - if (config.HasClientSentConnectionOption(kACKD, perspective)) { - ack_mode_ = ACK_DECIMATION; - } - if (config.HasClientSentConnectionOption(kAKD2, perspective)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; + if (!remove_unused_ack_options_) { + if (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; + if (!remove_unused_ack_options_) { + ack_mode_ = ACK_DECIMATION; + } ack_decimation_delay_ = kShortAckDecimationDelay; } - if (config.HasClientSentConnectionOption(kAKD4, perspective)) { - ack_mode_ = ACK_DECIMATION_WITH_REORDERING; - ack_decimation_delay_ = kShortAckDecimationDelay; + if (!remove_unused_ack_options_) { + 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; + if (!remove_unused_ack_options_) { + if (config.HasClientSentConnectionOption(kACKQ, perspective)) { + fast_ack_after_quiescence_ = true; + } } if (config.HasClientSentConnectionOption(k1ACK, perspective)) { one_immediate_ack_ = true; @@ -274,7 +285,10 @@ // If there are new missing packets to report, send an ack immediately. if (HasNewMissingPackets()) { + // TODO(haoyuewang) Remove ACK_DECIMATION_WITH_REORDERING after + // quic_remove_unused_ack_options is deprecated. if (ack_mode_ == ACK_DECIMATION_WITH_REORDERING) { + DCHECK(!remove_unused_ack_options_); // Wait the minimum of an eighth min_rtt and the existing ack time. QuicTime ack_time = now + kShortAckDecimationDelay * rtt_stats->min_rtt(); MaybeUpdateAckTimeoutTo(ack_time);
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h index b01390d..a350462 100644 --- a/quic/core/quic_received_packet_manager.h +++ b/quic/core/quic_received_packet_manager.h
@@ -177,6 +177,8 @@ bool unlimited_ack_decimation_; // When true, use a 1ms delayed ack timer if it's been an SRTT since a packet // was received. + // TODO(haoyuewang) Remove fast_ack_after_quiescence_ when + // quic_remove_unused_ack_options flag is deprecated. bool fast_ack_after_quiescence_; // When true, only send 1 immediate ACK when reordering is detected. bool one_immediate_ack_; @@ -193,6 +195,11 @@ // Whether the most recent packet was missing before it was received. bool was_last_packet_missing_; + // TODO(haoyuewang) Remove TCP_ACKING when + // fast_ack_after_quiescence_ when this flag is deprecated. + const bool remove_unused_ack_options_ = + GetQuicReloadableFlag(quic_remove_unused_ack_options); + // 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 bfdf338..56b6643 100644 --- a/quic/core/quic_received_packet_manager_test.cc +++ b/quic/core/quic_received_packet_manager_test.cc
@@ -402,6 +402,9 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, true); @@ -505,6 +508,9 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckAckDecimationAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION); QuicReceivedPacketManagerPeer::SetFastAckAfterQuiescence(&received_manager_, @@ -647,6 +653,9 @@ } TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -690,6 +699,9 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -735,6 +747,9 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING); @@ -776,6 +791,9 @@ TEST_P(QuicReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); QuicReceivedPacketManagerPeer::SetAckMode(&received_manager_, ACK_DECIMATION_WITH_REORDERING);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc index a2fb640..41dadcb 100644 --- a/quic/core/uber_received_packet_manager_test.cc +++ b/quic/core/uber_received_packet_manager_test.cc
@@ -349,6 +349,9 @@ } TEST_F(UberReceivedPacketManagerTest, SendDelayedAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetFastAckAfterQuiescence(manager_.get(), true); @@ -429,6 +432,9 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckAckDecimationAfterQuiescence) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION); UberReceivedPacketManagerPeer::SetFastAckAfterQuiescence(manager_.get(), @@ -570,6 +576,9 @@ } TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationWithReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION_WITH_REORDERING); @@ -613,6 +622,9 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReordering) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION_WITH_REORDERING); @@ -658,6 +670,9 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationWithReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION_WITH_REORDERING); @@ -698,6 +713,9 @@ TEST_F(UberReceivedPacketManagerTest, SendDelayedAckDecimationWithLargeReorderingEighthRtt) { + if (GetQuicReloadableFlag(quic_remove_unused_ack_options)) { + return; + } EXPECT_FALSE(HasPendingAck()); UberReceivedPacketManagerPeer::SetAckMode(manager_.get(), ACK_DECIMATION_WITH_REORDERING);