Remove unused HPACK entry timing infrastructure.
All of this code has been added to measure head of line blocking due to using
HPACK with gQUIC. The results of this experiment have informed the design of
different header compression algorithms. In the end, QPACK has been chosen for
HTTP/3. The experiment has concluded, and all of this code can be removed.
Removing a 64-bit time_added from http2::HpackDecoderTableEntry and
spdy::HpackEntry (also aliased as quic::QpackEntry) will slightly decrease
memory consumption.
Also remove HpackDecoderTablesDebugListener, QuicHpackDebugVisitor,
HpackHeaderTable::DebugVisitorInterface, their subclasses, and related
accessors.
PiperOrigin-RevId: 362635008
Change-Id: Ifed10c678a5f9ef5d7daccb019f205460d070b4a
diff --git a/http2/hpack/decoder/hpack_decoder.cc b/http2/hpack/decoder/hpack_decoder.cc
index 45d7a78..0f1336e 100644
--- a/http2/hpack/decoder/hpack_decoder.cc
+++ b/http2/hpack/decoder/hpack_decoder.cc
@@ -21,11 +21,6 @@
HpackDecoder::~HpackDecoder() = default;
-void HpackDecoder::set_tables_debug_listener(
- HpackDecoderTablesDebugListener* debug_listener) {
- decoder_state_.set_tables_debug_listener(debug_listener);
-}
-
void HpackDecoder::set_max_string_size_bytes(size_t max_string_size_bytes) {
entry_buffer_.set_max_string_size_bytes(max_string_size_bytes);
}
diff --git a/http2/hpack/decoder/hpack_decoder.h b/http2/hpack/decoder/hpack_decoder.h
index b5fdecd..01d32a3 100644
--- a/http2/hpack/decoder/hpack_decoder.h
+++ b/http2/hpack/decoder/hpack_decoder.h
@@ -45,11 +45,6 @@
HpackDecoder(const HpackDecoder&) = delete;
HpackDecoder& operator=(const HpackDecoder&) = delete;
- // Set listener to be notified of insertions into the HPACK dynamic table,
- // and uses of those entries.
- void set_tables_debug_listener(
- HpackDecoderTablesDebugListener* debug_listener);
-
// max_string_size specifies the maximum size of an on-the-wire string (name
// or value, plain or Huffman encoded) that will be accepted. See sections
// 5.1 and 5.2 of RFC 7541. This is a defense against OOM attacks; HTTP/2
diff --git a/http2/hpack/decoder/hpack_decoder_state.cc b/http2/hpack/decoder/hpack_decoder_state.cc
index 86df71b..6c23892 100644
--- a/http2/hpack/decoder/hpack_decoder_state.cc
+++ b/http2/hpack/decoder/hpack_decoder_state.cc
@@ -34,11 +34,6 @@
error_(HpackDecodingError::kOk) {}
HpackDecoderState::~HpackDecoderState() = default;
-void HpackDecoderState::set_tables_debug_listener(
- HpackDecoderTablesDebugListener* debug_listener) {
- decoder_tables_.set_debug_listener(debug_listener);
-}
-
void HpackDecoderState::ApplyHeaderTableSizeSetting(
uint32_t header_table_size) {
HTTP2_DVLOG(2) << "HpackDecoderState::ApplyHeaderTableSizeSetting("
diff --git a/http2/hpack/decoder/hpack_decoder_state.h b/http2/hpack/decoder/hpack_decoder_state.h
index cb2ff6d..14b4184 100644
--- a/http2/hpack/decoder/hpack_decoder_state.h
+++ b/http2/hpack/decoder/hpack_decoder_state.h
@@ -43,11 +43,6 @@
// The listener may be changed at any time.
HpackDecoderListener* listener() const { return listener_; }
- // Set listener to be notified of insertions into the HPACK dynamic table,
- // and uses of those entries.
- void set_tables_debug_listener(
- HpackDecoderTablesDebugListener* debug_listener);
-
// ApplyHeaderTableSizeSetting notifies this object that this endpoint has
// received a SETTINGS ACK frame acknowledging an earlier SETTINGS frame from
// this endpoint specifying a new value for SETTINGS_HEADER_TABLE_SIZE (the
diff --git a/http2/hpack/decoder/hpack_decoder_tables.cc b/http2/hpack/decoder/hpack_decoder_tables.cc
index 203aae0..06ad91e 100644
--- a/http2/hpack/decoder/hpack_decoder_tables.cc
+++ b/http2/hpack/decoder/hpack_decoder_tables.cc
@@ -34,9 +34,6 @@
} // namespace
-HpackDecoderTablesDebugListener::HpackDecoderTablesDebugListener() = default;
-HpackDecoderTablesDebugListener::~HpackDecoderTablesDebugListener() = default;
-
HpackDecoderStaticTable::HpackDecoderStaticTable(
const std::vector<HpackStringPair>* table)
: table_(table) {}
@@ -50,13 +47,8 @@
return nullptr;
}
-HpackDecoderDynamicTable::HpackDecoderTableEntry::HpackDecoderTableEntry(
- std::string name_arg,
- std::string value_arg)
- : HpackStringPair(std::move(name_arg), std::move(value_arg)) {}
-
HpackDecoderDynamicTable::HpackDecoderDynamicTable()
- : insert_count_(kFirstDynamicTableIndex - 1), debug_listener_(nullptr) {}
+ : insert_count_(kFirstDynamicTableIndex - 1) {}
HpackDecoderDynamicTable::~HpackDecoderDynamicTable() = default;
void HpackDecoderDynamicTable::DynamicTableSizeUpdate(size_t size_limit) {
@@ -84,11 +76,6 @@
return;
}
++insert_count_;
- if (debug_listener_ != nullptr) {
- entry.time_added = debug_listener_->OnEntryInserted(entry, insert_count_);
- HTTP2_DVLOG(2) << "OnEntryInserted returned time_added=" << entry.time_added
- << " for insert_count_=" << insert_count_;
- }
size_t insert_limit = size_limit_ - entry_size;
EnsureSizeNoMoreThan(insert_limit);
table_.push_front(entry);
@@ -100,13 +87,7 @@
const HpackStringPair* HpackDecoderDynamicTable::Lookup(size_t index) const {
if (index < table_.size()) {
- const HpackDecoderTableEntry& entry = table_[index];
- if (debug_listener_ != nullptr) {
- size_t insert_count_of_index = insert_count_ + table_.size() - index;
- debug_listener_->OnUseEntry(entry, insert_count_of_index,
- entry.time_added);
- }
- return &entry;
+ return &table_[index];
}
return nullptr;
}
@@ -137,11 +118,6 @@
HpackDecoderTables::HpackDecoderTables() = default;
HpackDecoderTables::~HpackDecoderTables() = default;
-void HpackDecoderTables::set_debug_listener(
- HpackDecoderTablesDebugListener* debug_listener) {
- dynamic_table_.set_debug_listener(debug_listener);
-}
-
const HpackStringPair* HpackDecoderTables::Lookup(size_t index) const {
if (index < kFirstDynamicTableIndex) {
return static_table_.Lookup(index);
diff --git a/http2/hpack/decoder/hpack_decoder_tables.h b/http2/hpack/decoder/hpack_decoder_tables.h
index 6ad8ca9..f79edb5 100644
--- a/http2/hpack/decoder/hpack_decoder_tables.h
+++ b/http2/hpack/decoder/hpack_decoder_tables.h
@@ -31,37 +31,6 @@
class HpackDecoderTablesPeer;
} // namespace test
-// HpackDecoderTablesDebugListener supports a QUIC experiment, enabling
-// the gathering of information about the time-line of use of HPACK
-// dynamic table entries.
-class QUICHE_EXPORT_PRIVATE HpackDecoderTablesDebugListener {
- public:
- HpackDecoderTablesDebugListener();
- virtual ~HpackDecoderTablesDebugListener();
-
- HpackDecoderTablesDebugListener(const HpackDecoderTablesDebugListener&) =
- delete;
- HpackDecoderTablesDebugListener& operator=(
- const HpackDecoderTablesDebugListener&) = delete;
-
- // The entry has been inserted into the dynamic table. insert_count starts at
- // 62 because 61 is the last index in the static table; insert_count increases
- // by 1 with each insert into the dynamic table; it is not incremented when
- // when a entry is too large to fit into the dynamic table at all (which has
- // the effect of emptying the dynamic table).
- // Returns a value that can be used as time_added in OnUseEntry.
- virtual int64_t OnEntryInserted(const HpackStringPair& entry,
- size_t insert_count) = 0;
-
- // The entry has been used, either for the name or for the name and value.
- // insert_count is the same as passed to OnEntryInserted when entry was
- // inserted to the dynamic table, and time_added is the value that was
- // returned by OnEntryInserted.
- virtual void OnUseEntry(const HpackStringPair& entry,
- size_t insert_count,
- int64_t time_added) = 0;
-};
-
// See http://httpwg.org/specs/rfc7541.html#static.table.definition for the
// contents, and http://httpwg.org/specs/rfc7541.html#index.address.space for
// info about accessing the static table.
@@ -93,13 +62,6 @@
HpackDecoderDynamicTable(const HpackDecoderDynamicTable&) = delete;
HpackDecoderDynamicTable& operator=(const HpackDecoderDynamicTable&) = delete;
- // Set the listener to be notified of insertions into this table, and later
- // uses of those entries. Added for evaluation of changes to QUIC's use
- // of HPACK.
- void set_debug_listener(HpackDecoderTablesDebugListener* debug_listener) {
- debug_listener_ = debug_listener;
- }
-
// Sets a new size limit, received from the peer; performs evictions if
// necessary to ensure that the current size does not exceed the new limit.
// The caller needs to have validated that size_limit does not
@@ -119,10 +81,8 @@
private:
friend class test::HpackDecoderTablesPeer;
- struct HpackDecoderTableEntry : public HpackStringPair {
- HpackDecoderTableEntry(std::string name_arg, std::string value_arg);
- int64_t time_added;
- };
+ // TODO(bnc): Move HpackStringPair class here.
+ using HpackDecoderTableEntry = HpackStringPair;
// Drop older entries to ensure the size is not greater than limit.
void EnsureSizeNoMoreThan(size_t limit);
@@ -141,7 +101,6 @@
// insert_count_ and debug_listener_ are used by a QUIC experiment; remove
// when the experiment is done.
size_t insert_count_;
- HpackDecoderTablesDebugListener* debug_listener_;
};
class QUICHE_EXPORT_PRIVATE HpackDecoderTables {
@@ -152,11 +111,6 @@
HpackDecoderTables(const HpackDecoderTables&) = delete;
HpackDecoderTables& operator=(const HpackDecoderTables&) = delete;
- // Set the listener to be notified of insertions into the dynamic table, and
- // later uses of those entries. Added for evaluation of changes to QUIC's use
- // of HPACK.
- void set_debug_listener(HpackDecoderTablesDebugListener* debug_listener);
-
// Sets a new size limit, received from the peer; performs evictions if
// necessary to ensure that the current size does not exceed the new limit.
// The caller needs to have validated that size_limit does not
diff --git a/quic/core/http/quic_headers_stream_test.cc b/quic/core/http/quic_headers_stream_test.cc
index cfccc92..55dafac 100644
--- a/quic/core/http/quic_headers_stream_test.cc
+++ b/quic/core/http/quic_headers_stream_test.cc
@@ -73,17 +73,6 @@
namespace quic {
namespace test {
-
-class MockQuicHpackDebugVisitor : public QuicHpackDebugVisitor {
- public:
- MockQuicHpackDebugVisitor() : QuicHpackDebugVisitor() {}
- MockQuicHpackDebugVisitor(const MockQuicHpackDebugVisitor&) = delete;
- MockQuicHpackDebugVisitor& operator=(const MockQuicHpackDebugVisitor&) =
- delete;
-
- MOCK_METHOD(void, OnUseEntry, (QuicTime::Delta elapsed), (override));
-};
-
namespace {
class MockVisitor : public SpdyFramerVisitorInterface {
@@ -766,95 +755,6 @@
headers_stream_));
}
-TEST_P(QuicHeadersStreamTest, HpackDecoderDebugVisitor) {
- auto hpack_decoder_visitor =
- std::make_unique<StrictMock<MockQuicHpackDebugVisitor>>();
- {
- InSequence seq;
- // Number of indexed representations generated in headers below.
- for (int i = 1; i < 28; i++) {
- EXPECT_CALL(*hpack_decoder_visitor,
- OnUseEntry(QuicTime::Delta::FromMilliseconds(i)))
- .Times(4);
- }
- }
- QuicSpdySessionPeer::SetHpackDecoderDebugVisitor(
- &session_, std::move(hpack_decoder_visitor));
-
- // Create some headers we expect to generate entries in HPACK's
- // dynamic table, in addition to content-length.
- headers_["key0"] = std::string(1 << 1, '.');
- headers_["key1"] = std::string(1 << 2, '.');
- headers_["key2"] = std::string(1 << 3, '.');
- for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_;
- stream_id += next_stream_id_) {
- for (bool fin : {false, true}) {
- for (SpdyPriority priority = 0; priority < 7; ++priority) {
- // Replace with "WriteHeadersAndSaveData"
- SpdySerializedFrame frame;
- if (perspective() == Perspective::IS_SERVER) {
- SpdyHeadersIR headers_frame(stream_id, headers_.Clone());
- headers_frame.set_fin(fin);
- headers_frame.set_has_priority(true);
- headers_frame.set_weight(Spdy3PriorityToHttp2Weight(0));
- frame = framer_->SerializeFrame(headers_frame);
- EXPECT_CALL(session_, OnStreamHeadersPriority(
- stream_id, spdy::SpdyStreamPrecedence(0)));
- } else {
- SpdyHeadersIR headers_frame(stream_id, headers_.Clone());
- headers_frame.set_fin(fin);
- frame = framer_->SerializeFrame(headers_frame);
- }
- EXPECT_CALL(session_,
- OnStreamHeaderList(stream_id, fin, frame.size(), _))
- .WillOnce(Invoke(this, &QuicHeadersStreamTest::SaveHeaderList));
- stream_frame_.data_buffer = frame.data();
- stream_frame_.data_length = frame.size();
- connection_->AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
- headers_stream_->OnStreamFrame(stream_frame_);
- stream_frame_.offset += frame.size();
- CheckHeaders();
- }
- }
- }
-}
-
-TEST_P(QuicHeadersStreamTest, HpackEncoderDebugVisitor) {
- auto hpack_encoder_visitor =
- std::make_unique<StrictMock<MockQuicHpackDebugVisitor>>();
- if (perspective() == Perspective::IS_SERVER) {
- InSequence seq;
- for (int i = 1; i < 4; i++) {
- EXPECT_CALL(*hpack_encoder_visitor,
- OnUseEntry(QuicTime::Delta::FromMilliseconds(i)));
- }
- } else {
- InSequence seq;
- for (int i = 1; i < 28; i++) {
- EXPECT_CALL(*hpack_encoder_visitor,
- OnUseEntry(QuicTime::Delta::FromMilliseconds(i)));
- }
- }
- QuicSpdySessionPeer::SetHpackEncoderDebugVisitor(
- &session_, std::move(hpack_encoder_visitor));
-
- for (QuicStreamId stream_id = client_id_1_; stream_id < client_id_3_;
- stream_id += next_stream_id_) {
- for (bool fin : {false, true}) {
- if (perspective() == Perspective::IS_SERVER) {
- WriteAndExpectResponseHeaders(stream_id, fin);
- connection_->AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
- } else {
- for (SpdyPriority priority = 0; priority < 7; ++priority) {
- // TODO(rch): implement priorities correctly.
- WriteAndExpectRequestHeaders(stream_id, fin, 0);
- connection_->AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
- }
- }
- }
- }
-}
-
TEST_P(QuicHeadersStreamTest, AckSentData) {
EXPECT_CALL(session_, WritevData(QuicUtils::GetHeadersStreamId(
connection_->transport_version()),
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index a7f3df8..6aa9829 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -60,34 +60,6 @@
#define ENDPOINT \
(perspective() == Perspective::IS_SERVER ? "Server: " : "Client: ")
-class HeaderTableDebugVisitor : public HpackHeaderTable::DebugVisitorInterface {
- public:
- HeaderTableDebugVisitor(const QuicClock* clock,
- std::unique_ptr<QuicHpackDebugVisitor> visitor)
- : clock_(clock), headers_stream_hpack_visitor_(std::move(visitor)) {}
- HeaderTableDebugVisitor(const HeaderTableDebugVisitor&) = delete;
- HeaderTableDebugVisitor& operator=(const HeaderTableDebugVisitor&) = delete;
-
- int64_t OnNewEntry(const HpackEntry& entry) override {
- QUIC_DVLOG(1) << entry.GetDebugString();
- return (clock_->ApproximateNow() - QuicTime::Zero()).ToMicroseconds();
- }
-
- void OnUseEntry(const HpackEntry& entry) override {
- const QuicTime::Delta elapsed(
- clock_->ApproximateNow() -
- QuicTime::Delta::FromMicroseconds(entry.time_added()) -
- QuicTime::Zero());
- QUIC_DVLOG(1) << entry.GetDebugString() << " " << elapsed.ToMilliseconds()
- << " ms";
- headers_stream_hpack_visitor_->OnUseEntry(elapsed);
- }
-
- private:
- const QuicClock* clock_;
- std::unique_ptr<QuicHpackDebugVisitor> headers_stream_hpack_visitor_;
-};
-
// Class to forward ACCEPT_CH frame to QuicSpdySession,
// and ignore every other frame.
class AlpsFrameDecoder : public HttpDecoder::Visitor {
@@ -474,10 +446,6 @@
QuicHeaderList header_list_;
};
-QuicHpackDebugVisitor::QuicHpackDebugVisitor() {}
-
-QuicHpackDebugVisitor::~QuicHpackDebugVisitor() {}
-
Http3DebugVisitor::Http3DebugVisitor() {}
Http3DebugVisitor::~Http3DebugVisitor() {}
@@ -1370,20 +1338,6 @@
frame_len_ += frame_len;
}
-void QuicSpdySession::SetHpackEncoderDebugVisitor(
- std::unique_ptr<QuicHpackDebugVisitor> visitor) {
- spdy_framer_.SetEncoderHeaderTableDebugVisitor(
- std::unique_ptr<HeaderTableDebugVisitor>(new HeaderTableDebugVisitor(
- connection()->helper()->GetClock(), std::move(visitor))));
-}
-
-void QuicSpdySession::SetHpackDecoderDebugVisitor(
- std::unique_ptr<QuicHpackDebugVisitor> visitor) {
- h2_deframer_.SetDecoderHeaderTableDebugVisitor(
- std::make_unique<HeaderTableDebugVisitor>(
- connection()->helper()->GetClock(), std::move(visitor)));
-}
-
void QuicSpdySession::CloseConnectionWithDetails(QuicErrorCode error,
const std::string& details) {
connection()->CloseConnection(
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h
index bbd97fc..596e9cb 100644
--- a/quic/core/http/quic_spdy_session.h
+++ b/quic/core/http/quic_spdy_session.h
@@ -37,25 +37,6 @@
class QuicSpdySessionPeer;
} // namespace test
-// QuicHpackDebugVisitor gathers data used for understanding HPACK HoL
-// dynamics. Specifically, it is to help predict the compression
-// penalty of avoiding HoL by chagning how the dynamic table is used.
-// In chromium, the concrete instance populates an UMA
-// histogram with the data.
-class QUIC_EXPORT_PRIVATE QuicHpackDebugVisitor {
- public:
- QuicHpackDebugVisitor();
- QuicHpackDebugVisitor(const QuicHpackDebugVisitor&) = delete;
- QuicHpackDebugVisitor& operator=(const QuicHpackDebugVisitor&) = delete;
-
- virtual ~QuicHpackDebugVisitor();
-
- // For each HPACK indexed representation processed, |elapsed| is
- // the time since the corresponding entry was added to the dynamic
- // table.
- virtual void OnUseEntry(QuicTime::Delta elapsed) = 0;
-};
-
class QUIC_EXPORT_PRIVATE Http3DebugVisitor {
public:
Http3DebugVisitor();
@@ -513,12 +494,6 @@
EncryptionLevel level,
std::unique_ptr<QuicEncrypter> encrypter) override;
- // Optional, enables instrumentation related to go/quic-hpack.
- void SetHpackEncoderDebugVisitor(
- std::unique_ptr<QuicHpackDebugVisitor> visitor);
- void SetHpackDecoderDebugVisitor(
- std::unique_ptr<QuicHpackDebugVisitor> visitor);
-
// Sets the maximum size of the header compression table spdy_framer_ is
// willing to use to encode header blocks.
void UpdateHeaderEncoderTableSize(uint32_t value);
diff --git a/quic/test_tools/quic_spdy_session_peer.cc b/quic/test_tools/quic_spdy_session_peer.cc
index aee5d12..c547ac2 100644
--- a/quic/test_tools/quic_spdy_session_peer.cc
+++ b/quic/test_tools/quic_spdy_session_peer.cc
@@ -39,18 +39,6 @@
return &session->spdy_framer_;
}
-void QuicSpdySessionPeer::SetHpackEncoderDebugVisitor(
- QuicSpdySession* session,
- std::unique_ptr<QuicHpackDebugVisitor> visitor) {
- session->SetHpackEncoderDebugVisitor(std::move(visitor));
-}
-
-void QuicSpdySessionPeer::SetHpackDecoderDebugVisitor(
- QuicSpdySession* session,
- std::unique_ptr<QuicHpackDebugVisitor> visitor) {
- session->SetHpackDecoderDebugVisitor(std::move(visitor));
-}
-
void QuicSpdySessionPeer::SetMaxInboundHeaderListSize(
QuicSpdySession* session,
size_t max_inbound_header_size) {
diff --git a/quic/test_tools/quic_spdy_session_peer.h b/quic/test_tools/quic_spdy_session_peer.h
index bbcb696..9ba5497 100644
--- a/quic/test_tools/quic_spdy_session_peer.h
+++ b/quic/test_tools/quic_spdy_session_peer.h
@@ -17,7 +17,6 @@
class QuicHeadersStream;
class QuicSpdySession;
-class QuicHpackDebugVisitor;
namespace test {
@@ -29,12 +28,6 @@
static void SetHeadersStream(QuicSpdySession* session,
QuicHeadersStream* headers_stream);
static spdy::SpdyFramer* GetSpdyFramer(QuicSpdySession* session);
- static void SetHpackEncoderDebugVisitor(
- QuicSpdySession* session,
- std::unique_ptr<QuicHpackDebugVisitor> visitor);
- static void SetHpackDecoderDebugVisitor(
- QuicSpdySession* session,
- std::unique_ptr<QuicHpackDebugVisitor> visitor);
// Must be called before Initialize().
static void SetMaxInboundHeaderListSize(QuicSpdySession* session,
size_t max_inbound_header_size);
diff --git a/spdy/core/hpack/hpack_decoder_adapter.cc b/spdy/core/hpack/hpack_decoder_adapter.cc
index 6b9b425..4d03cc5 100644
--- a/spdy/core/hpack/hpack_decoder_adapter.cc
+++ b/spdy/core/hpack/hpack_decoder_adapter.cc
@@ -107,18 +107,6 @@
return listener_adapter_.decoded_block();
}
-void HpackDecoderAdapter::SetHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor) {
- SPDY_DVLOG(2) << "HpackDecoderAdapter::SetHeaderTableDebugVisitor";
- if (visitor != nullptr) {
- listener_adapter_.SetHeaderTableDebugVisitor(std::move(visitor));
- hpack_decoder_.set_tables_debug_listener(&listener_adapter_);
- } else {
- hpack_decoder_.set_tables_debug_listener(nullptr);
- listener_adapter_.SetHeaderTableDebugVisitor(nullptr);
- }
-}
-
void HpackDecoderAdapter::set_max_decode_buffer_size_bytes(
size_t max_decode_buffer_size_bytes) {
SPDY_DVLOG(2) << "HpackDecoderAdapter::set_max_decode_buffer_size_bytes";
@@ -143,11 +131,6 @@
handler_ = handler;
}
-void HpackDecoderAdapter::ListenerAdapter::SetHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor) {
- visitor_ = std::move(visitor);
-}
-
void HpackDecoderAdapter::ListenerAdapter::OnHeaderListStart() {
SPDY_DVLOG(2) << "HpackDecoderAdapter::ListenerAdapter::OnHeaderListStart";
total_hpack_bytes_ = 0;
@@ -187,36 +170,4 @@
SPDY_VLOG(1) << error_message;
}
-int64_t HpackDecoderAdapter::ListenerAdapter::OnEntryInserted(
- const http2::HpackStringPair& entry,
- size_t insert_count) {
- SPDY_DVLOG(2) << "HpackDecoderAdapter::ListenerAdapter::OnEntryInserted: "
- << entry << ", insert_count=" << insert_count;
- if (visitor_ == nullptr) {
- return 0;
- }
- HpackEntry hpack_entry(entry.name, entry.value,
- /*is_static*/ false, insert_count);
- int64_t time_added = visitor_->OnNewEntry(hpack_entry);
- SPDY_DVLOG(2)
- << "HpackDecoderAdapter::ListenerAdapter::OnEntryInserted: time_added="
- << time_added;
- return time_added;
-}
-
-void HpackDecoderAdapter::ListenerAdapter::OnUseEntry(
- const http2::HpackStringPair& entry,
- size_t insert_count,
- int64_t time_added) {
- SPDY_DVLOG(2) << "HpackDecoderAdapter::ListenerAdapter::OnUseEntry: " << entry
- << ", insert_count=" << insert_count
- << ", time_added=" << time_added;
- if (visitor_ != nullptr) {
- HpackEntry hpack_entry(entry.name, entry.value, /*is_static*/ false,
- insert_count);
- hpack_entry.set_time_added(time_added);
- visitor_->OnUseEntry(hpack_entry);
- }
-}
-
} // namespace spdy
diff --git a/spdy/core/hpack/hpack_decoder_adapter.h b/spdy/core/hpack/hpack_decoder_adapter.h
index 2a21fd0..abb0562 100644
--- a/spdy/core/hpack/hpack_decoder_adapter.h
+++ b/spdy/core/hpack/hpack_decoder_adapter.h
@@ -68,9 +68,6 @@
// a SpdyHeadersHandlerInterface.
const SpdyHeaderBlock& decoded_block() const;
- void SetHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor);
-
// Set how much encoded data this decoder is willing to buffer.
// TODO(jamessynge): Resolve definition of this value, as it is currently
// too tied to a single implementation. We probably want to limit one or more
@@ -92,8 +89,7 @@
private:
class QUICHE_EXPORT_PRIVATE ListenerAdapter
- : public http2::HpackDecoderListener,
- public http2::HpackDecoderTablesDebugListener {
+ : public http2::HpackDecoderListener {
public:
ListenerAdapter();
~ListenerAdapter() override;
@@ -105,22 +101,12 @@
void set_handler(SpdyHeadersHandlerInterface* handler);
const SpdyHeaderBlock& decoded_block() const { return decoded_block_; }
- void SetHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor);
-
// Override the HpackDecoderListener methods:
void OnHeaderListStart() override;
void OnHeader(const std::string& name, const std::string& value) override;
void OnHeaderListEnd() override;
void OnHeaderErrorDetected(absl::string_view error_message) override;
- // Override the HpackDecoderTablesDebugListener methods:
- int64_t OnEntryInserted(const http2::HpackStringPair& entry,
- size_t insert_count) override;
- void OnUseEntry(const http2::HpackStringPair& entry,
- size_t insert_count,
- int64_t time_added) override;
-
void AddToTotalHpackBytes(size_t delta) { total_hpack_bytes_ += delta; }
size_t total_hpack_bytes() const { return total_hpack_bytes_; }
@@ -138,10 +124,6 @@
// Total bytes of the name and value strings in the current HPACK block.
size_t total_uncompressed_bytes_;
-
- // visitor_ is used by a QUIC experiment regarding HPACK; remove
- // when the experiment is done.
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor_;
};
// Converts calls to HpackDecoderListener into calls to
diff --git a/spdy/core/hpack/hpack_encoder.h b/spdy/core/hpack/hpack_encoder.h
index b1b7741..70fade8 100644
--- a/spdy/core/hpack/hpack_encoder.h
+++ b/spdy/core/hpack/hpack_encoder.h
@@ -93,11 +93,6 @@
// this encoder.
void SetHeaderListener(HeaderListener listener) { listener_ = listener; }
- void SetHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor) {
- header_table_.set_debug_visitor(std::move(visitor));
- }
-
void DisableCompression() { enable_compression_ = false; }
// Returns the estimate of dynamically allocated memory in bytes.
diff --git a/spdy/core/hpack/hpack_entry.cc b/spdy/core/hpack/hpack_entry.cc
index e8db1d7..74debd3 100644
--- a/spdy/core/hpack/hpack_entry.cc
+++ b/spdy/core/hpack/hpack_entry.cc
@@ -21,22 +21,15 @@
name_ref_(name_),
value_ref_(value_),
insertion_index_(insertion_index),
- type_(is_static ? STATIC : DYNAMIC),
- time_added_(0) {}
+ type_(is_static ? STATIC : DYNAMIC) {}
HpackEntry::HpackEntry(absl::string_view name, absl::string_view value)
- : name_ref_(name),
- value_ref_(value),
- insertion_index_(0),
- type_(LOOKUP),
- time_added_(0) {}
+ : name_ref_(name), value_ref_(value), insertion_index_(0), type_(LOOKUP) {}
-HpackEntry::HpackEntry() : insertion_index_(0), type_(LOOKUP), time_added_(0) {}
+HpackEntry::HpackEntry() : insertion_index_(0), type_(LOOKUP) {}
HpackEntry::HpackEntry(const HpackEntry& other)
- : insertion_index_(other.insertion_index_),
- type_(other.type_),
- time_added_(0) {
+ : insertion_index_(other.insertion_index_), type_(other.type_) {
if (type_ == LOOKUP) {
name_ref_ = other.name_ref_;
value_ref_ = other.value_ref_;
diff --git a/spdy/core/hpack/hpack_entry.h b/spdy/core/hpack/hpack_entry.h
index 6141a42..736cb14 100644
--- a/spdy/core/hpack/hpack_entry.h
+++ b/spdy/core/hpack/hpack_entry.h
@@ -72,9 +72,6 @@
std::string GetDebugString() const;
- int64_t time_added() const { return time_added_; }
- void set_time_added(int64_t now) { time_added_ = now; }
-
// Returns the estimate of dynamically allocated memory in bytes.
size_t EstimateMemoryUsage() const;
@@ -99,9 +96,6 @@
size_t insertion_index_;
EntryType type_;
-
- // For HpackHeaderTable::DebugVisitorInterface
- int64_t time_added_;
};
} // namespace spdy
diff --git a/spdy/core/hpack/hpack_header_table.cc b/spdy/core/hpack/hpack_header_table.cc
index 4dac492..fe900b7 100644
--- a/spdy/core/hpack/hpack_header_table.cc
+++ b/spdy/core/hpack/hpack_header_table.cc
@@ -52,11 +52,7 @@
}
index -= static_entries_.size();
if (index < dynamic_entries_.size()) {
- const HpackEntry* result = &dynamic_entries_[index];
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnUseEntry(*result);
- }
- return result;
+ return &dynamic_entries_[index];
}
return nullptr;
}
@@ -71,11 +67,7 @@
{
NameToEntryMap::const_iterator it = dynamic_name_index_.find(name);
if (it != dynamic_name_index_.end()) {
- const HpackEntry* result = it->second;
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnUseEntry(*result);
- }
- return result;
+ return it->second;
}
}
return nullptr;
@@ -93,11 +85,7 @@
{
auto it = dynamic_index_.find(&query);
if (it != dynamic_index_.end()) {
- const HpackEntry* result = *it;
- if (debug_visitor_ != nullptr) {
- debug_visitor_->OnUseEntry(*result);
- }
- return result;
+ return *it;
}
}
return nullptr;
@@ -230,15 +218,6 @@
size_ += entry_size;
++total_insertions_;
- if (debug_visitor_ != nullptr) {
- // Call |debug_visitor_->OnNewEntry()| to get the current time.
- HpackEntry& entry = dynamic_entries_.front();
- entry.set_time_added(debug_visitor_->OnNewEntry(entry));
- SPDY_DVLOG(2) << "HpackHeaderTable::OnNewEntry: name=" << entry.name()
- << ", value=" << entry.value()
- << ", insert_index=" << entry.InsertionIndex()
- << ", time_added=" << entry.time_added();
- }
return &dynamic_entries_.front();
}
diff --git a/spdy/core/hpack/hpack_header_table.h b/spdy/core/hpack/hpack_header_table.h
index 02a438c..d522aaa 100644
--- a/spdy/core/hpack/hpack_header_table.h
+++ b/spdy/core/hpack/hpack_header_table.h
@@ -30,28 +30,6 @@
public:
friend class test::HpackHeaderTablePeer;
- // Debug visitor my be used to extract debug/internal information
- // about the HpackHeaderTable as it operates.
- //
- // Most HpackHeaderTable implementations do not need to bother with
- // this interface at all.
- class DebugVisitorInterface {
- public:
- virtual ~DebugVisitorInterface() {}
-
- // |OnNewEntry()| and |OnUseEntry()| can be used together to
- // gather data about the distribution of time intervals between
- // creation and reference of entries in the dynamic table. The
- // data is desired to sanity check a proposed extension to HPACK
- // for QUIC that would eliminate inter-stream head of line
- // blocking (due to standard HPACK). The visitor should return
- // the current time from |OnNewEntry()|, which will be passed
- // to |OnUseEntry()| each time that particular entry is used to
- // emit an indexed representation.
- virtual int64_t OnNewEntry(const HpackEntry& entry) = 0;
- virtual void OnUseEntry(const HpackEntry& entry) = 0;
- };
-
// HpackHeaderTable takes advantage of the deque property that references
// remain valid, so long as insertions & deletions are at the head & tail.
// This precludes the use of base::circular_deque.
@@ -124,10 +102,6 @@
void DebugLogTableState() const ABSL_ATTRIBUTE_UNUSED;
- void set_debug_visitor(std::unique_ptr<DebugVisitorInterface> visitor) {
- debug_visitor_ = std::move(visitor);
- }
-
// Returns the estimate of dynamically allocated memory in bytes.
size_t EstimateMemoryUsage() const;
@@ -173,8 +147,6 @@
// Total number of table insertions which have occurred. Referenced by
// IndexOf() for determination of an HpackEntry's table index.
size_t total_insertions_;
-
- std::unique_ptr<DebugVisitorInterface> debug_visitor_;
};
} // namespace spdy
diff --git a/spdy/core/http2_frame_decoder_adapter.cc b/spdy/core/http2_frame_decoder_adapter.cc
index 40ce2ff..c41f258 100644
--- a/spdy/core/http2_frame_decoder_adapter.cc
+++ b/spdy/core/http2_frame_decoder_adapter.cc
@@ -277,12 +277,6 @@
extension_ = visitor;
}
-// Passes the call on to the HPACK decoder.
-void Http2DecoderAdapter::SetDecoderHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor) {
- GetHpackDecoder()->SetHeaderTableDebugVisitor(std::move(visitor));
-}
-
size_t Http2DecoderAdapter::ProcessInput(const char* data, size_t len) {
size_t limit = recv_frame_size_limit_;
frame_decoder_->set_maximum_payload_size(limit);
diff --git a/spdy/core/http2_frame_decoder_adapter.h b/spdy/core/http2_frame_decoder_adapter.h
index 0bc428f..f05152b 100644
--- a/spdy/core/http2_frame_decoder_adapter.h
+++ b/spdy/core/http2_frame_decoder_adapter.h
@@ -127,10 +127,6 @@
return debug_visitor_;
}
- // Set debug callbacks to be called from the HPACK decoder.
- void SetDecoderHeaderTableDebugVisitor(
- std::unique_ptr<spdy::HpackHeaderTable::DebugVisitorInterface> visitor);
-
// Decode the |len| bytes of encoded HTTP/2 starting at |*data|. Returns
// the number of bytes consumed. It is safe to pass more bytes in than
// may be consumed. Should process (or otherwise buffer) as much as
diff --git a/spdy/core/spdy_framer.cc b/spdy/core/spdy_framer.cc
index 8b9d6ce..68cda07 100644
--- a/spdy/core/spdy_framer.cc
+++ b/spdy/core/spdy_framer.cc
@@ -1380,11 +1380,6 @@
}
}
-void SpdyFramer::SetEncoderHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor) {
- GetHpackEncoder()->SetHeaderTableDebugVisitor(std::move(visitor));
-}
-
size_t SpdyFramer::EstimateMemoryUsage() const {
return SpdyEstimateMemoryUsage(hpack_encoder_);
}
diff --git a/spdy/core/spdy_framer.h b/spdy/core/spdy_framer.h
index d7a81c1..4e3dc35 100644
--- a/spdy/core/spdy_framer.h
+++ b/spdy/core/spdy_framer.h
@@ -233,9 +233,6 @@
// Returns the maximum size of the header encoder compression table.
size_t header_encoder_table_size() const;
- void SetEncoderHeaderTableDebugVisitor(
- std::unique_ptr<HpackHeaderTable::DebugVisitorInterface> visitor);
-
// Get (and lazily initialize) the HPACK encoder state.
HpackEncoder* GetHpackEncoder();
