Combine two WriteOrBufferData to one while writing HEADERS frames.
Two writes can cause HEADERS frame header and payload be sent in separate stream frames, which is less efficient.
Protected by FLAGS_quic_reloadable_flag_quic_one_write_for_headers.
PiperOrigin-RevId: 479624606
diff --git a/quiche/quic/core/http/quic_spdy_stream.cc b/quiche/quic/core/http/quic_spdy_stream.cc
index 7b580be..cd5590e 100644
--- a/quiche/quic/core/http/quic_spdy_stream.cc
+++ b/quiche/quic/core/http/quic_spdy_stream.cc
@@ -1165,16 +1165,28 @@
send_buffer().stream_offset(),
send_buffer().stream_offset() + headers_frame_header.length());
- QUIC_DLOG(INFO) << ENDPOINT << "Stream " << id()
- << " is writing HEADERS frame header of length "
- << headers_frame_header.length();
- WriteOrBufferData(headers_frame_header, /* fin = */ false,
- /* ack_listener = */ nullptr);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_one_write_for_headers);
- QUIC_DLOG(INFO) << ENDPOINT << "Stream " << id()
- << " is writing HEADERS frame payload of length "
- << encoded_headers.length() << " with fin " << fin;
- WriteOrBufferData(encoded_headers, fin, nullptr);
+ QUIC_DLOG(INFO) << ENDPOINT << "Stream " << id()
+ << " is writing HEADERS frame header of length "
+ << headers_frame_header.length()
+ << ", and payload of length " << encoded_headers.length()
+ << " with fin " << fin;
+ WriteOrBufferData(absl::StrCat(headers_frame_header, encoded_headers), fin,
+ /*ack_listener=*/nullptr);
+ } else {
+ QUIC_DLOG(INFO) << ENDPOINT << "Stream " << id()
+ << " is writing HEADERS frame header of length "
+ << headers_frame_header.length();
+ WriteOrBufferData(headers_frame_header, /* fin = */ false,
+ /* ack_listener = */ nullptr);
+
+ QUIC_DLOG(INFO) << ENDPOINT << "Stream " << id()
+ << " is writing HEADERS frame payload of length "
+ << encoded_headers.length() << " with fin " << fin;
+ WriteOrBufferData(encoded_headers, fin, nullptr);
+ }
QuicSpdySession::LogHeaderCompressionRatioHistogram(
/* using_qpack = */ true,
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc
index c6cd92c..960060e 100644
--- a/quiche/quic/core/http/quic_spdy_stream_test.cc
+++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -1494,7 +1494,11 @@
// In this case, TestStream::WriteHeadersImpl() does not prevent writes.
// Four writes on the request stream: HEADERS frame header and payload both
// for headers and trailers.
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(4);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ } else {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(4);
+ }
}
// Write the initial headers, without a FIN.
@@ -1520,7 +1524,11 @@
// Four writes on the request stream: HEADERS frame header and payload both
// for headers and trailers.
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(4);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ } else {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(4);
+ }
// No PRIORITY_UPDATE frames on the control stream,
// because the stream has default priority.
@@ -1553,7 +1561,11 @@
session_->set_debug_visitor(&debug_visitor);
// Two writes on the request stream: HEADERS frame header and payload.
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(1);
+ } else {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ }
EXPECT_CALL(*stream_, WriteHeadersMock(false));
EXPECT_CALL(debug_visitor, OnHeadersFrameSent(stream_->id(), _));
stream_->WriteHeaders(Http2HeaderBlock(), /*fin=*/false, nullptr);
@@ -1587,7 +1599,11 @@
// Two writes on the request stream: HEADERS frame header and payload.
// PRIORITY_UPDATE frame is not sent this time, because one is already sent.
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(1);
+ } else {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ }
EXPECT_CALL(*stream_, WriteHeadersMock(true));
stream_->WriteHeaders(Http2HeaderBlock(), /*fin=*/true, nullptr);
}
@@ -1600,7 +1616,11 @@
if (UsesHttp3()) {
// In this case, TestStream::WriteHeadersImpl() does not prevent writes.
// HEADERS frame header and payload on the request stream.
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(1);
+ } else {
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _)).Times(2);
+ }
}
// Write the initial headers.
@@ -3002,23 +3022,35 @@
EXPECT_CALL(*session_, WritevData(encoder_stream->id(), _, _, _, _, _))
.Times(AnyNumber());
- // HEADERS frame header.
- EXPECT_CALL(*session_,
- WritevData(stream_->id(), _, /* offset = */ 0, _, _, _));
- // HEADERS frame payload.
- size_t headers_frame_payload_length = 0;
- EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _))
- .WillOnce(
- DoAll(SaveArg<1>(&headers_frame_payload_length),
- Invoke(session_.get(), &MockQuicSpdySession::ConsumeData)));
+ size_t bytes_written = 0;
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ EXPECT_CALL(*session_,
+ WritevData(stream_->id(), _, /* offset = */ 0, _, _, _))
+ .WillOnce(
+ DoAll(SaveArg<1>(&bytes_written),
+ Invoke(session_.get(), &MockQuicSpdySession::ConsumeData)));
+ } else {
+ // HEADERS frame header.
+ EXPECT_CALL(*session_,
+ WritevData(stream_->id(), _, /* offset = */ 0, _, _, _));
+ // HEADERS frame payload.
+ EXPECT_CALL(*session_, WritevData(stream_->id(), _, _, _, _, _))
+ .WillOnce(
+ DoAll(SaveArg<1>(&bytes_written),
+ Invoke(session_.get(), &MockQuicSpdySession::ConsumeData)));
+ }
Http2HeaderBlock request_headers;
request_headers["foo"] = "bar";
size_t write_headers_return_value =
stream_->WriteHeaders(std::move(request_headers), /*fin=*/true, nullptr);
EXPECT_TRUE(stream_->fin_sent());
-
- EXPECT_EQ(headers_frame_payload_length, write_headers_return_value);
+ if (GetQuicReloadableFlag(quic_one_write_for_headers)) {
+ // bytes_written includes HEADERS frame header.
+ EXPECT_GT(bytes_written, write_headers_return_value);
+ } else {
+ EXPECT_EQ(bytes_written, write_headers_return_value);
+ }
}
// Regression test for https://crbug.com/1177662.
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index 2cdef18..d8e8cd8 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -33,6 +33,8 @@
QUIC_FLAG(quic_reloadable_flag_quic_can_send_ack_frequency, true)
// If true, allow client to enable BBRv2 on server via connection option \'B2ON\'.
QUIC_FLAG(quic_reloadable_flag_quic_allow_client_enabled_bbr_v2, true)
+// If true, combine two WriteOrBufferData to one while writing headers.
+QUIC_FLAG(quic_reloadable_flag_quic_one_write_for_headers, true)
// If true, default-enable 5RTO blachole detection.
QUIC_FLAG(quic_reloadable_flag_quic_default_enable_5rto_blackhole_detection2, true)
// If true, disable QUIC version Q043.
