Add debug visitor for logging when QPACK encoder cannot insert into dynamic table or cannot refer to unacknowledged entries.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 270715196
Change-Id: I94cece5ca466ddc58a668805e91acff70e468528
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index 61ee8fc..bd16f36 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -35,7 +35,8 @@
DecoderStreamErrorDelegate* decoder_stream_error_delegate)
: decoder_stream_error_delegate_(decoder_stream_error_delegate),
decoder_stream_receiver_(this),
- maximum_blocked_streams_(0) {
+ maximum_blocked_streams_(0),
+ debug_visitor_(nullptr) {
DCHECK(decoder_stream_error_delegate_);
}
@@ -110,6 +111,10 @@
const bool blocking_allowed = blocking_manager_.blocking_allowed_on_stream(
stream_id, maximum_blocked_streams_);
+ // Track events for debug visitor.
+ bool dynamic_table_insertion_blocked = false;
+ bool blocked_stream_limit_exhausted = false;
+
for (const auto& header : ValueSplittingHeaderList(&header_list)) {
// These strings are owned by |header_list|.
QuicStringPiece name = header.first;
@@ -131,20 +136,26 @@
break;
}
- if (blocking_allowed || index < known_received_count) {
- if (index >= draining_index) {
- // If allowed, refer to entry directly.
+ if (index >= draining_index) {
+ // If allowed, refer to entry directly.
+ if (!blocking_allowed && index >= known_received_count) {
+ blocked_stream_limit_exhausted = true;
+ } else {
instructions.push_back(
EncodeIndexedHeaderField(is_static, index, referred_indices));
smallest_blocking_index = std::min(smallest_blocking_index, index);
break;
}
-
- if (blocking_allowed &&
- QpackEntry::Size(name, value) <=
- header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
- std::min(smallest_blocking_index, index))) {
+ } else {
+ // Entry is draining, needs to be duplicated.
+ if (!blocking_allowed) {
+ blocked_stream_limit_exhausted = true;
+ } else if (QpackEntry::Size(name, value) >
+ header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
+ std::min(smallest_blocking_index, index))) {
+ dynamic_table_insertion_blocked = true;
+ } else {
// If allowed, duplicate entry and refer to it.
sent_byte_count += encoder_stream_sender_.SendDuplicate(
QpackAbsoluteIndexToEncoderStreamRelativeIndex(
@@ -196,10 +207,13 @@
break;
}
- if (blocking_allowed &&
- QpackEntry::Size(name, value) <=
- header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
- std::min(smallest_blocking_index, index))) {
+ if (!blocking_allowed) {
+ blocked_stream_limit_exhausted = true;
+ } else if (QpackEntry::Size(name, value) >
+ header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
+ std::min(smallest_blocking_index, index))) {
+ dynamic_table_insertion_blocked = true;
+ } else {
// If allowed, insert entry with name reference and refer to it.
sent_byte_count += encoder_stream_sender_.SendInsertWithNameReference(
is_static,
@@ -236,11 +250,14 @@
break;
case QpackHeaderTable::MatchType::kNoMatch:
- if (blocking_allowed &&
- QpackEntry::Size(name, value) <=
- header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
- smallest_blocking_index)) {
- // If allowed, insert entry and refer to it.
+ // If allowed, insert entry and refer to it.
+ if (!blocking_allowed) {
+ blocked_stream_limit_exhausted = true;
+ } else if (QpackEntry::Size(name, value) >
+ header_table_.MaxInsertSizeWithoutEvictingGivenEntry(
+ smallest_blocking_index)) {
+ dynamic_table_insertion_blocked = true;
+ } else {
sent_byte_count +=
encoder_stream_sender_.SendInsertWithoutNameReference(name,
value);
@@ -255,6 +272,9 @@
}
// Encode entry as string literals.
+ // TODO(b/112770235): Consider also adding to dynamic table to improve
+ // compression ratio for subsequent header blocks with peers that do not
+ // allow any blocked streams.
instructions.push_back(EncodeLiteralHeaderField(name, value));
break;
@@ -267,6 +287,11 @@
*encoder_stream_sent_byte_count = sent_byte_count;
}
+ if (debug_visitor_) {
+ debug_visitor_->OnHeaderListEncoded(dynamic_table_insertion_blocked,
+ blocked_stream_limit_exhausted);
+ }
+
return instructions;
}
diff --git a/quic/core/qpack/qpack_encoder.h b/quic/core/qpack/qpack_encoder.h
index 2ffbc99..c4aaced 100644
--- a/quic/core/qpack/qpack_encoder.h
+++ b/quic/core/qpack/qpack_encoder.h
@@ -46,6 +46,25 @@
virtual void OnDecoderStreamError(QuicStringPiece error_message) = 0;
};
+ // Interface for logging whenever dynamic table insertion is blocked and
+ // whenever using unacknowledged entries is not allowed because of the blocked
+ // stream limit.
+ class QUIC_EXPORT_PRIVATE DebugVisitor {
+ public:
+ virtual ~DebugVisitor() = default;
+
+ // Called once for each header list encoded.
+ // |dynamic_table_insertion_blocked| it true if for any header field in this
+ // header list the encoder would have inserted an entry into the dynamic
+ // table, but could not because that insertion would have evicted a blocking
+ // entry. |blocked_stream_limit_exhausted| is true if for any header field
+ // in this header list the encoder would have referred to an unacknowledged
+ // entry in the dynamic table, but could not because that would have
+ // violated the limit on the number of blocked streams.
+ virtual void OnHeaderListEncoded(bool dynamic_table_insertion_blocked,
+ bool blocked_stream_limit_exhausted) = 0;
+ };
+
QpackEncoder(DecoderStreamErrorDelegate* decoder_stream_error_delegate);
~QpackEncoder() override;
@@ -86,6 +105,10 @@
return &decoder_stream_receiver_;
}
+ void set_debug_visitor(DebugVisitor* debug_visitor) {
+ debug_visitor_ = debug_visitor;
+ }
+
private:
friend class test::QpackEncoderPeer;
@@ -148,6 +171,7 @@
QpackHeaderTable header_table_;
uint64_t maximum_blocked_streams_;
QpackBlockingManager blocking_manager_;
+ DebugVisitor* debug_visitor_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_encoder_test.cc b/quic/core/qpack/qpack_encoder_test.cc
index b0464c8..e6ee259 100644
--- a/quic/core/qpack/qpack_encoder_test.cc
+++ b/quic/core/qpack/qpack_encoder_test.cc
@@ -23,6 +23,13 @@
namespace test {
namespace {
+class MockDebugVisitor : public QpackEncoder::DebugVisitor {
+ public:
+ virtual ~MockDebugVisitor() = default;
+
+ MOCK_METHOD(void, OnHeaderListEncoded, (bool, bool), (override));
+};
+
class QpackEncoderTest : public QuicTest {
protected:
QpackEncoderTest()
@@ -466,6 +473,70 @@
EXPECT_EQ(30u, QpackHeaderTablePeer::dynamic_table_capacity(header_table));
}
+TEST_F(QpackEncoderTest, DebugVisitor) {
+ StrictMock<MockDebugVisitor> debug_visitor;
+ encoder_.set_debug_visitor(&debug_visitor);
+ encoder_.SetMaximumDynamicTableCapacity(QpackEntry::Size("foo", "bar"));
+
+ // Set Dynamic Table Capacity instruction.
+ EXPECT_CALL(encoder_stream_sender_delegate_, WriteStreamData(_));
+ encoder_.SetDynamicTableCapacity(QpackEntry::Size("foo", "bar"));
+
+ // Static table entry only, no blocking.
+ spdy::SpdyHeaderBlock header_list1;
+ header_list1[":method"] = "GET";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ false,
+ /* blocked_stream_limit_exhausted = */ false));
+ encoder_.EncodeHeaderList(/* stream_id = */ 1, header_list1, nullptr);
+
+ // Insert single dynamic table entry. First stream is allowed to be blocked.
+ spdy::SpdyHeaderBlock header_list2;
+ header_list2["foo"] = "bar";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ false,
+ /* blocked_stream_limit_exhausted = */ false));
+ // Add entry to dynamic table.
+ EXPECT_CALL(encoder_stream_sender_delegate_, WriteStreamData(_));
+ encoder_.EncodeHeaderList(/* stream_id = */ 1, header_list2, nullptr);
+
+ // First stream is blocked. Second stream is not allowed to be blocked.
+ // This would have been an insertion with name reference.
+ spdy::SpdyHeaderBlock header_list3;
+ header_list3["foo"] = "baz";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ false,
+ /* blocked_stream_limit_exhausted = */ true));
+ encoder_.EncodeHeaderList(/* stream_id = */ 2, header_list3, nullptr);
+
+ // First stream is blocked. Second stream is not allowed to be blocked.
+ // This would have been an insertion without name reference.
+ spdy::SpdyHeaderBlock header_list4;
+ header_list4["bar"] = "baz";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ false,
+ /* blocked_stream_limit_exhausted = */ true));
+ encoder_.EncodeHeaderList(/* stream_id = */ 2, header_list4, nullptr);
+
+ // Cannot insert into dynamic table because first entry takes up all space and
+ // it is blocking. This would have been an insertion with name reference.
+ spdy::SpdyHeaderBlock header_list5;
+ header_list5["foo"] = "baz";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ true,
+ /* blocked_stream_limit_exhausted = */ false));
+ encoder_.EncodeHeaderList(/* stream_id = */ 1, header_list5, nullptr);
+
+ // Cannot insert into dynamic table because first entry takes up all space and
+ // it is blocking. This would have been an insertion without name reference.
+ spdy::SpdyHeaderBlock header_list6;
+ header_list6["bar"] = "baz";
+ EXPECT_CALL(debug_visitor, OnHeaderListEncoded(
+ /* dynamic_table_insertion_blocked = */ true,
+ /* blocked_stream_limit_exhausted = */ false));
+ encoder_.EncodeHeaderList(/* stream_id = */ 1, header_list6, nullptr);
+}
+
} // namespace
} // namespace test
} // namespace quic
