For quic loss detection tuning, add a new reward function using average http latency and run it side by side with the existing reward function. protected by --gfe2_reloadable_flag_quic_enable_loss_detection_experiment_at_gfe.
PiperOrigin-RevId: 319825747
Change-Id: Ic990510ce43f71990edb1ecc7180b8459e706173
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 0888a0e..e3dab64 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -2631,9 +2631,11 @@
public:
TestAckListener() {}
- void OnPacketAcked(int acked_bytes,
- QuicTime::Delta /*delta_largest_observed*/) override {
+ QuicTime::Delta OnPacketAcked(
+ int acked_bytes,
+ QuicTime::Delta /*delta_largest_observed*/) override {
total_bytes_acked_ += acked_bytes;
+ return QuicTime::Delta::Zero();
}
void OnPacketRetransmitted(int /*retransmitted_bytes*/) override {}
diff --git a/quic/core/http/quic_headers_stream.cc b/quic/core/http/quic_headers_stream.cc
index 8157dae..7fc10dc 100644
--- a/quic/core/http/quic_headers_stream.cc
+++ b/quic/core/http/quic_headers_stream.cc
@@ -91,7 +91,11 @@
return false;
}
if (header.ack_listener != nullptr && header_length > 0) {
- header.ack_listener->OnPacketAcked(header_length, ack_delay_time);
+ QuicTime::Delta response_time =
+ header.ack_listener->OnPacketAcked(header_length, ack_delay_time);
+ if (!response_time.IsZero()) {
+ spdy_session_->RecordServerResponseTime(response_time);
+ }
}
header.unacked_length -= header_length;
acked_offset += header_length;
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index d6b5a0e..f470322 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -1368,6 +1368,14 @@
quiche::QuicheStrCat(type, " stream is received twice."));
}
+void QuicSpdySession::RecordServerResponseTime(QuicTime::Delta response_time) {
+ DCHECK_EQ(perspective(), Perspective::IS_SERVER);
+ QuicConnectionStats& stats = connection()->mutable_stats();
+
+ ++stats.num_responses;
+ stats.total_response_time = stats.total_response_time + response_time;
+}
+
// static
void QuicSpdySession::LogHeaderCompressionRatioHistogram(
bool using_qpack,
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index 4d6ee63..3773b88 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -22,6 +22,7 @@
#include "net/third_party/quiche/src/quic/core/qpack/qpack_receive_stream.h"
#include "net/third_party/quiche/src/quic/core/qpack/qpack_send_stream.h"
#include "net/third_party/quiche/src/quic/core/quic_session.h"
+#include "net/third_party/quiche/src/quic/core/quic_time.h"
#include "net/third_party/quiche/src/quic/core/quic_versions.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_optional.h"
@@ -384,6 +385,9 @@
// Decode SETTINGS from |cached_state| and apply it to the session.
bool ResumeApplicationState(ApplicationState* cached_state) override;
+ // (Server only) Records the response time of a completed request.
+ void RecordServerResponseTime(QuicTime::Delta response_time);
+
protected:
// Override CreateIncomingStream(), CreateOutgoingBidirectionalStream() and
// CreateOutgoingUnidirectionalStream() with QuicSpdyStream return type to
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index 4c979c1..c80ecc9 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -919,8 +919,11 @@
DCHECK_LE(newly_acked_header_length, *newly_acked_length);
unacked_frame_headers_offsets_.Difference(offset, offset + data_length);
if (ack_listener_ != nullptr && new_data_acked) {
- ack_listener_->OnPacketAcked(
+ QuicTime::Delta response_time = ack_listener_->OnPacketAcked(
*newly_acked_length - newly_acked_header_length, ack_delay_time);
+ if (!response_time.IsZero()) {
+ spdy_session_->RecordServerResponseTime(response_time);
+ }
}
return new_data_acked;
}
diff --git a/quic/core/quic_ack_listener_interface.h b/quic/core/quic_ack_listener_interface.h
index 0a9c694..873015a 100644
--- a/quic/core/quic_ack_listener_interface.h
+++ b/quic/core/quic_ack_listener_interface.h
@@ -20,8 +20,10 @@
// Called when a packet is acked. Called once per packet.
// |acked_bytes| is the number of data bytes acked.
- virtual void OnPacketAcked(int acked_bytes,
- QuicTime::Delta ack_delay_time) = 0;
+ // Return the http response time if this is the last ack of a server stream.
+ // Otherwise return QuicTime::Delta::Zero().
+ virtual QuicTime::Delta OnPacketAcked(int acked_bytes,
+ QuicTime::Delta ack_delay_time) = 0;
// Called when a packet is retransmitted. Called once per packet.
// |retransmitted_bytes| is the number of data bytes retransmitted.
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 4cb08c5..69f00e4 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -3449,8 +3449,11 @@
FlushPackets();
connected_ = false;
DCHECK(visitor_ != nullptr);
- sent_packet_manager_.OnConnectionClosed();
visitor_->OnConnectionClosed(frame, source);
+ // LossDetectionTunerInterface::Finish() may be called from
+ // sent_packet_manager_.OnConnectionClosed. Which may require the session to
+ // finish its business first.
+ sent_packet_manager_.OnConnectionClosed();
if (debug_visitor_ != nullptr) {
debug_visitor_->OnConnectionClosed(frame, source);
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index dc2a00f..b4f6445 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -466,6 +466,8 @@
const std::string& details,
ConnectionCloseBehavior connection_close_behavior);
+ QuicConnectionStats& mutable_stats() { return stats_; }
+
// Returns statistics tracked for this connection.
const QuicConnectionStats& GetStats();
@@ -850,6 +852,7 @@
bool ack_frame_updated() const;
QuicConnectionHelperInterface* helper() { return helper_; }
+ const QuicConnectionHelperInterface* helper() const { return helper_; }
QuicAlarmFactory* alarm_factory() { return alarm_factory_; }
quiche::QuicheStringPiece GetCurrentPacket();
diff --git a/quic/core/quic_connection_stats.h b/quic/core/quic_connection_stats.h
index 3579be0..529a4ef 100644
--- a/quic/core/quic_connection_stats.h
+++ b/quic/core/quic_connection_stats.h
@@ -162,6 +162,15 @@
// Number of sent packets that were encapsulated using Legacy Version
// Encapsulation.
QuicPacketCount sent_legacy_version_encapsulated_packets = 0;
+
+ // HTTP server metrics.
+ // Number of success streams. i.e. streams in which all data are acked and
+ // have no stream error.
+ uint64_t num_responses = 0;
+ // Sum of response times for all success streams. A stream's response time is
+ // the time between stream creation to the time its last ack is received,
+ // minus the peer ack delay in the last ack.
+ QuicTime::Delta total_response_time = QuicTime::Delta::Zero();
};
} // namespace quic
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index c936767..706eb64 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -481,6 +481,10 @@
connection()->OnUserAgentIdKnown();
}
+ const QuicClock* GetClock() const {
+ return connection()->helper()->GetClock();
+ }
+
protected:
using StreamMap = QuicSmallMap<QuicStreamId, std::unique_ptr<QuicStream>, 10>;
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index 0dba17b..32a8450 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -835,7 +835,10 @@
MockLossAlgorithm::~MockLossAlgorithm() {}
-MockAckListener::MockAckListener() {}
+MockAckListener::MockAckListener() {
+ ON_CALL(*this, OnPacketAcked(_, _))
+ .WillByDefault(testing::Return(QuicTime::Delta::Zero()));
+}
MockAckListener::~MockAckListener() {}
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 650213e..7be7edf 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -1320,7 +1320,7 @@
MockAckListener(const MockAckListener&) = delete;
MockAckListener& operator=(const MockAckListener&) = delete;
- MOCK_METHOD(void,
+ MOCK_METHOD(QuicTime::Delta,
OnPacketAcked,
(int acked_bytes, QuicTime::Delta ack_delay_time),
(override));
