Refactor HPACK/QPACK lookup entry.
Before this CL, an HpackEntry could serve as a storage entry: to store strings
in a deque, for easy FIFO insertion and deletion; or as a reference entry used
for lookup: to point to strings stored in storage entries by means of
string_views, used in a set to allow convenient lookup by name and value without
having to store duplicate copies of strings.
This CL creates a dedicated HpackLookupEntry struct, so that the EntryType and
the string_view parts of HpackEntry can be removed in a future CL, substantially
reducing HpackEntry size and complexity.
Note that previously separate EntryHasher and EntriesEq functors were used,
mostly because until recently STL containers were used in Chromium and Absail
ones internally, and the hasher had to be defined as a template parameter to
work with both, and partially because conceptually two HpackEntries with same
string values were not equal, so operator==() would have been a lie. Now both
obstacles have been removed, allowing HpackLookupEntry to define operator==()
and an Absail-style hasher, eliminating the need for extra container template
parameters.
Also remove unused HpackHeaderTable::DebugLogTableState().
PiperOrigin-RevId: 363679331
Change-Id: Iad2f198c57e8e60c2b6e0e679b391d50d1dbcf7a
diff --git a/spdy/core/hpack/hpack_header_table.cc b/spdy/core/hpack/hpack_header_table.cc
index 006618d..18ae9a8 100644
--- a/spdy/core/hpack/hpack_header_table.cc
+++ b/spdy/core/hpack/hpack_header_table.cc
@@ -14,23 +14,6 @@
namespace spdy {
-size_t HpackHeaderTable::EntryHasher::operator()(
- const HpackEntry* entry) const {
- return absl::Hash<std::pair<absl::string_view, absl::string_view>>()(
- std::make_pair(entry->name(), entry->value()));
-}
-
-bool HpackHeaderTable::EntriesEq::operator()(const HpackEntry* lhs,
- const HpackEntry* rhs) const {
- if (lhs == nullptr) {
- return rhs == nullptr;
- }
- if (rhs == nullptr) {
- return false;
- }
- return lhs->name() == rhs->name() && lhs->value() == rhs->value();
-}
-
HpackHeaderTable::HpackHeaderTable()
: static_entries_(ObtainHpackStaticTable().GetStaticEntries()),
static_index_(ObtainHpackStaticTable().GetStaticIndex()),
@@ -76,17 +59,18 @@
size_t HpackHeaderTable::GetByNameAndValue(absl::string_view name,
absl::string_view value) {
- HpackEntry query(name, value);
+ HpackLookupEntry query{name, value};
{
- auto it = static_index_.find(&query);
+ auto it = static_index_.find(query);
if (it != static_index_.end()) {
- return 1 + (*it)->InsertionIndex();
+ return 1 + it->second->InsertionIndex();
}
}
{
- auto it = dynamic_index_.find(&query);
+ auto it = dynamic_index_.find(query);
if (it != dynamic_index_.end()) {
- return total_insertions_ - (*it)->InsertionIndex() + kStaticTableSize;
+ return total_insertions_ - it->second->InsertionIndex() +
+ kStaticTableSize;
}
}
return kHpackEntryNotFound;
@@ -143,12 +127,12 @@
HpackEntry* entry = &dynamic_entries_.back();
size_ -= entry->Size();
- auto it = dynamic_index_.find(entry);
+ auto it = dynamic_index_.find({entry->name(), entry->value()});
QUICHE_DCHECK(it != dynamic_index_.end());
// Only remove an entry from the index if its insertion index matches;
// otherwise, the index refers to another entry with the same name and
// value.
- if ((*it)->InsertionIndex() == entry->InsertionIndex()) {
+ if (it->second->InsertionIndex() == entry->InsertionIndex()) {
dynamic_index_.erase(it);
}
auto name_it = dynamic_name_index_.find(entry->name());
@@ -178,17 +162,20 @@
false, // is_static
total_insertions_);
HpackEntry* new_entry = &dynamic_entries_.front();
- auto index_result = dynamic_index_.insert(new_entry);
+ auto index_result = dynamic_index_.insert(std::make_pair(
+ HpackLookupEntry{new_entry->name(), new_entry->value()}, new_entry));
if (!index_result.second) {
// An entry with the same name and value already exists in the dynamic
// index. We should replace it with the newly added entry.
SPDY_DVLOG(1) << "Found existing entry: "
- << (*index_result.first)->GetDebugString()
+ << index_result.first->second->GetDebugString()
<< " replacing with: " << new_entry->GetDebugString();
QUICHE_DCHECK_GT(new_entry->InsertionIndex(),
- (*index_result.first)->InsertionIndex());
+ index_result.first->second->InsertionIndex());
dynamic_index_.erase(index_result.first);
- QUICHE_CHECK(dynamic_index_.insert(new_entry).second);
+ auto insert_result = dynamic_index_.insert(std::make_pair(
+ HpackLookupEntry{new_entry->name(), new_entry->value()}, new_entry));
+ QUICHE_CHECK(insert_result.second);
}
auto name_result =
@@ -213,29 +200,6 @@
return &dynamic_entries_.front();
}
-void HpackHeaderTable::DebugLogTableState() const {
- SPDY_DVLOG(2) << "Dynamic table:";
- for (auto it = dynamic_entries_.begin(); it != dynamic_entries_.end(); ++it) {
- SPDY_DVLOG(2) << " " << it->GetDebugString();
- }
- SPDY_DVLOG(2) << "Full Static Index:";
- for (const auto* entry : static_index_) {
- SPDY_DVLOG(2) << " " << entry->GetDebugString();
- }
- SPDY_DVLOG(2) << "Full Static Name Index:";
- for (const auto& it : static_name_index_) {
- SPDY_DVLOG(2) << " " << it.first << ": " << it.second->GetDebugString();
- }
- SPDY_DVLOG(2) << "Full Dynamic Index:";
- for (const auto* entry : dynamic_index_) {
- SPDY_DVLOG(2) << " " << entry->GetDebugString();
- }
- SPDY_DVLOG(2) << "Full Dynamic Name Index:";
- for (const auto& it : dynamic_name_index_) {
- SPDY_DVLOG(2) << " " << it.first << ": " << it.second->GetDebugString();
- }
-}
-
size_t HpackHeaderTable::EstimateMemoryUsage() const {
return SpdyEstimateMemoryUsage(dynamic_entries_) +
SpdyEstimateMemoryUsage(dynamic_index_) +
