For quic loss detection, replace quicconnectionstats.total_loss_detection_time by quicconnectionstats. total_loss_detection_response_time to improve the accuracy of detection speed. protected by existing --gfe2_reloadable_flag_quic_enable_loss_detection_experiment_at_gfe. On skmobile-cjj5 this reduced the threshold value from 8 to 4.5: http://shortn/_vT0AbUjKU6. shift value did not change. PiperOrigin-RevId: 317739228 Change-Id: I091fabc50753c754c9377d79f7b5a77878d8fbc8
diff --git a/quic/core/congestion_control/general_loss_algorithm.cc b/quic/core/congestion_control/general_loss_algorithm.cc index eb0ed77..666e7ea 100644 --- a/quic/core/congestion_control/general_loss_algorithm.cc +++ b/quic/core/congestion_control/general_loss_algorithm.cc
@@ -12,6 +12,20 @@ namespace quic { +namespace { +float DetectionResponseTime(QuicTime::Delta rtt, + QuicTime send_time, + QuicTime detection_time) { + if (detection_time <= send_time || rtt.IsZero()) { + // Time skewed, assume a very fast detection where |detection_time| is + // |send_time| + |rtt|. + return 1.0; + } + 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), @@ -100,6 +114,8 @@ if (!skip_packet_threshold_detection && largest_newly_acked - packet_number >= reordering_threshold_) { packets_lost->push_back(LostPacket(packet_number, it->bytes_sent)); + detection_stats.total_loss_detection_response_time += + DetectionResponseTime(max_rtt, it->sent_time, time); continue; } @@ -118,6 +134,8 @@ break; } packets_lost->push_back(LostPacket(packet_number, it->bytes_sent)); + detection_stats.total_loss_detection_response_time += + DetectionResponseTime(max_rtt, it->sent_time, time); } if (!least_in_flight_.IsInitialized()) { // There is no in flight packet.
diff --git a/quic/core/congestion_control/loss_detection_interface.h b/quic/core/congestion_control/loss_detection_interface.h index d5e0190..6799de9 100644 --- a/quic/core/congestion_control/loss_detection_interface.h +++ b/quic/core/congestion_control/loss_detection_interface.h
@@ -30,6 +30,9 @@ // Maximum sequence reordering observed in newly acked packets. QuicPacketCount sent_packets_max_sequence_reordering = 0; QuicPacketCount sent_packets_num_borderline_time_reorderings = 0; + // Total detection response time for lost packets from this detection. + // See QuicConnectionStats for the definition of detection response time. + float total_loss_detection_response_time = 0.0; }; // Called when a new ack arrives or the loss alarm fires.
diff --git a/quic/core/congestion_control/uber_loss_algorithm.cc b/quic/core/congestion_control/uber_loss_algorithm.cc index 243f563..5debad6 100644 --- a/quic/core/congestion_control/uber_loss_algorithm.cc +++ b/quic/core/congestion_control/uber_loss_algorithm.cc
@@ -56,6 +56,8 @@ stats.sent_packets_max_sequence_reordering); overall_stats.sent_packets_num_borderline_time_reorderings += stats.sent_packets_num_borderline_time_reorderings; + overall_stats.total_loss_detection_response_time += + stats.total_loss_detection_response_time; } return overall_stats;
diff --git a/quic/core/quic_connection_stats.h b/quic/core/quic_connection_stats.h index 67220ae..d0211da 100644 --- a/quic/core/quic_connection_stats.h +++ b/quic/core/quic_connection_stats.h
@@ -47,9 +47,21 @@ QuicPacketCount packets_lost = 0; QuicPacketCount packet_spuriously_detected_lost = 0; - // The sum of the detection time of all lost packets. The detection time of a - // lost packet is defined as: T(detection) - T(send). - QuicTime::Delta total_loss_detection_time = QuicTime::Delta::Zero(); + // The sum of loss detection response times of all lost packets, in number of + // round trips. + // Given a packet detected as lost: + // T(S) T(1Rtt) T(D) + // |_________________________________|_______| + // Where + // T(S) is the time when the packet is sent. + // T(1Rtt) is one rtt after T(S), using the rtt at the time of detection. + // T(D) is the time of detection, i.e. when the packet is declared as lost. + // The loss detection response time is defined as + // (T(D) - T(S)) / (T(1Rtt) - T(S)) + // + // The average loss detection response time is this number divided by + // |packets_lost|. Smaller result means detection is faster. + float total_loss_detection_response_time = 0.0; // Number of times this connection went through the slow start phase. uint32_t slowstart_count = 0;
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 86628ef..4373013 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -978,14 +978,13 @@ stats_->sent_packets_num_borderline_time_reorderings += detection_stats.sent_packets_num_borderline_time_reorderings; + stats_->total_loss_detection_response_time += + detection_stats.total_loss_detection_response_time; + for (const LostPacket& packet : packets_lost_) { QuicTransmissionInfo* info = unacked_packets_.GetMutableTransmissionInfo(packet.packet_number); ++stats_->packets_lost; - if (time > info->sent_time) { - stats_->total_loss_detection_time = - stats_->total_loss_detection_time + (time - info->sent_time); - } if (debug_delegate_ != nullptr) { debug_delegate_->OnPacketLoss(packet.packet_number, info->encryption_level, LOSS_RETRANSMISSION,
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index 23aec1e..97d83f5 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -622,7 +622,7 @@ // know packet 2 is a spurious until it gets acked. EXPECT_EQ(1u, stats_.packets_spuriously_retransmitted); EXPECT_EQ(1u, stats_.packets_lost); - EXPECT_LT(QuicTime::Delta::Zero(), stats_.total_loss_detection_time); + EXPECT_LT(0.0, stats_.total_loss_detection_response_time); EXPECT_LE(1u, stats_.sent_packets_max_sequence_reordering); }