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.