Make QpackDecoderStreamSender buffer serialized instructions.
QpackEncoderStreamSender has already been modified the same way at cr/273553497.
This way Header Acknowledgement and Insert Count Increment emitted from a single
QpackDecoder::OnDecodingCompleted() call can be coalesced into a single write
without any copying.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 274280271
Change-Id: I8152636818ef2c0dbbd3b95355c5280cebcd04c6
diff --git a/quic/core/qpack/qpack_decoder.cc b/quic/core/qpack/qpack_decoder.cc
index 1f64945..4f39e3b 100644
--- a/quic/core/qpack/qpack_decoder.cc
+++ b/quic/core/qpack/qpack_decoder.cc
@@ -30,6 +30,7 @@
// TODO(bnc): SendStreamCancellation should not be called if maximum dynamic
// table capacity is zero.
decoder_stream_sender_.SendStreamCancellation(stream_id);
+ decoder_stream_sender_.Flush();
}
bool QpackDecoder::OnStreamBlocked(QuicStreamId stream_id) {
@@ -62,6 +63,8 @@
header_table_.inserted_entry_count() - known_received_count_);
known_received_count_ = header_table_.inserted_entry_count();
}
+
+ decoder_stream_sender_.Flush();
}
void QpackDecoder::OnInsertWithNameReference(bool is_static,
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.cc b/quic/core/qpack/qpack_decoder_stream_sender.cc
index 7f3f346..68e4d67 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_decoder_stream_sender.cc
@@ -18,29 +18,32 @@
void QpackDecoderStreamSender::SendInsertCountIncrement(uint64_t increment) {
values_.varint = increment;
- std::string output;
instruction_encoder_.Encode(InsertCountIncrementInstruction(), values_,
- &output);
- delegate_->WriteStreamData(output);
+ &buffer_);
}
void QpackDecoderStreamSender::SendHeaderAcknowledgement(
QuicStreamId stream_id) {
values_.varint = stream_id;
- std::string output;
instruction_encoder_.Encode(HeaderAcknowledgementInstruction(), values_,
- &output);
- delegate_->WriteStreamData(output);
+ &buffer_);
}
void QpackDecoderStreamSender::SendStreamCancellation(QuicStreamId stream_id) {
values_.varint = stream_id;
- std::string output;
instruction_encoder_.Encode(StreamCancellationInstruction(), values_,
- &output);
- delegate_->WriteStreamData(output);
+ &buffer_);
+}
+
+void QpackDecoderStreamSender::Flush() {
+ if (buffer_.empty()) {
+ return;
+ }
+
+ delegate_->WriteStreamData(buffer_);
+ buffer_.clear();
}
} // namespace quic
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.h b/quic/core/qpack/qpack_decoder_stream_sender.h
index 6bf466f..93d95d9 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.h
+++ b/quic/core/qpack/qpack_decoder_stream_sender.h
@@ -15,15 +15,15 @@
namespace quic {
-// This class serializes (encodes) instructions for transmission on the decoder
-// stream.
+// This class serializes instructions for transmission on the decoder stream.
+// Serialized instructions are buffered until Flush() is called.
class QUIC_EXPORT_PRIVATE QpackDecoderStreamSender {
public:
QpackDecoderStreamSender();
QpackDecoderStreamSender(const QpackDecoderStreamSender&) = delete;
QpackDecoderStreamSender& operator=(const QpackDecoderStreamSender&) = delete;
- // Methods for sending instructions, see
+ // Methods for serializing and buffering instructions, see
// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#rfc.section.5.3
// 5.3.1 Insert Count Increment
@@ -33,6 +33,9 @@
// 5.3.3 Stream Cancellation
void SendStreamCancellation(QuicStreamId stream_id);
+ // Writes all buffered instructions on the decoder stream.
+ void Flush();
+
// delegate must be set if dynamic table capacity is not zero.
void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
delegate_ = delegate;
@@ -42,6 +45,7 @@
QpackStreamSenderDelegate* delegate_;
QpackInstructionEncoder instruction_encoder_;
QpackInstructionEncoder::Values values_;
+ std::string buffer_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_decoder_stream_sender_test.cc b/quic/core/qpack/qpack_decoder_stream_sender_test.cc
index ccb42a3..6e04259 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender_test.cc
+++ b/quic/core/qpack/qpack_decoder_stream_sender_test.cc
@@ -29,45 +29,74 @@
TEST_F(QpackDecoderStreamSenderTest, InsertCountIncrement) {
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("00"))));
stream_.SendInsertCountIncrement(0);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("0a"))));
stream_.SendInsertCountIncrement(10);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("3f00"))));
stream_.SendInsertCountIncrement(63);
+ stream_.Flush();
EXPECT_CALL(delegate_,
WriteStreamData(Eq(QuicTextUtils::HexDecode("3f8901"))));
stream_.SendInsertCountIncrement(200);
+ stream_.Flush();
}
TEST_F(QpackDecoderStreamSenderTest, HeaderAcknowledgement) {
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("80"))));
stream_.SendHeaderAcknowledgement(0);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("a5"))));
stream_.SendHeaderAcknowledgement(37);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("ff00"))));
stream_.SendHeaderAcknowledgement(127);
+ stream_.Flush();
EXPECT_CALL(delegate_,
WriteStreamData(Eq(QuicTextUtils::HexDecode("fff802"))));
stream_.SendHeaderAcknowledgement(503);
+ stream_.Flush();
}
TEST_F(QpackDecoderStreamSenderTest, StreamCancellation) {
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("40"))));
stream_.SendStreamCancellation(0);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("53"))));
stream_.SendStreamCancellation(19);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("7f00"))));
stream_.SendStreamCancellation(63);
+ stream_.Flush();
EXPECT_CALL(delegate_, WriteStreamData(Eq(QuicTextUtils::HexDecode("7f2f"))));
stream_.SendStreamCancellation(110);
+ stream_.Flush();
+}
+
+TEST_F(QpackDecoderStreamSenderTest, Coalesce) {
+ stream_.SendInsertCountIncrement(10);
+ stream_.SendHeaderAcknowledgement(37);
+ stream_.SendStreamCancellation(0);
+
+ EXPECT_CALL(delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("0aa540"))));
+ stream_.Flush();
+
+ stream_.SendInsertCountIncrement(63);
+ stream_.SendStreamCancellation(110);
+
+ EXPECT_CALL(delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("3f007f2f"))));
+ stream_.Flush();
}
} // namespace
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index faa0f9d..d0ff30f 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -861,14 +861,14 @@
EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
EXPECT_CALL(handler_, OnDecodingCompleted());
- EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(kHeaderAcknowledgement)));
// Decoder received two insertions, but Header Acknowledgement only increases
// Known Insert Count to one. Decoder should send an Insert Count Increment
// instruction with increment of one to update Known Insert Count to two.
EXPECT_CALL(decoder_stream_sender_delegate_,
- WriteStreamData(Eq(QuicTextUtils::HexDecode("01"))));
+ WriteStreamData(Eq(QuicTextUtils::HexDecode(
+ "81" // Header Acknowledgement on stream 1
+ "01")))); // Insert Count Increment with increment of one
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0200" // Required Insert Count 1 and Delta Base 0.