Delay the start of loss detection tuner to the time when the first reordering happens. protected by existing --gfe2_reloadable_flag_quic_enable_loss_detection_tuner. PiperOrigin-RevId: 320984808 Change-Id: Ife4abd76b8d66f1a2af3302cbac5d961fad08dcc
diff --git a/quic/core/congestion_control/general_loss_algorithm.cc b/quic/core/congestion_control/general_loss_algorithm.cc index 666e7ea..504a163 100644 --- a/quic/core/congestion_control/general_loss_algorithm.cc +++ b/quic/core/congestion_control/general_loss_algorithm.cc
@@ -24,17 +24,13 @@ float send_to_detection_us = (detection_time - send_time).ToMicroseconds(); return send_to_detection_us / rtt.ToMicroseconds(); } -} // namespace -GeneralLossAlgorithm::GeneralLossAlgorithm() - : loss_detection_timeout_(QuicTime::Zero()), - reordering_shift_(kDefaultLossDelayShift), - reordering_threshold_(kDefaultPacketReorderingThreshold), - use_adaptive_reordering_threshold_(true), - use_adaptive_time_threshold_(false), - use_packet_threshold_for_runt_packets_(true), - least_in_flight_(1), - packet_number_space_(NUM_PACKET_NUMBER_SPACES) {} +QuicTime::Delta GetMaxRtt(const RttStats& rtt_stats) { + return std::max(kAlarmGranularity, + std::max(rtt_stats.previous_srtt(), rtt_stats.latest_rtt())); +} + +} // namespace // Uses nack counts to decide when packets are lost. LossDetectionInterface::DetectionStats GeneralLossAlgorithm::DetectLosses( @@ -67,10 +63,8 @@ } } - QuicTime::Delta max_rtt = - std::max(rtt_stats.previous_srtt(), rtt_stats.latest_rtt()); - max_rtt = std::max(kAlarmGranularity, max_rtt); - const QuicTime::Delta loss_delay = max_rtt + (max_rtt >> reordering_shift_); + const QuicTime::Delta max_rtt = GetMaxRtt(rtt_stats); + QuicPacketNumber packet_number = unacked_packets.GetLeastUnacked(); auto it = unacked_packets.begin(); if (least_in_flight_.IsInitialized() && least_in_flight_ >= packet_number) { @@ -99,6 +93,10 @@ continue; } + if (parent_ != nullptr && largest_newly_acked != packet_number) { + parent_->OnReorderingDetected(); + } + if (largest_newly_acked - packet_number > detection_stats.sent_packets_max_sequence_reordering) { detection_stats.sent_packets_max_sequence_reordering = @@ -120,6 +118,7 @@ } // Time threshold loss detection. + const QuicTime::Delta loss_delay = max_rtt + (max_rtt >> reordering_shift_); QuicTime when_lost = it->sent_time + loss_delay; if (time < when_lost) { if (time >= @@ -178,8 +177,9 @@ } } -void GeneralLossAlgorithm::SetPacketNumberSpace( - PacketNumberSpace packet_number_space) { +void GeneralLossAlgorithm::Initialize(PacketNumberSpace packet_number_space, + LossDetectionInterface* parent) { + parent_ = parent; if (packet_number_space_ < NUM_PACKET_NUMBER_SPACES) { QUIC_BUG << "Cannot switch packet_number_space"; return;
diff --git a/quic/core/congestion_control/general_loss_algorithm.h b/quic/core/congestion_control/general_loss_algorithm.h index 950831e..b2ec008 100644 --- a/quic/core/congestion_control/general_loss_algorithm.h +++ b/quic/core/congestion_control/general_loss_algorithm.h
@@ -22,7 +22,7 @@ // Also implements TCP's early retransmit(RFC5827). class QUIC_EXPORT_PRIVATE GeneralLossAlgorithm : public LossDetectionInterface { public: - GeneralLossAlgorithm(); + GeneralLossAlgorithm() = default; GeneralLossAlgorithm(const GeneralLossAlgorithm&) = delete; GeneralLossAlgorithm& operator=(const GeneralLossAlgorithm&) = delete; ~GeneralLossAlgorithm() override {} @@ -68,7 +68,13 @@ << "Unexpected call to GeneralLossAlgorithm::OnConnectionClosed"; } - void SetPacketNumberSpace(PacketNumberSpace packet_number_space); + void OnReorderingDetected() override { + DCHECK(false) + << "Unexpected call to GeneralLossAlgorithm::OnReorderingDetected"; + } + + void Initialize(PacketNumberSpace packet_number_space, + LossDetectionInterface* parent); void Reset(); @@ -107,23 +113,24 @@ } private: - QuicTime loss_detection_timeout_; + LossDetectionInterface* parent_ = nullptr; + QuicTime loss_detection_timeout_ = QuicTime::Zero(); // Fraction of a max(SRTT, latest_rtt) to permit reordering before declaring // loss. Fraction calculated by shifting max(SRTT, latest_rtt) to the right // by reordering_shift. - int reordering_shift_; + int reordering_shift_ = kDefaultLossDelayShift; // Reordering threshold for loss detection. - QuicPacketCount reordering_threshold_; + QuicPacketCount reordering_threshold_ = kDefaultPacketReorderingThreshold; // If true, uses adaptive reordering threshold for loss detection. - bool use_adaptive_reordering_threshold_; + bool use_adaptive_reordering_threshold_ = true; // If true, uses adaptive time threshold for time based loss detection. - bool use_adaptive_time_threshold_; + bool use_adaptive_time_threshold_ = false; // If true, uses packet threshold when largest acked is a runt packet. - bool use_packet_threshold_for_runt_packets_; + bool use_packet_threshold_for_runt_packets_ = true; // The least in flight packet. Loss detection should start from this. Please // note, least_in_flight_ could be largest packet ever sent + 1. - QuicPacketNumber least_in_flight_; - PacketNumberSpace packet_number_space_; + QuicPacketNumber least_in_flight_{1}; + PacketNumberSpace packet_number_space_ = NUM_PACKET_NUMBER_SPACES; }; } // namespace quic
diff --git a/quic/core/congestion_control/general_loss_algorithm_test.cc b/quic/core/congestion_control/general_loss_algorithm_test.cc index be33c25..9124e49 100644 --- a/quic/core/congestion_control/general_loss_algorithm_test.cc +++ b/quic/core/congestion_control/general_loss_algorithm_test.cc
@@ -27,7 +27,7 @@ rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(100), QuicTime::Delta::Zero(), clock_.Now()); EXPECT_LT(0, rtt_stats_.smoothed_rtt().ToMicroseconds()); - loss_algorithm_.SetPacketNumberSpace(HANDSHAKE_DATA); + loss_algorithm_.Initialize(HANDSHAKE_DATA, nullptr); } ~GeneralLossAlgorithmTest() override {}
diff --git a/quic/core/congestion_control/loss_detection_interface.h b/quic/core/congestion_control/loss_detection_interface.h index 6799de9..8e7bbc3 100644 --- a/quic/core/congestion_control/loss_detection_interface.h +++ b/quic/core/congestion_control/loss_detection_interface.h
@@ -63,6 +63,11 @@ virtual void OnUserAgentIdKnown() = 0; virtual void OnConnectionClosed() = 0; + + // Called when a reordering is detected by the loss algorithm, but _before_ + // the reordering_shift and reordering_threshold are consulted to see whether + // it is a loss. + virtual void OnReorderingDetected() = 0; }; } // namespace quic
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc index 5debad6..986478b 100644 --- a/quic/core/congestion_control/uber_loss_algorithm.cc +++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -13,8 +13,8 @@ UberLossAlgorithm::UberLossAlgorithm() { for (int8_t i = INITIAL_DATA; i < NUM_PACKET_NUMBER_SPACES; ++i) { - general_loss_algorithms_[i].SetPacketNumberSpace( - static_cast<PacketNumberSpace>(i)); + general_loss_algorithms_[i].Initialize(static_cast<PacketNumberSpace>(i), + this); } } @@ -101,7 +101,7 @@ void UberLossAlgorithm::MaybeStartTuning() { if (tuner_started_ || !tuning_enabled_ || !min_rtt_available_ || - !user_agent_known_) { + !user_agent_known_ || !reorder_happened_) { return; } @@ -141,6 +141,22 @@ } } +void UberLossAlgorithm::OnReorderingDetected() { + const bool tuner_started_before = tuner_started_; + const bool reorder_happened_before = reorder_happened_; + + reorder_happened_ = true; + MaybeStartTuning(); + + if (!tuner_started_before && tuner_started_) { + if (reorder_happened_before) { + QUIC_CODE_COUNT(quic_loss_tuner_started_after_first_reorder); + } else { + QUIC_CODE_COUNT(quic_loss_tuner_started_on_first_reorder); + } + } +} + void UberLossAlgorithm::SetReorderingShift(int reordering_shift) { for (int8_t i = INITIAL_DATA; i < NUM_PACKET_NUMBER_SPACES; ++i) { general_loss_algorithms_[i].set_reordering_shift(reordering_shift);
diff --git a/quic/core/congestion_control/uber_loss_algorithm.h b/quic/core/congestion_control/uber_loss_algorithm.h index 0d1453c..a2c8887 100644 --- a/quic/core/congestion_control/uber_loss_algorithm.h +++ b/quic/core/congestion_control/uber_loss_algorithm.h
@@ -75,6 +75,7 @@ void OnMinRttAvailable() override; void OnUserAgentIdKnown() override; void OnConnectionClosed() override; + void OnReorderingDetected() override; // Sets reordering_shift for all packet number spaces. void SetReorderingShift(int reordering_shift); @@ -132,6 +133,7 @@ bool user_agent_known_ = !GetQuicReloadableFlag(quic_save_user_agent_in_quic_session); bool tuning_enabled_ = false; // Whether tuning is enabled by config. + bool reorder_happened_ = false; // Whether any reordered packet is observed. }; } // namespace quic
diff --git a/quic/core/congestion_control/uber_loss_algorithm_test.cc b/quic/core/congestion_control/uber_loss_algorithm_test.cc index e663540..458152f 100644 --- a/quic/core/congestion_control/uber_loss_algorithm_test.cc +++ b/quic/core/congestion_control/uber_loss_algorithm_test.cc
@@ -262,8 +262,12 @@ EXPECT_EQ(old_reordering_threshold, loss_algorithm_.GetPacketReorderingThreshold()); - // Tuning should start when MinRtt becomes available. + // MinRtt available. Tuner should not start yet because no reordering yet. loss_algorithm_.OnMinRttAvailable(); + EXPECT_FALSE(test_tuner->start_called()); + + // Reordering happened. Tuner should start now. + loss_algorithm_.OnReorderingDetected(); EXPECT_TRUE(test_tuner->start_called()); EXPECT_NE(old_reordering_shift, loss_algorithm_.GetPacketReorderingShift()); EXPECT_NE(old_reordering_threshold, @@ -294,6 +298,10 @@ EXPECT_EQ(old_reordering_threshold, loss_algorithm_.GetPacketReorderingThreshold()); + // Pretend a reodering has happened. + loss_algorithm_.OnReorderingDetected(); + EXPECT_FALSE(test_tuner->start_called()); + QuicConfig config; QuicTagVector connection_options; connection_options.push_back(kELDT); @@ -336,6 +344,10 @@ EXPECT_EQ(old_reordering_threshold, loss_algorithm_.GetPacketReorderingThreshold()); + // Pretend a reodering has happened. + loss_algorithm_.OnReorderingDetected(); + EXPECT_FALSE(test_tuner->start_called()); + // Parameters should not change since test_tuner->Start() returns false. loss_algorithm_.OnMinRttAvailable(); EXPECT_TRUE(test_tuner->start_called());
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 870f99d..2b483b5 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1317,6 +1317,7 @@ MOCK_METHOD(void, OnMinRttAvailable, (), (override)); MOCK_METHOD(void, OnUserAgentIdKnown, (), (override)); MOCK_METHOD(void, OnConnectionClosed, (), (override)); + MOCK_METHOD(void, OnReorderingDetected, (), (override)); }; class MockAckListener : public QuicAckListenerInterface {