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 {
