diff --git a/bazel/envoy_library.bzl b/bazel/envoy_library.bzl index 9855f9a7bf565..59b66091363ae 100644 --- a/bazel/envoy_library.bzl +++ b/bazel/envoy_library.bzl @@ -105,8 +105,10 @@ def envoy_cc_library( deps = deps + [envoy_external_dep_path(dep) for dep in external_deps] + [ repository + "//include/envoy/common:base_includes", repository + "//source/common/common:fmt_lib", + envoy_external_dep_path("abseil_btree"), envoy_external_dep_path("abseil_flat_hash_map"), envoy_external_dep_path("abseil_flat_hash_set"), + envoy_external_dep_path("abseil_inlined_vector"), envoy_external_dep_path("abseil_strings"), envoy_external_dep_path("spdlog"), envoy_external_dep_path("fmtlib"), diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index f77167aeae289..3e0b70bdbdc80 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -438,6 +438,10 @@ def _com_google_absl(): name = "absl-base", actual = "@com_google_absl//absl/base", ) + native.bind( + name = "abseil_btree", + actual = "@com_google_absl//absl/container:btree", + ) native.bind( name = "abseil_flat_hash_map", actual = "@com_google_absl//absl/container:flat_hash_map", diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 9c65dfa1bc9b0..18a67e3fa1ebc 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -497,7 +497,8 @@ class HeaderMap { virtual uint64_t byteSize() const PURE; /** - * Get a header by key. + * Get the first instance of a header by key. Subsequent entries for the same key are ignored. + * * @param key supplies the header key. * @return the header entry if it exists otherwise nullptr. */ diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index ecc98017b8419..52b4aed1d9ff0 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -20,6 +20,7 @@ namespace { constexpr size_t MinDynamicCapacity{32}; // This includes the NULL (StringUtil::itoa technically only needs 21). constexpr size_t MaxIntegerLength{32}; +#define MAP_SIZE_THRESHOLD 0 uint64_t newCapacity(uint32_t existing_capacity, uint32_t size_to_append) { return (static_cast(existing_capacity) + size_to_append) * 2; @@ -289,10 +290,10 @@ struct HeaderMapImpl::StaticLookupTable : public TrieLookupTable { } }; -uint64_t HeaderMapImpl::appendToHeader(HeaderString& header, absl::string_view data, - absl::string_view delimiter) { +void HeaderMapImpl::HeaderList::appendToHeader(HeaderString& header, absl::string_view data, + absl::string_view delimiter) { if (data.empty()) { - return 0; + return; } uint64_t byte_size = 0; if (!header.empty()) { @@ -300,7 +301,7 @@ uint64_t HeaderMapImpl::appendToHeader(HeaderString& header, absl::string_view d byte_size += delimiter.size(); } header.append(data.data(), data.size()); - return data.size() + byte_size; + addSize(data.size() + byte_size); } HeaderMapImpl::HeaderMapImpl() { inline_headers_.clear(); } @@ -318,15 +319,15 @@ HeaderMapImpl::HeaderMapImpl( verifyByteSize(); } -void HeaderMapImpl::updateSize(uint64_t from_size, uint64_t to_size) { +void HeaderMapImpl::HeaderList::updateSize(uint64_t from_size, uint64_t to_size) { ASSERT(cached_byte_size_ >= from_size); cached_byte_size_ -= from_size; cached_byte_size_ += to_size; } -void HeaderMapImpl::addSize(uint64_t size) { cached_byte_size_ += size; } +void HeaderMapImpl::HeaderList::addSize(uint64_t size) { cached_byte_size_ += size; } -void HeaderMapImpl::subtractSize(uint64_t size) { +void HeaderMapImpl::HeaderList::subtractSize(uint64_t size) { ASSERT(cached_byte_size_ >= size); cached_byte_size_ -= size; } @@ -348,7 +349,7 @@ void HeaderMapImpl::copyFrom(const HeaderMap& header_map) { verifyByteSize(); } -bool HeaderMapImpl::operator==(const HeaderMapImpl& rhs) const { +bool HeaderMapImpl::HeaderList::operator==(const HeaderList& rhs) const { if (size() != rhs.size()) { return false; } @@ -362,8 +363,6 @@ bool HeaderMapImpl::operator==(const HeaderMapImpl& rhs) const { return true; } -bool HeaderMapImpl::operator!=(const HeaderMapImpl& rhs) const { return !operator==(rhs); } - void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) { EntryCb cb = ConstSingleton::get().find(key.getStringView()); if (cb) { @@ -372,14 +371,11 @@ void HeaderMapImpl::insertByKey(HeaderString&& key, HeaderString&& value) { if (*ref_lookup_response.entry_ == nullptr) { maybeCreateInline(ref_lookup_response.entry_, *ref_lookup_response.key_, std::move(value)); } else { - const uint64_t added_size = - appendToHeader((*ref_lookup_response.entry_)->value(), value.getStringView()); - addSize(added_size); + headers_.appendToHeader((*ref_lookup_response.entry_)->value(), value.getStringView()); value.clear(); } } else { - addSize(key.size() + value.size()); - std::list::iterator i = headers_.insert(std::move(key), std::move(value)); + HeaderNode i = headers_.insert(std::move(key), std::move(value)); i->entry_ = i; } } @@ -389,8 +385,7 @@ void HeaderMapImpl::addViaMove(HeaderString&& key, HeaderString&& value) { // the existing value. auto* entry = getExistingInline(key.getStringView()); if (entry != nullptr) { - const uint64_t added_size = appendToHeader(entry->value(), value.getStringView()); - addSize(added_size); + headers_.appendToHeader(entry->value(), value.getStringView()); key.clear(); value.clear(); } else { @@ -429,8 +424,7 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { if (entry != nullptr) { char buf[32]; StringUtil::itoa(buf, sizeof(buf), value); - const uint64_t added_size = appendToHeader(entry->value(), buf); - addSize(added_size); + headers_.appendToHeader(entry->value(), buf); return; } HeaderString new_key; @@ -446,8 +440,7 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { void HeaderMapImpl::addCopy(const LowerCaseString& key, absl::string_view value) { auto* entry = getExistingInline(key.get()); if (entry != nullptr) { - const uint64_t added_size = appendToHeader(entry->value(), value); - addSize(added_size); + headers_.appendToHeader(entry->value(), value); return; } HeaderString new_key; @@ -464,8 +457,7 @@ void HeaderMapImpl::appendCopy(const LowerCaseString& key, absl::string_view val // TODO(#9221): converge on and document a policy for coalescing multiple headers. auto* entry = getExisting(key); if (entry) { - const uint64_t added_size = appendToHeader(entry->value(), value); - addSize(added_size); + headers_.appendToHeader(entry->value(), value); } else { addCopy(key, value); } @@ -496,7 +488,7 @@ void HeaderMapImpl::setCopy(const LowerCaseString& key, absl::string_view value) // TODO(#9221): converge on and document a policy for coalescing multiple headers. auto* entry = getExisting(key); if (entry) { - updateSize(entry->value().size(), value.size()); + headers_.updateSize(entry->value().size(), value.size()); entry->value(value); } else { addCopy(key, value); @@ -504,9 +496,7 @@ void HeaderMapImpl::setCopy(const LowerCaseString& key, absl::string_view value) verifyByteSize(); } -uint64_t HeaderMapImpl::byteSize() const { return cached_byte_size_; } - -uint64_t HeaderMapImpl::byteSizeInternal() const { +uint64_t HeaderMapImpl::HeaderList::byteSizeInternal() const { // Computes the total byte size by summing the byte size of the keys and values. uint64_t byte_size = 0; for (const HeaderEntryImpl& header : headers_) { @@ -516,28 +506,54 @@ uint64_t HeaderMapImpl::byteSizeInternal() const { return byte_size; } +#if HEADER_MAP_USE_SLIST +void HeaderMapImpl::HeaderList::addSlistEntry(HeaderNode node) const { + HeaderSlistEntry& entry = lazy_map_[node->key().getStringView()]; + HeaderCellPtr cell = std::make_unique(); + cell->node_ = node; + if (entry.first_ == nullptr) { + entry.first_ = cell.get(); + } else { + cell->next_ = std::move(entry.list_); + } + entry.list_ = std::move(cell); +} +#endif + const HeaderEntry* HeaderMapImpl::get(const LowerCaseString& key) const { - for (const HeaderEntryImpl& header : headers_) { - if (header.key() == key.get().c_str()) { - return &header; + if (headers_.maybeMakeMap()) { + HeaderLazyMap::iterator iter = headers_.lazy_map_.find(key.get()); + if (iter != headers_.lazy_map_.end()) { +#if HEADER_MAP_USE_FLAT_HASH_MAP +#if HEADER_MAP_USE_SLIST + HeaderEntryImpl& header_entry = *(iter->second.first_->node_); +#else + const HeaderNodeVector& v = iter->second; + ASSERT(!v.empty()); // It's impossible to have a map entry with an empty vector as its value. + HeaderEntryImpl& header_entry = *v[0]; +#endif +#endif +#if HEADER_MAP_USE_MULTI_MAP + HeaderEntryImpl& header_entry = *(iter->second); +#endif + return &header_entry; + } + } else { + for (const HeaderEntryImpl& header : headers_.headers_) { + if (header.key() == key.get().c_str()) { + return &header; + } } } - return nullptr; } HeaderEntry* HeaderMapImpl::getExisting(const LowerCaseString& key) { - for (HeaderEntryImpl& header : headers_) { - if (header.key() == key.get().c_str()) { - return &header; - } - } - - return nullptr; + return const_cast(get(key)); } void HeaderMapImpl::iterate(ConstIterateCb cb, void* context) const { - for (const HeaderEntryImpl& header : headers_) { + for (const HeaderEntryImpl& header : headers_.headers_) { if (cb(header, context) == HeaderMap::Iterate::Break) { break; } @@ -545,7 +561,7 @@ void HeaderMapImpl::iterate(ConstIterateCb cb, void* context) const { } void HeaderMapImpl::iterateReverse(ConstIterateCb cb, void* context) const { - for (auto it = headers_.rbegin(); it != headers_.rend(); it++) { + for (auto it = headers_.headers_.rbegin(); it != headers_.headers_.rend(); it++) { if (cb(*it, context) == HeaderMap::Iterate::Break) { break; } @@ -578,7 +594,30 @@ HeaderMap::Lookup HeaderMapImpl::lookup(const LowerCaseString& key, void HeaderMapImpl::clear() { inline_headers_.clear(); headers_.clear(); - cached_byte_size_ = 0; +} + +bool HeaderMapImpl::HeaderList::maybeMakeMap() const { + if (lazy_map_.empty()) { +#if MAP_SIZE_THRESHOLD != 0 + if (headers_.size() < MAP_SIZE_THRESHOLD) { + return false; + } +#endif + for (auto node = headers_.begin(); node != headers_.end(); ++node) { +#if HEADER_MAP_USE_FLAT_HASH_MAP +#if HEADER_MAP_USE_SLIST + addSlistEntry(node); +#else + HeaderNodeVector& v = lazy_map_[node->key().getStringView()]; + v.push_back(node); +#endif +#endif +#if HEADER_MAP_USE_MULTI_MAP + lazy_map_.insert(std::make_pair(key, node)); +#endif + } + } + return true; } void HeaderMapImpl::remove(const LowerCaseString& key) { @@ -587,16 +626,64 @@ void HeaderMapImpl::remove(const LowerCaseString& key) { StaticLookupResponse ref_lookup_response = cb(*this); removeInline(ref_lookup_response.entry_); } else { - for (auto i = headers_.begin(); i != headers_.end();) { - if (i->key() == key.get().c_str()) { - subtractSize(i->key().size() + i->value().size()); - i = headers_.erase(i); + headers_.remove(key.get()); + } +} + +void HeaderMapImpl::HeaderList::remove(absl::string_view key) { + if (maybeMakeMap()) { + auto iter = lazy_map_.find(key); + if (iter != lazy_map_.end()) { +#if HEADER_MAP_USE_FLAT_HASH_MAP +#if HEADER_MAP_USE_SLIST + HeaderCellPtr cell = std::move(iter->second.list_); + do { + const HeaderNode& node = cell->node_; + ASSERT(node->key() == key); + erase(node, false /* clear_from_map */); + cell = std::move(cell->next_); + } while (cell != nullptr); + lazy_map_.erase(iter); +#else + HeaderNodeVector header_nodes = std::move(iter->second); + lazy_map_.erase(iter); + for (const HeaderNode& node : header_nodes) { + ASSERT(node->key() == key); + erase(node, false /* clear_from_map */); + } +#endif +#endif +#if HEADER_MAP_USE_MULTI_MAP + ASSERT(iter->second->key() == key); + do { + auto iter_to_erase = iter; + ++iter; + HeaderNode& node = iter_to_erase->second; + erase(node, false /* clear_from_map */); + lazy_map_.erase(iter_to_erase); + } while (iter != lazy_map_.end() && iter->second->key() == key); +#endif + } + } else { + for (HeaderNode i = headers_.begin(); i != headers_.end();) { + if (i->key() == key) { + i = erase(i, false); } else { ++i; } } } - verifyByteSize(); +} + +HeaderMapImpl::HeaderNode HeaderMapImpl::HeaderList::erase(HeaderNode i, bool clear_from_map) { + if (pseudo_headers_end_ == i) { + pseudo_headers_end_++; + } + subtractSize(i->key().size() + i->value().size()); + if (clear_from_map) { + lazy_map_.erase(i->key().getStringView()); + } + return headers_.erase(i); } void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { @@ -611,11 +698,11 @@ void HeaderMapImpl::removePrefix(const LowerCaseString& prefix) { if (ref_lookup_response.entry_) { const uint32_t key_value_size = (*ref_lookup_response.entry_)->key().size() + (*ref_lookup_response.entry_)->value().size(); - subtractSize(key_value_size); + headers_.subtractSize(key_value_size); *ref_lookup_response.entry_ = nullptr; } } else { - subtractSize(entry.key().size() + entry.value().size()); + headers_.subtractSize(entry.key().size() + entry.value().size()); } } return to_remove; @@ -643,8 +730,7 @@ HeaderMapImpl::HeaderEntryImpl& HeaderMapImpl::maybeCreateInline(HeaderEntryImpl return **entry; } - addSize(key.get().size()); - std::list::iterator i = headers_.insert(key); + HeaderNode i = headers_.insert(key); i->entry_ = i; *entry = &(*i); return **entry; @@ -658,8 +744,7 @@ HeaderMapImpl::HeaderEntryImpl& HeaderMapImpl::maybeCreateInline(HeaderEntryImpl return **entry; } - addSize(key.get().size() + value.size()); - std::list::iterator i = headers_.insert(key, std::move(value)); + HeaderNode i = headers_.insert(key, std::move(value)); i->entry_ = i; *entry = &(*i); return **entry; @@ -680,10 +765,8 @@ void HeaderMapImpl::removeInline(HeaderEntryImpl** ptr_to_entry) { } HeaderEntryImpl* entry = *ptr_to_entry; - const uint64_t size_to_subtract = entry->entry_->key().size() + entry->entry_->value().size(); - subtractSize(size_to_subtract); *ptr_to_entry = nullptr; - headers_.erase(entry->entry_); + headers_.erase(entry->entry_, true); verifyByteSize(); } diff --git a/source/common/http/header_map_impl.h b/source/common/http/header_map_impl.h index bba7221c7d367..9d64b21b4048a 100644 --- a/source/common/http/header_map_impl.h +++ b/source/common/http/header_map_impl.h @@ -1,8 +1,21 @@ #pragma once +#define HEADER_MAP_USE_MULTI_MAP false +#define HEADER_MAP_USE_FLAT_HASH_MAP true +#define HEADER_MAP_USE_BTREE false +#define HEADER_MAP_USE_SLIST true + #include #include #include + +#if HEADER_MAP_USE_MULTI_MAP +#if HEADER_MAP_USE_BTREE +#include "absl/container/btree_map.h" +#else +#include +#endif +#endif #include #include @@ -11,6 +24,13 @@ #include "common/common/non_copyable.h" #include "common/http/headers.h" +#if HEADER_MAP_USE_FLAT_HASH_MAP +#include "absl/container/flat_hash_map.h" +#if !HEADER_MAP_USE_SLIST +#include "absl/container/inlined_vector.h" +#endif +#endif + namespace Envoy { namespace Http { @@ -23,26 +43,26 @@ public: const HeaderEntry* name() const override { return inline_headers_.name##_; } \ void append##name(absl::string_view data, absl::string_view delimiter) override { \ HeaderEntry& entry = maybeCreateInline(&inline_headers_.name##_, Headers::get().name); \ - addSize(HeaderMapImpl::appendToHeader(entry.value(), data, delimiter)); \ + headers_.appendToHeader(entry.value(), data, delimiter); \ verifyByteSize(); \ } \ void setReference##name(absl::string_view value) override { \ HeaderEntry& entry = maybeCreateInline(&inline_headers_.name##_, Headers::get().name); \ - updateSize(entry.value().size(), value.size()); \ + headers_.updateSize(entry.value().size(), value.size()); \ entry.value().setReference(value); \ verifyByteSize(); \ } \ void set##name(absl::string_view value) override { \ HeaderEntry& entry = maybeCreateInline(&inline_headers_.name##_, Headers::get().name); \ - updateSize(entry.value().size(), value.size()); \ + headers_.updateSize(entry.value().size(), value.size()); \ entry.value().setCopy(value); \ verifyByteSize(); \ } \ void set##name(uint64_t value) override { \ HeaderEntry& entry = maybeCreateInline(&inline_headers_.name##_, Headers::get().name); \ - subtractSize(inline_headers_.name##_->value().size()); \ + headers_.subtractSize(inline_headers_.name##_->value().size()); \ entry.value().setInteger(value); \ - addSize(inline_headers_.name##_->value().size()); \ + headers_.addSize(inline_headers_.name##_->value().size()); \ verifyByteSize(); \ } \ void remove##name() override { removeInline(&inline_headers_.name##_); } @@ -73,8 +93,8 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { * For testing. Equality is based on equality of the backing list. This is an exact match * comparison (order matters). */ - bool operator==(const HeaderMapImpl& rhs) const; - bool operator!=(const HeaderMapImpl& rhs) const; + bool operator==(const HeaderMapImpl& rhs) const { return headers_ == rhs.headers_; } + bool operator!=(const HeaderMapImpl& rhs) const { return !(headers_ == rhs.headers_); } // Http::HeaderMap void addReference(const LowerCaseString& key, absl::string_view value) override; @@ -86,7 +106,7 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { void setReference(const LowerCaseString& key, absl::string_view value) override; void setReferenceKey(const LowerCaseString& key, absl::string_view value) override; void setCopy(const LowerCaseString& key, absl::string_view value) override; - uint64_t byteSize() const override; + uint64_t byteSize() const override { return headers_.byteSize(); } const HeaderEntry* get(const LowerCaseString& key) const override; void iterate(ConstIterateCb cb, void* context) const override; void iterateReverse(ConstIterateCb cb, void* context) const override; @@ -99,6 +119,46 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { void dumpState(std::ostream& os, int indent_level = 0) const override; protected: + struct HeaderEntryImpl; + using HeaderNode = std::list::iterator; +#if HEADER_MAP_USE_FLAT_HASH_MAP +#if HEADER_MAP_USE_SLIST + struct HeaderCell; + using HeaderCellPtr = std::unique_ptr; + struct HeaderCell { + HeaderNode node_; + HeaderCellPtr next_; + }; + // The mechanics of the single-linked list implementation result in the + // elements reversing in order as they are inserted. HeaderMap::get() + // requires we retrieve the first item. To avoid having to walk down + // the entire list on get(), we store a pointer to the first element as + // our map value, giving us instant access. We need access to the remaining + // occurrences of a key only for HeaderMap::remove(), which removes all + // the instances. + // + // We could simplify this code if it was feasible to change the semantics + // of get(), e.g. maybe we want get() to return all the elements too, or + // for setCopy() to leave only one element behind, instead of replacing + // only the first occurrence (which seems non-obvious). + struct HeaderSlistEntry { + HeaderCell* first_{nullptr}; + HeaderCellPtr list_; + }; + using HeaderLazyMap = absl::flat_hash_map; +#else + using HeaderNodeVector = absl::InlinedVector; + using HeaderLazyMap = absl::flat_hash_map; +#endif +#endif +#if HEADER_MAP_USE_MULTI_MAP +#if HEADER_MAP_USE_BTREE + using HeaderLazyMap = std::multimap; +#else + using HeaderLazyMap = absl::btree_multimap; +#endif +#endif + // For tests only, unoptimized, they aren't intended for regular HeaderMapImpl users. void copyFrom(const HeaderMap& rhs); @@ -117,7 +177,7 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { HeaderString key_; HeaderString value_; - std::list::iterator entry_; + HeaderNode entry_; }; struct StaticLookupResponse { @@ -152,30 +212,42 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { */ class HeaderList : NonCopyable { public: + friend HeaderMapImpl; + HeaderList() : pseudo_headers_end_(headers_.end()) {} template bool isPseudoHeader(const Key& key) { return !key.getStringView().empty() && key.getStringView()[0] == ':'; } - template - std::list::iterator insert(Key&& key, Value&&... value) { +#if HEADER_MAP_USE_SLIST + void addSlistEntry(HeaderNode node) const; +#endif + + template HeaderNode insert(Key&& key, Value&&... value) { const bool is_pseudo_header = isPseudoHeader(key); - std::list::iterator i = - headers_.emplace(is_pseudo_header ? pseudo_headers_end_ : headers_.end(), - std::forward(key), std::forward(value)...); + HeaderNode i = headers_.emplace(is_pseudo_header ? pseudo_headers_end_ : headers_.end(), + std::forward(key), std::forward(value)...); + addSize(i->key().size() + i->value().size()); + if (!lazy_map_.empty()) { +#if HEADER_MAP_USE_FLAT_HASH_MAP +#if HEADER_MAP_USE_SLIST + addSlistEntry(i); +#else + lazy_map_[i->key().getStringView()].push_back(i); +#endif +#endif +#if HEADER_MAP_USE_MULTI_MAP + lazy_map_.insert(std::make_pair(i->key().getStringView(), i)); +#endif + } if (!is_pseudo_header && pseudo_headers_end_ == headers_.end()) { pseudo_headers_end_ = i; } return i; } - std::list::iterator erase(std::list::iterator i) { - if (pseudo_headers_end_ == i) { - pseudo_headers_end_++; - } - return headers_.erase(i); - } + HeaderNode erase(HeaderNode i, bool clear_from_map); template void remove_if(UnaryPredicate p) { headers_.remove_if([&](const HeaderEntryImpl& entry) { @@ -187,29 +259,53 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { } return to_remove; }); + // It is possible to keep the lazy_map_ valid, but it's not clear whether + // it's worth it for this use case. Another option is to use a + // std::multi_map which would allow us to efficiently run this operation + // in the first place. + lazy_map_.clear(); } - std::list::iterator begin() { return headers_.begin(); } - std::list::iterator end() { return headers_.end(); } - std::list::const_iterator begin() const { return headers_.begin(); } - std::list::const_iterator end() const { return headers_.end(); } - std::list::const_reverse_iterator rbegin() const { return headers_.rbegin(); } - std::list::const_reverse_iterator rend() const { return headers_.rend(); } + // Makes a map. + bool maybeMakeMap() const; + + bool operator==(const HeaderList& rhs) const; size_t size() const { return headers_.size(); } bool empty() const { return headers_.empty(); } void clear() { headers_.clear(); + lazy_map_.clear(); + pseudo_headers_end_ = headers_.end(); + cached_byte_size_ = 0; pseudo_headers_end_ = headers_.end(); } + uint64_t byteSize() const { return cached_byte_size_; } + void remove(absl::string_view key); + + void verifyByteSize() { ASSERT(cached_byte_size_ == byteSizeInternal()); } + void addSize(uint64_t size); + void updateSize(uint64_t from_size, uint64_t to_size); + void subtractSize(uint64_t size); + void appendToHeader(HeaderString& header, absl::string_view data, + absl::string_view delimiter = ","); + // Performs a manual byte size count. + uint64_t byteSizeInternal() const; + private: - std::list headers_; - std::list::iterator pseudo_headers_end_; + // We make headers_ and lazy_map_ mutable to allow find() to populate lazy_map_, associating + // keys with a vector of nodes. We could also accomplish this by making HeaderMap::get() be + // non-const, or by using const_cast in he implementation. + mutable std::list headers_; + HeaderNode pseudo_headers_end_; + mutable HeaderLazyMap lazy_map_; + + // This holds the internal byte size of the HeaderMap. + uint64_t cached_byte_size_ = 0; + // Performs a manual byte size count. }; void insertByKey(HeaderString&& key, HeaderString&& value); - static uint64_t appendToHeader(HeaderString& header, absl::string_view data, - absl::string_view delimiter = ","); HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key); HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key, HeaderString&& value); @@ -217,17 +313,10 @@ class HeaderMapImpl : public HeaderMap, NonCopyable { HeaderEntryImpl* getExistingInline(absl::string_view key); void removeInline(HeaderEntryImpl** entry); - void updateSize(uint64_t from_size, uint64_t to_size); - void addSize(uint64_t size); - void subtractSize(uint64_t size); AllInlineHeaders inline_headers_; HeaderList headers_; - // This holds the internal byte size of the HeaderMap. - uint64_t cached_byte_size_ = 0; - // Performs a manual byte size count. - uint64_t byteSizeInternal() const; // In TestHeaderMapImpl and VerifiedHeaderMapImpl, this method is overridden to performs a // time-consuming manual byte size count on each operation to verify the byte size. For prod // HeaderMaps, this verification is skipped. diff --git a/source/extensions/quic_listeners/quiche/platform/BUILD b/source/extensions/quic_listeners/quiche/platform/BUILD index 8b03fa291ea79..3cb703c744a86 100644 --- a/source/extensions/quic_listeners/quiche/platform/BUILD +++ b/source/extensions/quic_listeners/quiche/platform/BUILD @@ -139,7 +139,6 @@ envoy_cc_library( external_deps = [ "abseil_base", "abseil_hash", - "abseil_inlined_vector", "abseil_memory", "abseil_node_hash_map", "abseil_node_hash_set", @@ -296,7 +295,6 @@ envoy_cc_library( external_deps = [ "abseil_base", "abseil_hash", - "abseil_inlined_vector", "abseil_memory", "abseil_str_format", ], diff --git a/test/common/http/header_map_impl_speed_test.cc b/test/common/http/header_map_impl_speed_test.cc index f313d26b4a173..6989b51517d7e 100644 --- a/test/common/http/header_map_impl_speed_test.cc +++ b/test/common/http/header_map_impl_speed_test.cc @@ -42,7 +42,7 @@ static void HeaderMapImplSetReference(benchmark::State& state) { } benchmark::DoNotOptimize(headers.size()); } -BENCHMARK(HeaderMapImplSetReference)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplSetReference)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of retrieving a header value. The numeric Arg passed by the @@ -64,7 +64,7 @@ static void HeaderMapImplGet(benchmark::State& state) { } benchmark::DoNotOptimize(successes); } -BENCHMARK(HeaderMapImplGet)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplGet)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the retrieval speed of a header for which HeaderMapImpl is expected to @@ -81,7 +81,7 @@ static void HeaderMapImplGetInline(benchmark::State& state) { } benchmark::DoNotOptimize(size); } -BENCHMARK(HeaderMapImplGetInline)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplGetInline)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of writing to a header for which HeaderMapImpl is expected to @@ -96,7 +96,7 @@ static void HeaderMapImplSetInlineMacro(benchmark::State& state) { } benchmark::DoNotOptimize(headers.size()); } -BENCHMARK(HeaderMapImplSetInlineMacro)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplSetInlineMacro)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of writing to a header for which HeaderMapImpl is expected to @@ -111,7 +111,7 @@ static void HeaderMapImplSetInlineInteger(benchmark::State& state) { } benchmark::DoNotOptimize(headers.size()); } -BENCHMARK(HeaderMapImplSetInlineInteger)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplSetInlineInteger)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** Measure the speed of the byteSize() estimation method. */ static void HeaderMapImplGetByteSize(benchmark::State& state) { @@ -123,7 +123,7 @@ static void HeaderMapImplGetByteSize(benchmark::State& state) { } benchmark::DoNotOptimize(size); } -BENCHMARK(HeaderMapImplGetByteSize)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplGetByteSize)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** Measure the speed of iteration with a lightweight callback. */ static void HeaderMapImplIterate(benchmark::State& state) { @@ -139,10 +139,10 @@ static void HeaderMapImplIterate(benchmark::State& state) { } benchmark::DoNotOptimize(num_callbacks); } -BENCHMARK(HeaderMapImplIterate)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplIterate)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); -/** Measure the speed of the HeaderMapImpl lookup() method. */ -static void HeaderMapImplLookup(benchmark::State& state) { +/** Measure the speed of the HeaderMapImpl lookup() method on an inline header */ +static void HeaderMapImplLookupInline(benchmark::State& state) { const LowerCaseString key("connection"); const std::string value("01234567890123456789"); HeaderMapImpl headers; @@ -154,7 +154,21 @@ static void HeaderMapImplLookup(benchmark::State& state) { benchmark::DoNotOptimize(result); } } -BENCHMARK(HeaderMapImplLookup)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplLookupInline)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); + +/** Measure the speed of the HeaderMapImpl lookup() method on a custom header. */ +static void HeaderMapImplGetCustom(benchmark::State& state) { + const LowerCaseString key("custom-header"); + const std::string value("01234567890123456789"); + HeaderMapImpl headers; + addDummyHeaders(headers, state.range(0)); + headers.addReference(key, value); + for (auto _ : state) { + auto result = headers.get(key); + benchmark::DoNotOptimize(result); + } +} +BENCHMARK(HeaderMapImplGetCustom)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of removing a header by key name. @@ -172,7 +186,7 @@ static void HeaderMapImplRemove(benchmark::State& state) { } benchmark::DoNotOptimize(headers.size()); } -BENCHMARK(HeaderMapImplRemove)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplRemove)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of removing a header by key name, for the special case of @@ -191,7 +205,7 @@ static void HeaderMapImplRemoveInline(benchmark::State& state) { } benchmark::DoNotOptimize(headers.size()); } -BENCHMARK(HeaderMapImplRemoveInline)->Arg(0)->Arg(1)->Arg(10)->Arg(50); +BENCHMARK(HeaderMapImplRemoveInline)->Arg(0)->Arg(1)->Arg(10)->Arg(50)->Arg(200); /** * Measure the speed of creating a HeaderMapImpl and populating it with a realistic diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 74f2d225e308d..cdc5b887ef60b 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -21,7 +21,7 @@ class VerifiedHeaderMapImpl : public HeaderMapImpl { const std::initializer_list>& values) : HeaderMapImpl(values) {} - void verifyByteSize() override { ASSERT(cached_byte_size_ == byteSizeInternal()); } + void verifyByteSize() override { headers_.verifyByteSize(); } }; TEST(HeaderStringTest, All) { diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 2483603de41c8..1077b3161e690 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -672,7 +672,7 @@ class TestHeaderMapImpl : public HeaderMapImpl { bool has(const std::string& key) const; bool has(const LowerCaseString& key) const; - void verifyByteSize() override { ASSERT(cached_byte_size_ == byteSizeInternal()); } + void verifyByteSize() override { headers_.verifyByteSize(); } }; // Helper method to create a header map from an initializer list. Useful due to make_unique's