Simplify QpackEncoderStreamSender API.
Instead of each Send*() method returning the number of bytes encoded, return the
total number of bytes written from Flush().
This is a follow-up to cr/273553497.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 273775165
Change-Id: Ifaf1b55d6d0ab26cc3ab23686bcb7427e3c9627f
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index eac158b..b1f7fbe 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -93,7 +93,6 @@
QuicByteCount* encoder_stream_sent_byte_count) {
Instructions instructions;
instructions.reserve(header_list.size());
- QuicByteCount sent_byte_count = 0;
// The index of the oldest entry that must not be evicted.
uint64_t smallest_blocking_index =
@@ -157,7 +156,7 @@
dynamic_table_insertion_blocked = true;
} else {
// If allowed, duplicate entry and refer to it.
- sent_byte_count += encoder_stream_sender_.SendDuplicate(
+ encoder_stream_sender_.SendDuplicate(
QpackAbsoluteIndexToEncoderStreamRelativeIndex(
index, header_table_.inserted_entry_count()));
auto entry = header_table_.InsertEntry(name, value);
@@ -187,9 +186,8 @@
header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
smallest_blocking_index)) {
// If allowed, insert entry into dynamic table and refer to it.
- sent_byte_count +=
- encoder_stream_sender_.SendInsertWithNameReference(
- is_static, index, value);
+ encoder_stream_sender_.SendInsertWithNameReference(is_static, index,
+ value);
auto entry = header_table_.InsertEntry(name, value);
instructions.push_back(EncodeIndexedHeaderField(
/* is_static = */ false, entry->InsertionIndex(),
@@ -215,7 +213,7 @@
dynamic_table_insertion_blocked = true;
} else {
// If allowed, insert entry with name reference and refer to it.
- sent_byte_count += encoder_stream_sender_.SendInsertWithNameReference(
+ encoder_stream_sender_.SendInsertWithNameReference(
is_static,
QpackAbsoluteIndexToEncoderStreamRelativeIndex(
index, header_table_.inserted_entry_count()),
@@ -258,9 +256,7 @@
smallest_blocking_index)) {
dynamic_table_insertion_blocked = true;
} else {
- sent_byte_count +=
- encoder_stream_sender_.SendInsertWithoutNameReference(name,
- value);
+ encoder_stream_sender_.SendInsertWithoutNameReference(name, value);
auto entry = header_table_.InsertEntry(name, value);
instructions.push_back(EncodeIndexedHeaderField(
/* is_static = */ false, entry->InsertionIndex(),
@@ -281,10 +277,7 @@
}
}
- encoder_stream_sender_.Flush();
-
- // Use local |sent_byte_count| variable to avoid branching and dereferencing
- // each time encoder stream data is sent.
+ const QuicByteCount sent_byte_count = encoder_stream_sender_.Flush();
if (encoder_stream_sent_byte_count) {
*encoder_stream_sent_byte_count = sent_byte_count;
}
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.cc b/quic/core/qpack/qpack_encoder_stream_sender.cc
index 8df0b03..4a7f12c 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_encoder_stream_sender.cc
@@ -15,7 +15,7 @@
QpackEncoderStreamSender::QpackEncoderStreamSender() : delegate_(nullptr) {}
-QuicByteCount QpackEncoderStreamSender::SendInsertWithNameReference(
+void QpackEncoderStreamSender::SendInsertWithNameReference(
bool is_static,
uint64_t name_index,
QuicStringPiece value) {
@@ -23,45 +23,42 @@
values_.varint = name_index;
values_.value = value;
- return Encode(InsertWithNameReferenceInstruction());
+ instruction_encoder_.Encode(InsertWithNameReferenceInstruction(), values_,
+ &buffer_);
}
-QuicByteCount QpackEncoderStreamSender::SendInsertWithoutNameReference(
+void QpackEncoderStreamSender::SendInsertWithoutNameReference(
QuicStringPiece name,
QuicStringPiece value) {
values_.name = name;
values_.value = value;
- return Encode(InsertWithoutNameReferenceInstruction());
+ instruction_encoder_.Encode(InsertWithoutNameReferenceInstruction(), values_,
+ &buffer_);
}
-QuicByteCount QpackEncoderStreamSender::SendDuplicate(uint64_t index) {
+void QpackEncoderStreamSender::SendDuplicate(uint64_t index) {
values_.varint = index;
- return Encode(DuplicateInstruction());
+ instruction_encoder_.Encode(DuplicateInstruction(), values_, &buffer_);
}
-QuicByteCount QpackEncoderStreamSender::SendSetDynamicTableCapacity(
- uint64_t capacity) {
+void QpackEncoderStreamSender::SendSetDynamicTableCapacity(uint64_t capacity) {
values_.varint = capacity;
- return Encode(SetDynamicTableCapacityInstruction());
+ instruction_encoder_.Encode(SetDynamicTableCapacityInstruction(), values_,
+ &buffer_);
}
-void QpackEncoderStreamSender::Flush() {
+QuicByteCount QpackEncoderStreamSender::Flush() {
if (buffer_.empty()) {
- return;
+ return 0;
}
delegate_->WriteStreamData(buffer_);
+ const QuicByteCount bytes_written = buffer_.size();
buffer_.clear();
-}
-
-QuicByteCount QpackEncoderStreamSender::Encode(
- const QpackInstruction* instruction) {
- const size_t old_buffer_size = buffer_.size();
- instruction_encoder_.Encode(instruction, values_, &buffer_);
- return buffer_.size() - old_buffer_size;
+ return bytes_written;
}
} // namespace quic
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.h b/quic/core/qpack/qpack_encoder_stream_sender.h
index 5c4af7d..efbfbc6 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.h
+++ b/quic/core/qpack/qpack_encoder_stream_sender.h
@@ -27,19 +27,20 @@
// https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#rfc.section.5.2
// 5.2.1. Insert With Name Reference
- QuicByteCount SendInsertWithNameReference(bool is_static,
- uint64_t name_index,
- QuicStringPiece value);
+ void SendInsertWithNameReference(bool is_static,
+ uint64_t name_index,
+ QuicStringPiece value);
// 5.2.2. Insert Without Name Reference
- QuicByteCount SendInsertWithoutNameReference(QuicStringPiece name,
- QuicStringPiece value);
+ void SendInsertWithoutNameReference(QuicStringPiece name,
+ QuicStringPiece value);
// 5.2.3. Duplicate
- QuicByteCount SendDuplicate(uint64_t index);
+ void SendDuplicate(uint64_t index);
// 5.2.4. Set Dynamic Table Capacity
- QuicByteCount SendSetDynamicTableCapacity(uint64_t capacity);
+ void SendSetDynamicTableCapacity(uint64_t capacity);
// Writes all buffered instructions on the encoder stream.
- void Flush();
+ // Returns the number of bytes written.
+ QuicByteCount Flush();
// delegate must be set if dynamic table capacity is not zero.
void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
@@ -47,10 +48,6 @@
}
private:
- // Encodes |instruction| with |values_| using |instruction_encoder_| into
- // |buffer_|. Returns number of bytes encoded.
- QuicByteCount Encode(const QpackInstruction* instruction);
-
QpackStreamSenderDelegate* delegate_;
QpackInstructionEncoder instruction_encoder_;
QpackInstructionEncoder::Values values_;
diff --git a/quic/core/qpack/qpack_encoder_stream_sender_test.cc b/quic/core/qpack/qpack_encoder_stream_sender_test.cc
index b1ddd97..80a6ed3 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender_test.cc
+++ b/quic/core/qpack/qpack_encoder_stream_sender_test.cc
@@ -30,23 +30,20 @@
// Static, index fits in prefix, empty value.
std::string expected_encoded_data = QuicTextUtils::HexDecode("c500");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithNameReference(true, 5, ""));
- stream_.Flush();
+ stream_.SendInsertWithNameReference(true, 5, "");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Static, index fits in prefix, Huffman encoded value.
expected_encoded_data = QuicTextUtils::HexDecode("c28294e7");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithNameReference(true, 2, "foo"));
- stream_.Flush();
+ stream_.SendInsertWithNameReference(true, 2, "foo");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Not static, index does not fit in prefix, not Huffman encoded value.
expected_encoded_data = QuicTextUtils::HexDecode("bf4a03626172");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithNameReference(false, 137, "bar"));
- stream_.Flush();
+ stream_.SendInsertWithNameReference(false, 137, "bar");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Value length does not fit in prefix.
// 'Z' would be Huffman encoded to 8 bits, so no Huffman encoding is used.
@@ -56,33 +53,28 @@
"5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a"
"5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(
- expected_encoded_data.size(),
- stream_.SendInsertWithNameReference(false, 42, std::string(127, 'Z')));
- stream_.Flush();
+ stream_.SendInsertWithNameReference(false, 42, std::string(127, 'Z'));
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
}
TEST_F(QpackEncoderStreamSenderTest, InsertWithoutNameReference) {
// Empty name and value.
std::string expected_encoded_data = QuicTextUtils::HexDecode("4000");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithoutNameReference("", ""));
- stream_.Flush();
+ stream_.SendInsertWithoutNameReference("", "");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Huffman encoded short strings.
expected_encoded_data = QuicTextUtils::HexDecode("6294e78294e7");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithoutNameReference("foo", "foo"));
- stream_.Flush();
+ stream_.SendInsertWithoutNameReference("foo", "foo");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Not Huffman encoded short strings.
expected_encoded_data = QuicTextUtils::HexDecode("4362617203626172");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithoutNameReference("bar", "bar"));
- stream_.Flush();
+ stream_.SendInsertWithoutNameReference("bar", "bar");
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Not Huffman encoded long strings; length does not fit on prefix.
// 'Z' would be Huffman encoded to 8 bits, so no Huffman encoding is used.
@@ -93,74 +85,67 @@
"5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a"
"5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a5a");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendInsertWithoutNameReference(std::string(31, 'Z'),
- std::string(127, 'Z')));
- stream_.Flush();
+ stream_.SendInsertWithoutNameReference(std::string(31, 'Z'),
+ std::string(127, 'Z'));
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
}
TEST_F(QpackEncoderStreamSenderTest, Duplicate) {
// Small index fits in prefix.
std::string expected_encoded_data = QuicTextUtils::HexDecode("11");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(), stream_.SendDuplicate(17));
- stream_.Flush();
+ stream_.SendDuplicate(17);
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Large index requires two extension bytes.
expected_encoded_data = QuicTextUtils::HexDecode("1fd503");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(), stream_.SendDuplicate(500));
- stream_.Flush();
+ stream_.SendDuplicate(500);
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
}
TEST_F(QpackEncoderStreamSenderTest, SetDynamicTableCapacity) {
// Small capacity fits in prefix.
std::string expected_encoded_data = QuicTextUtils::HexDecode("31");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendSetDynamicTableCapacity(17));
- stream_.Flush();
+ stream_.SendSetDynamicTableCapacity(17);
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
// Large capacity requires two extension bytes.
expected_encoded_data = QuicTextUtils::HexDecode("3fd503");
EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
- EXPECT_EQ(expected_encoded_data.size(),
- stream_.SendSetDynamicTableCapacity(500));
- stream_.Flush();
+ stream_.SendSetDynamicTableCapacity(500);
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
}
// No writes should happen until Flush is called.
TEST_F(QpackEncoderStreamSenderTest, Coalesce) {
// Insert entry with static name reference, empty value.
- std::string expected_encoded_data1 = QuicTextUtils::HexDecode("c500");
- EXPECT_EQ(expected_encoded_data1.size(),
- stream_.SendInsertWithNameReference(true, 5, ""));
+ stream_.SendInsertWithNameReference(true, 5, "");
// Insert entry with static name reference, Huffman encoded value.
- std::string expected_encoded_data2 = QuicTextUtils::HexDecode("c28294e7");
- EXPECT_EQ(expected_encoded_data2.size(),
- stream_.SendInsertWithNameReference(true, 2, "foo"));
+ stream_.SendInsertWithNameReference(true, 2, "foo");
// Insert literal entry, Huffman encoded short strings.
- std::string expected_encoded_data3 = QuicTextUtils::HexDecode("6294e78294e7");
- EXPECT_EQ(expected_encoded_data3.size(),
- stream_.SendInsertWithoutNameReference("foo", "foo"));
+ stream_.SendInsertWithoutNameReference("foo", "foo");
// Duplicate entry.
- std::string expected_encoded_data4 = QuicTextUtils::HexDecode("11");
- EXPECT_EQ(expected_encoded_data4.size(), stream_.SendDuplicate(17));
+ stream_.SendDuplicate(17);
- EXPECT_CALL(delegate_,
- WriteStreamData(Eq(QuicStrCat(
- expected_encoded_data1 + expected_encoded_data2 +
- expected_encoded_data3 + expected_encoded_data4))));
- stream_.Flush();
+ std::string expected_encoded_data = QuicTextUtils::HexDecode(
+ "c500" // Insert entry with static name reference.
+ "c28294e7" // Insert entry with static name reference.
+ "6294e78294e7" // Insert literal entry.
+ "11"); // Duplicate entry.
+
+ EXPECT_CALL(delegate_, WriteStreamData(Eq(expected_encoded_data)));
+ EXPECT_EQ(expected_encoded_data.size(), stream_.Flush());
}
// No writes should happen if QpackEncoderStreamSender::Flush() is called
// when the buffer is empty.
TEST_F(QpackEncoderStreamSenderTest, FlushEmpty) {
- stream_.Flush();
+ EXPECT_EQ(0u, stream_.Flush());
}
} // namespace
