diff --git a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs b/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs deleted file mode 100644 index 208ce1fcb6..0000000000 --- a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs +++ /dev/null @@ -1,17 +0,0 @@ -#region Copyright notice and license -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file or at -// https://developers.google.com/open-source/licenses/bsd -#endregion - -namespace Google.Protobuf.Reflection; - -internal sealed partial class FeatureSetDescriptor -{ - // Canonical serialized form of the edition defaults, generated by embed_edition_defaults. - private const string DefaultsBase64 = - "ChMYhAciACoMCAEQAhgCIAMoATACChMY5wciACoMCAIQARgBIAIoATABChMY6AciDAgBEAEYASACKAEwASoAIOYHKOgH"; -} diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 937894bb00..d71cc8713a 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -105,8 +105,6 @@ class DynamicMapKey { google::protobuf::MapKey ToMapKey() const ABSL_ATTRIBUTE_LIFETIME_BOUND; - VariantKey ToVariantKey() const ABSL_ATTRIBUTE_LIFETIME_BOUND; - bool IsString() const { return absl::holds_alternative(variant_); } @@ -133,35 +131,8 @@ inline void SetMapKey(MapKey* map_key, const DynamicMapKey& value) { template <> struct is_internal_map_key_type : std::true_type {}; -template <> -struct RealKeyToVariantKey : public RealKeyToVariantKey { - // Bring in for heterogeneous lookups. - using RealKeyToVariantKey::operator(); - - VariantKey operator()(const DynamicMapKey& value) const { - return value.ToVariantKey(); - } -}; - -template <> -struct RealKeyToVariantKeyAlternative - : public RealKeyToVariantKeyAlternative { - using RealKeyToVariantKeyAlternative::operator(); - - VariantKey operator()(const DynamicMapKey& value) const { - return RealKeyToVariantKey{}(value); - } -}; - template <> struct TransparentSupport { - using hash = absl::Hash; - - template - static bool Equals(T&& t, U&& u) { - return ToView(t) == ToView(u); - } - template using key_arg = K; @@ -222,10 +193,6 @@ google::protobuf::MapKey DynamicMapKey::ToMapKey() const { return result; } -VariantKey DynamicMapKey::ToVariantKey() const { - return absl::visit([](const auto& alt) { return VariantKey(alt); }, variant_); -} - class DynamicMapField final : public TypeDefinedMapFieldBase { public: diff --git a/src/google/protobuf/map.cc b/src/google/protobuf/map.cc index 00094aac1d..2ef7083a82 100644 --- a/src/google/protobuf/map.cc +++ b/src/google/protobuf/map.cc @@ -27,108 +27,19 @@ namespace google { namespace protobuf { namespace internal { -const TableEntryPtr kGlobalEmptyTable[kGlobalEmptyTableSize] = {}; - -NodeBase* UntypedMapBase::DestroyTree(Tree* tree) { - NodeBase* head = tree->empty() ? nullptr : tree->begin()->second; - if (alloc_.arena() == nullptr) { - delete tree; - } - return head; -} - -void UntypedMapBase::EraseFromTree(map_index_t b, - typename Tree::iterator tree_it) { - ABSL_DCHECK(TableEntryIsTree(b)); - Tree* tree = TableEntryToTree(table_[b]); - if (tree_it != tree->begin()) { - NodeBase* prev = std::prev(tree_it)->second; - prev->next = prev->next->next; - } - tree->erase(tree_it); - if (tree->empty()) { - DestroyTree(tree); - table_[b] = TableEntryPtr{}; - } -} - -void UntypedMapBase::InsertUniqueInTree(map_index_t b, GetKey get_key, - NodeBase* node) { - if (TableEntryIsNonEmptyList(b)) { - // To save in binary size, we delegate to an out-of-line function to do - // the conversion. - table_[b] = ConvertToTree(TableEntryToNode(table_[b]), get_key); - } - ABSL_DCHECK(TableEntryIsTree(b)) - << (void*)table_[b] << " " << (uintptr_t)table_[b]; - - Tree* tree = TableEntryToTree(table_[b]); - auto it = tree->try_emplace(get_key(node), node).first; - // Maintain the linked list of the nodes in the tree. - // For simplicity, they are in the same order as the tree iteration. - if (it != tree->begin()) { - NodeBase* prev = std::prev(it)->second; - prev->next = node; - } - auto next = std::next(it); - node->next = next != tree->end() ? next->second : nullptr; -} - -void UntypedMapBase::TransferTree(Tree* tree, GetKey get_key) { - NodeBase* node = DestroyTree(tree); - do { - NodeBase* next = node->next; - - map_index_t b = VariantBucketNumber(get_key(node)); - // This is similar to InsertUnique, but with erasure. - if (TableEntryIsEmpty(b)) { - InsertUniqueInList(b, node); - index_of_first_non_null_ = (std::min)(index_of_first_non_null_, b); - } else if (TableEntryIsNonEmptyList(b) && !TableEntryIsTooLong(b)) { - InsertUniqueInList(b, node); - } else { - InsertUniqueInTree(b, get_key, node); - } - - node = next; - } while (node != nullptr); -} - -TableEntryPtr UntypedMapBase::ConvertToTree(NodeBase* node, GetKey get_key) { - auto* tree = Arena::Create(alloc_.arena(), typename Tree::key_compare(), - typename Tree::allocator_type(alloc_)); - for (; node != nullptr; node = node->next) { - tree->try_emplace(get_key(node), node); - } - ABSL_DCHECK_EQ(MapTreeLengthThreshold(), tree->size()); - - // Relink the nodes. - NodeBase* next = nullptr; - auto it = tree->end(); - do { - node = (--it)->second; - node->next = next; - next = node; - } while (it != tree->begin()); - - return TreeToTableEntry(tree); -} +NodeBase* const kGlobalEmptyTable[kGlobalEmptyTableSize] = {}; void UntypedMapBase::ClearTable(const ClearInput input) { ABSL_DCHECK_NE(num_buckets_, kGlobalEmptyTableSize); if (alloc_.arena() == nullptr) { const auto loop = [&, this](auto destroy_node) { - const TableEntryPtr* table = table_; + NodeBase** table = table_; for (map_index_t b = index_of_first_non_null_, end = num_buckets_; b < end; ++b) { - NodeBase* node = - ABSL_PREDICT_FALSE(internal::TableEntryIsTree(table[b])) - ? DestroyTree(TableEntryToTree(table[b])) - : TableEntryToNode(table[b]); - - while (node != nullptr) { + for (NodeBase* node = table[b]; node != nullptr;) { NodeBase* next = node->next; + absl::PrefetchToLocalCacheNta(next); destroy_node(node); SizedDelete(node, SizeFromInfo(input.size_info)); node = next; @@ -177,7 +88,7 @@ void UntypedMapBase::ClearTable(const ClearInput input) { } if (input.reset_table) { - std::fill(table_, table_ + num_buckets_, TableEntryPtr{}); + std::fill(table_, table_ + num_buckets_, nullptr); num_elements_ = 0; index_of_first_non_null_ = num_buckets_; } else { @@ -185,31 +96,12 @@ void UntypedMapBase::ClearTable(const ClearInput input) { } } -auto UntypedMapBase::FindFromTree(map_index_t b, VariantKey key, - Tree::iterator* it) const -> NodeAndBucket { - Tree* tree = TableEntryToTree(table_[b]); - auto tree_it = tree->find(key); - if (it != nullptr) *it = tree_it; - if (tree_it != tree->end()) { - return {tree_it->second, b}; - } - return {nullptr, b}; -} - size_t UntypedMapBase::SpaceUsedInTable(size_t sizeof_node) const { size_t size = 0; // The size of the table. size += sizeof(void*) * num_buckets_; // All the nodes. size += sizeof_node * num_elements_; - // For each tree, count the overhead of those nodes. - // Two buckets at a time because we only care about trees. - for (map_index_t b = 0; b < num_buckets_; ++b) { - if (TableEntryIsTree(b)) { - size += sizeof(Tree); - size += sizeof(Tree::value_type) * TableEntryToTree(table_[b])->size(); - } - } return size; } diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 637926c2f2..48a467a069 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -221,13 +221,6 @@ struct TransparentSupport { static_assert(std::is_scalar::value, "Should only be used for ints."); - // We hash all the scalars as uint64_t so that we can implement the same hash - // function for VariantKey. This way we can have MapKey provide the same hash - // as the underlying value would have. - using hash = absl::Hash; - - static bool Equals(key_type a, key_type b) { return a == b; } - template using key_arg = key_type; @@ -255,22 +248,6 @@ struct TransparentSupport { } } - struct hash : public absl::Hash { - using is_transparent = void; - - template - size_t operator()(T&& str) const { - return absl::Hash::operator()( - ImplicitConvert(std::forward(str))); - } - }; - - template - static bool Equals(T&& t, U&& u) { - return ImplicitConvert(std::forward(t)) == - ImplicitConvert(std::forward(u)); - } - template using key_arg = K; @@ -307,133 +284,6 @@ struct NodeBase { } }; -inline NodeBase* EraseFromLinkedList(NodeBase* item, NodeBase* head) { - if (head == item) { - return head->next; - } else { - head->next = EraseFromLinkedList(item, head->next); - return head; - } -} - -constexpr size_t MapTreeLengthThreshold() { return 8; } -inline bool TableEntryIsTooLong(NodeBase* node) { - const size_t kMaxLength = MapTreeLengthThreshold(); - size_t count = 0; - do { - ++count; - node = node->next; - } while (node != nullptr); - // Invariant: no linked list ever is more than kMaxLength in length. - ABSL_DCHECK_LE(count, kMaxLength); - return count >= kMaxLength; -} - -// Similar to the public MapKey, but specialized for the internal -// implementation. -struct VariantKey { - // We make this value 16 bytes to make it cheaper to pass in the ABI. - // Can't overload string_view this way, so we unpack the fields. - // data==nullptr means this is a number and `integral` is the value. - // data!=nullptr means this is a string and `integral` is the size. - const char* data; - uint64_t integral; - - explicit VariantKey(uint64_t v) : data(nullptr), integral(v) {} - explicit VariantKey(absl::string_view v) - : data(v.data()), integral(v.size()) { - // We use `data` to discriminate between the types, so make sure it is never - // null here. - if (data == nullptr) data = ""; - } - - friend bool operator<(const VariantKey& left, const VariantKey& right) { - ABSL_DCHECK_EQ(left.data == nullptr, right.data == nullptr); - if (left.integral != right.integral) { - // If they are numbers with different value, or strings with different - // size, check the number only. - return left.integral < right.integral; - } - if (left.data == nullptr) { - // If they are numbers they have the same value, so return. - return false; - } - // They are strings of the same size, so check the bytes. - return memcmp(left.data, right.data, left.integral) < 0; - } -}; - -// This is to be specialized by MapKey. -template -struct RealKeyToVariantKey { - VariantKey operator()(T value) const { return VariantKey(value); } -}; - -template -struct RealKeyToVariantKeyAlternative; - -template -struct RealKeyToVariantKeyAlternative< - T, typename std::enable_if::value>::type> { - uint64_t operator()(uint64_t value) const { return value; } -}; - -template <> -struct RealKeyToVariantKey { - template - VariantKey operator()(const T& value) const { - return VariantKey(TransparentSupport::ImplicitConvert(value)); - } -}; - -template <> -struct RealKeyToVariantKeyAlternative { - absl::string_view operator()(absl::string_view value) const { return value; } -}; - -// We use a single kind of tree for all maps. This reduces code duplication. -using TreeForMap = - absl::btree_map, - MapAllocator>>; - -// Type safe tagged pointer. -// We convert to/from nodes and trees using the operations below. -// They ensure that the tags are used correctly. -// There are three states: -// - x == 0: the entry is empty -// - x != 0 && (x&1) == 0: the entry is a node list -// - x != 0 && (x&1) == 1: the entry is a tree -enum class TableEntryPtr : uintptr_t; - -inline bool TableEntryIsEmpty(TableEntryPtr entry) { - return entry == TableEntryPtr{}; -} -inline bool TableEntryIsTree(TableEntryPtr entry) { - return (static_cast(entry) & 1) == 1; -} -inline bool TableEntryIsList(TableEntryPtr entry) { - return !TableEntryIsTree(entry); -} -inline bool TableEntryIsNonEmptyList(TableEntryPtr entry) { - return !TableEntryIsEmpty(entry) && TableEntryIsList(entry); -} -inline NodeBase* TableEntryToNode(TableEntryPtr entry) { - ABSL_DCHECK(TableEntryIsList(entry)); - return reinterpret_cast(static_cast(entry)); -} -inline TableEntryPtr NodeToTableEntry(NodeBase* node) { - ABSL_DCHECK((reinterpret_cast(node) & 1) == 0); - return static_cast(reinterpret_cast(node)); -} -inline TreeForMap* TableEntryToTree(TableEntryPtr entry) { - ABSL_DCHECK(TableEntryIsTree(entry)); - return reinterpret_cast(static_cast(entry) - 1); -} -inline TableEntryPtr TreeToTableEntry(TreeForMap* node) { - ABSL_DCHECK((reinterpret_cast(node) & 1) == 0); - return static_cast(reinterpret_cast(node) | 1); -} - // This captures all numeric types. inline size_t MapValueSpaceUsedExcludingSelfLong(bool) { return 0; } inline size_t MapValueSpaceUsedExcludingSelfLong(const std::string& str) { @@ -446,8 +296,7 @@ size_t MapValueSpaceUsedExcludingSelfLong(const T& message) { } constexpr size_t kGlobalEmptyTableSize = 1; -PROTOBUF_EXPORT extern const TableEntryPtr - kGlobalEmptyTable[kGlobalEmptyTableSize]; +PROTOBUF_EXPORT extern NodeBase* const kGlobalEmptyTable[kGlobalEmptyTableSize]; template next == nullptr) { - SearchFrom(bucket_index_ + 1); - } else { - node_ = node_->next; - } - } + void PlusPlus(); // Conversion to and from a typed iterator child class is used by FFI. template @@ -550,7 +389,6 @@ static_assert( // parser) by having non-template code that can handle all instantiations. class PROTOBUF_EXPORT UntypedMapBase { using Allocator = internal::MapAllocator; - using Tree = internal::TreeForMap; public: using size_type = size_t; @@ -559,7 +397,7 @@ class PROTOBUF_EXPORT UntypedMapBase { : num_elements_(0), num_buckets_(internal::kGlobalEmptyTableSize), index_of_first_non_null_(internal::kGlobalEmptyTableSize), - table_(const_cast(internal::kGlobalEmptyTable)), + table_(const_cast(internal::kGlobalEmptyTable)), alloc_(arena) {} UntypedMapBase(const UntypedMapBase&) = delete; @@ -618,38 +456,6 @@ class PROTOBUF_EXPORT UntypedMapBase { #endif } - // Helper for InsertUnique. Handles the case where bucket b is a - // not-too-long linked list. - void InsertUniqueInList(map_index_t b, NodeBase* node) { - if (!TableEntryIsEmpty(b) && ShouldInsertAfterHead(node)) { - auto* first = TableEntryToNode(table_[b]); - node->next = first->next; - first->next = node; - } else { - node->next = TableEntryToNode(table_[b]); - table_[b] = NodeToTableEntry(node); - } - } - - bool TableEntryIsEmpty(map_index_t b) const { - return internal::TableEntryIsEmpty(table_[b]); - } - bool TableEntryIsNonEmptyList(map_index_t b) const { - return internal::TableEntryIsNonEmptyList(table_[b]); - } - bool TableEntryIsTree(map_index_t b) const { - return internal::TableEntryIsTree(table_[b]); - } - bool TableEntryIsList(map_index_t b) const { - return internal::TableEntryIsList(table_[b]); - } - - // Return whether table_[b] is a linked list that seems awfully long. - // Requires table_[b] to point to a non-empty linked list. - bool TableEntryIsTooLong(map_index_t b) { - return internal::TableEntryIsTooLong(TableEntryToNode(table_[b])); - } - // Return a power of two no less than max(kMinTableSize, n). // Assumes either n < kMinTableSize or n is a power of two. map_index_t TableSize(map_index_t n) { @@ -678,42 +484,18 @@ class PROTOBUF_EXPORT UntypedMapBase { AllocFor(alloc_).deallocate(node, node_size / sizeof(NodeBase)); } - void DeleteTable(TableEntryPtr* table, map_index_t n) { + void DeleteTable(NodeBase** table, map_index_t n) { if (auto* a = arena()) { - a->ReturnArrayMemory(table, n * sizeof(TableEntryPtr)); + a->ReturnArrayMemory(table, n * sizeof(NodeBase*)); } else { - internal::SizedDelete(table, n * sizeof(TableEntryPtr)); + internal::SizedDelete(table, n * sizeof(NodeBase*)); } } - NodeBase* DestroyTree(Tree* tree); - using GetKey = VariantKey (*)(NodeBase*); - void InsertUniqueInTree(map_index_t b, GetKey get_key, NodeBase* node); - void TransferTree(Tree* tree, GetKey get_key); - TableEntryPtr ConvertToTree(NodeBase* node, GetKey get_key); - void EraseFromTree(map_index_t b, typename Tree::iterator tree_it); - - map_index_t VariantBucketNumber(VariantKey key) const { - return key.data == nullptr - ? VariantBucketNumber(key.integral) - : VariantBucketNumber(absl::string_view( - key.data, static_cast(key.integral))); - } - - map_index_t VariantBucketNumber(absl::string_view key) const { - return static_cast(absl::HashOf(key, table_) & - (num_buckets_ - 1)); - } - - map_index_t VariantBucketNumber(uint64_t key) const { - return static_cast(absl::HashOf(key, table_) & - (num_buckets_ - 1)); - } - - TableEntryPtr* CreateEmptyTable(map_index_t n) { + NodeBase** CreateEmptyTable(map_index_t n) { ABSL_DCHECK_GE(n, kMinTableSize); ABSL_DCHECK_EQ(n & (n - 1), 0u); - TableEntryPtr* result = AllocFor(alloc_).allocate(n); + NodeBase** result = AllocFor(alloc_).allocate(n); memset(result, 0, n * sizeof(result[0])); return result; } @@ -768,9 +550,6 @@ class PROTOBUF_EXPORT UntypedMapBase { void ClearTable(ClearInput input); - NodeAndBucket FindFromTree(map_index_t b, VariantKey key, - Tree::iterator* it) const; - // Space used for the table, trees, and nodes. // Does not include the indirect space used. Eg the data of a std::string. size_t SpaceUsedInTable(size_t sizeof_node) const; @@ -778,7 +557,7 @@ class PROTOBUF_EXPORT UntypedMapBase { map_index_t num_elements_; map_index_t num_buckets_; map_index_t index_of_first_non_null_; - TableEntryPtr* table_; // an array with num_buckets_ entries + NodeBase** table_; // an array with num_buckets_ entries Allocator alloc_; }; @@ -790,31 +569,26 @@ inline UntypedMapIterator UntypedMapBase::begin() const { node = nullptr; } else { bucket_index = index_of_first_non_null_; - TableEntryPtr entry = table_[bucket_index]; - node = ABSL_PREDICT_TRUE(internal::TableEntryIsList(entry)) - ? TableEntryToNode(entry) - : TableEntryToTree(entry)->begin()->second; + node = table_[bucket_index]; PROTOBUF_ASSUME(node != nullptr); } return UntypedMapIterator{node, this, bucket_index}; } -inline void UntypedMapIterator::SearchFrom(map_index_t start_bucket) { - ABSL_DCHECK(m_->index_of_first_non_null_ == m_->num_buckets_ || - !m_->TableEntryIsEmpty(m_->index_of_first_non_null_)); - for (map_index_t i = start_bucket; i < m_->num_buckets_; ++i) { - TableEntryPtr entry = m_->table_[i]; - if (entry == TableEntryPtr{}) continue; +inline void UntypedMapIterator::PlusPlus() { + if (node_->next != nullptr) { + node_ = node_->next; + return; + } + + for (map_index_t i = bucket_index_ + 1; i < m_->num_buckets_; ++i) { + NodeBase* node = m_->table_[i]; + if (node == nullptr) continue; + node_ = node; bucket_index_ = i; - if (ABSL_PREDICT_TRUE(TableEntryIsList(entry))) { - node_ = TableEntryToNode(entry); - } else { - TreeForMap* tree = TableEntryToTree(entry); - ABSL_DCHECK(!tree->empty()); - node_ = tree->begin()->second; - } return; } + node_ = nullptr; bucket_index_ = 0; } @@ -862,11 +636,7 @@ struct KeyNode : NodeBase { decltype(auto) key() const { return ReadKey(GetVoidKey()); } }; -// KeyMapBase is a chaining hash map with the additional feature that some -// buckets can be converted to use an ordered container. This ensures O(lg n) -// bounds on find, insert, and erase, while avoiding the overheads of ordered -// containers most of the time. -// +// KeyMapBase is a chaining hash map. // The implementation doesn't need the full generality of unordered_map, // and it doesn't have it. More bells and whistles can be added as needed. // Some implementation details: @@ -874,16 +644,9 @@ struct KeyNode : NodeBase { // 2. As is typical for hash_map and such, the Keys and Values are always // stored in linked list nodes. Pointers to elements are never invalidated // until the element is deleted. -// 3. The trees' payload type is pointer to linked-list node. Tree-converting -// a bucket doesn't copy Key-Value pairs. -// 4. Once we've tree-converted a bucket, it is never converted back unless the -// bucket is completely emptied out. Note that the items a tree contains may -// wind up assigned to trees or lists upon a rehash. -// 5. Mutations to a map do not invalidate the map's iterators, pointers to +// 3. Mutations to a map do not invalidate the map's iterators, pointers to // elements, or references to elements. -// 6. Except for erase(iterator), any non-const method can reorder iterators. -// 7. Uses VariantKey when using the Tree representation, which holds all -// possible key types as a variant value. +// 4. Except for erase(iterator), any non-const method can reorder iterators. template class KeyMapBase : public UntypedMapBase { @@ -893,24 +656,11 @@ class KeyMapBase : public UntypedMapBase { using TS = TransparentSupport; public: - using hasher = typename TS::hash; - using UntypedMapBase::UntypedMapBase; protected: using KeyNode = internal::KeyNode; - // Trees. The payload type is a copy of Key, so that we can query the tree - // with Keys that are not in any particular data structure. - // The value is a void* pointing to Node. We use void* instead of Node* to - // avoid code bloat. That way there is only one instantiation of the tree - // class per key type. - using Tree = internal::TreeForMap; - using TreeIterator = typename Tree::iterator; - - public: - hasher hash_function() const { return {}; } - protected: friend class TcParser; friend struct MapTestPeer; @@ -919,39 +669,41 @@ class KeyMapBase : public UntypedMapBase { friend class v2::TableDriven; PROTOBUF_NOINLINE void erase_no_destroy(map_index_t b, KeyNode* node) { - TreeIterator tree_it; - const bool is_list = revalidate_if_necessary(b, node, &tree_it); - if (is_list) { - ABSL_DCHECK(TableEntryIsNonEmptyList(b)); - auto* head = TableEntryToNode(table_[b]); - head = EraseFromLinkedList(node, head); - table_[b] = NodeToTableEntry(head); - } else { - EraseFromTree(b, tree_it); + // Force bucket_index to be in range. + b &= (num_buckets_ - 1); + + const auto find_prev = [&] { + NodeBase** prev = table_ + b; + for (; *prev != nullptr && *prev != node; prev = &(*prev)->next) { + } + return prev; + }; + + NodeBase** prev = find_prev(); + if (*prev == nullptr) { + // The bucket index is wrong. The table was modified since the iterator + // was made, so let's find the new bucket. + b = FindHelper(TS::ToView(node->key())).bucket; + prev = find_prev(); } + ABSL_DCHECK_EQ(*prev, node); + *prev = (*prev)->next; + --num_elements_; if (ABSL_PREDICT_FALSE(b == index_of_first_non_null_)) { while (index_of_first_non_null_ < num_buckets_ && - TableEntryIsEmpty(index_of_first_non_null_)) { + table_[index_of_first_non_null_] == nullptr) { ++index_of_first_non_null_; } } } - NodeAndBucket FindHelper(typename TS::ViewType k, - TreeIterator* it = nullptr) const { + NodeAndBucket FindHelper(typename TS::ViewType k) const { map_index_t b = BucketNumber(k); - if (TableEntryIsNonEmptyList(b)) { - auto* node = internal::TableEntryToNode(table_[b]); - do { - if (TS::Equals(static_cast(node)->key(), k)) { - return {node, b}; - } else { - node = node->next; - } - } while (node != nullptr); - } else if (TableEntryIsTree(b)) { - return FindFromTree(b, internal::RealKeyToVariantKey{}(k), it); + for (auto* node = table_[b]; node != nullptr; node = node->next) { + if (TS::ToView(static_cast(node)->key()) == k) { + return {node, b}; + } } return {nullptr, b}; } @@ -981,27 +733,26 @@ class KeyMapBase : public UntypedMapBase { // bucket. num_elements_ is not modified. void InsertUnique(map_index_t b, KeyNode* node) { ABSL_DCHECK(index_of_first_non_null_ == num_buckets_ || - !TableEntryIsEmpty(index_of_first_non_null_)); + table_[index_of_first_non_null_] != nullptr); // In practice, the code that led to this point may have already // determined whether we are inserting into an empty list, a short list, // or whatever. But it's probably cheap enough to recompute that here; // it's likely that we're inserting into an empty or short list. ABSL_DCHECK(FindHelper(TS::ToView(node->key())).node == nullptr); - if (TableEntryIsEmpty(b)) { - InsertUniqueInList(b, node); + auto*& head = table_[b]; + if (head == nullptr) { + head = node; + node->next = nullptr; index_of_first_non_null_ = (std::min)(index_of_first_non_null_, b); - } else if (TableEntryIsNonEmptyList(b) && !TableEntryIsTooLong(b)) { - InsertUniqueInList(b, node); + } else if (ShouldInsertAfterHead(node)) { + node->next = head->next; + head->next = node; } else { - InsertUniqueInTree(b, NodeToVariantKey, node); + node->next = head; + head = node; } } - static VariantKey NodeToVariantKey(NodeBase* node) { - return internal::RealKeyToVariantKey{}( - static_cast(node)->key()); - } - // Have it a separate function for testing. static size_type CalculateHiCutoff(size_type num_buckets) { // We want the high cutoff to follow this rules: @@ -1071,58 +822,19 @@ class KeyMapBase : public UntypedMapBase { const map_index_t start = index_of_first_non_null_; index_of_first_non_null_ = num_buckets_; for (map_index_t i = start; i < old_table_size; ++i) { - if (internal::TableEntryIsNonEmptyList(old_table[i])) { - TransferList(static_cast(TableEntryToNode(old_table[i]))); - } else if (internal::TableEntryIsTree(old_table[i])) { - this->TransferTree(TableEntryToTree(old_table[i]), NodeToVariantKey); + for (KeyNode* node = static_cast(old_table[i]); + node != nullptr;) { + auto* next = static_cast(node->next); + InsertUnique(BucketNumber(TS::ToView(node->key())), node); + node = next; } } DeleteTable(old_table, old_table_size); } - // Transfer all nodes in the list `node` into `this`. - void TransferList(KeyNode* node) { - do { - auto* next = static_cast(node->next); - InsertUnique(BucketNumber(TS::ToView(node->key())), node); - node = next; - } while (node != nullptr); - } - map_index_t BucketNumber(typename TS::ViewType k) const { - ABSL_DCHECK_EQ( - VariantBucketNumber(RealKeyToVariantKeyAlternative{}(k)), - VariantBucketNumber(RealKeyToVariantKey{}(k))); - return VariantBucketNumber(RealKeyToVariantKeyAlternative{}(k)); - } - - // Assumes node_ and m_ are correct and non-null, but other fields may be - // stale. Fix them as needed. Then return true iff node_ points to a - // Node in a list. If false is returned then *it is modified to be - // a valid iterator for node_. - bool revalidate_if_necessary(map_index_t& bucket_index, KeyNode* node, - TreeIterator* it) const { - // Force bucket_index to be in range. - bucket_index &= (num_buckets_ - 1); - // Common case: the bucket we think is relevant points to `node`. - if (table_[bucket_index] == NodeToTableEntry(node)) return true; - // Less common: the bucket is a linked list with node_ somewhere in it, - // but not at the head. - if (TableEntryIsNonEmptyList(bucket_index)) { - auto* l = TableEntryToNode(table_[bucket_index]); - while ((l = l->next) != nullptr) { - if (l == node) { - return true; - } - } - } - // Well, bucket_index_ still might be correct, but probably - // not. Revalidate just to be sure. This case is rare enough that we - // don't worry about potential optimizations, such as having a custom - // find-like method that compares Node* instead of the key. - auto res = FindHelper(TS::ToView(node->key()), it); - bucket_index = res.bucket; - return TableEntryIsList(bucket_index); + return static_cast(absl::HashOf(k, table_) & + (num_buckets_ - 1)); } }; @@ -1238,7 +950,7 @@ class Map : private internal::KeyMapBase> { using const_reference = const value_type&; using size_type = size_t; - using hasher = typename TS::hash; + using hasher = absl::Hash; constexpr Map() : Base(nullptr) { StaticValidityCheck(); } Map(const Map& other) : Map(nullptr, other) {} @@ -1674,17 +1386,6 @@ class Map : private internal::KeyMapBase> { value_type kv; }; - using Tree = internal::TreeForMap; - using TreeIterator = typename Tree::iterator; - using TableEntryPtr = internal::TableEntryPtr; - - static Node* NodeFromTreeIterator(TreeIterator it) { - static_assert( - PROTOBUF_FIELD_OFFSET(Node, kv.first) == Base::KeyNode::kOffset, ""); - static_assert(alignof(Node) == alignof(internal::NodeBase), ""); - return static_cast(it->second); - } - void DestroyNode(Node* node) { if (this->alloc_.arena() == nullptr) { node->kv.first.~key_type(); diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index 0e18995476..161a6af748 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc @@ -29,26 +29,6 @@ namespace google { namespace protobuf { namespace internal { -VariantKey RealKeyToVariantKey::operator()(const MapKey& value) const { - switch (value.type()) { - case FieldDescriptor::CPPTYPE_STRING: - return VariantKey(value.GetStringValue()); - case FieldDescriptor::CPPTYPE_INT64: - return VariantKey(value.GetInt64Value()); - case FieldDescriptor::CPPTYPE_INT32: - return VariantKey(value.GetInt32Value()); - case FieldDescriptor::CPPTYPE_UINT64: - return VariantKey(value.GetUInt64Value()); - case FieldDescriptor::CPPTYPE_UINT32: - return VariantKey(value.GetUInt32Value()); - case FieldDescriptor::CPPTYPE_BOOL: - return VariantKey(static_cast(value.GetBoolValue())); - default: - Unreachable(); - return VariantKey(uint64_t{}); - } -} - MapFieldBase::~MapFieldBase() { ABSL_DCHECK_EQ(arena(), nullptr); delete maybe_payload(); diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index 81d8d64168..b54e30fe19 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -235,6 +235,26 @@ class PROTOBUF_EXPORT MapKey { friend class MapIterator; friend class internal::DynamicMapField; + template + friend auto AbslHashValue(H state, const MapKey& key) { + switch (key.type()) { + case FieldDescriptor::CPPTYPE_STRING: + return H::combine(std::move(state), key.GetStringValue()); + case FieldDescriptor::CPPTYPE_INT64: + return H::combine(std::move(state), key.GetInt64Value()); + case FieldDescriptor::CPPTYPE_INT32: + return H::combine(std::move(state), key.GetInt32Value()); + case FieldDescriptor::CPPTYPE_UINT64: + return H::combine(std::move(state), key.GetUInt64Value()); + case FieldDescriptor::CPPTYPE_UINT32: + return H::combine(std::move(state), key.GetUInt32Value()); + case FieldDescriptor::CPPTYPE_BOOL: + return H::combine(std::move(state), key.GetBoolValue()); + default: + internal::Unreachable(); + } + } + union KeyValue { KeyValue() {} absl::string_view string_value; @@ -257,18 +277,6 @@ namespace internal { template <> struct is_internal_map_key_type : std::true_type {}; -template <> -struct RealKeyToVariantKey { - VariantKey operator()(const MapKey& value) const; -}; - -template <> -struct RealKeyToVariantKeyAlternative { - VariantKey operator()(const MapKey& value) const { - return RealKeyToVariantKey{}(value); - } -}; - } // namespace internal namespace internal { diff --git a/src/google/protobuf/map_probe_benchmark.cc b/src/google/protobuf/map_probe_benchmark.cc index 550de51a4e..cee6d3ffd3 100644 --- a/src/google/protobuf/map_probe_benchmark.cc +++ b/src/google/protobuf/map_probe_benchmark.cc @@ -30,36 +30,16 @@ struct MapBenchmarkPeer { static double GetMeanProbeLength(const T& map) { double total_probe_cost = 0; for (map_index_t b = 0; b < map.num_buckets_; ++b) { - if (map.TableEntryIsList(b)) { - auto* node = internal::TableEntryToNode(map.table_[b]); - size_t cost = 0; - while (node != nullptr) { - total_probe_cost += static_cast(cost); - cost++; - node = node->next; - } - } else if (map.TableEntryIsTree(b)) { - // Overhead factor to account for more costly binary search. - constexpr double kTreeOverhead = 2.0; - size_t tree_size = TableEntryToTree(map.table_[b])->size(); - total_probe_cost += kTreeOverhead * static_cast(tree_size) * - std::log2(tree_size); + auto* node = map.table_[b]; + size_t cost = 0; + while (node != nullptr) { + total_probe_cost += static_cast(cost); + cost++; + node = node->next; } } return total_probe_cost / map.size(); } - - template - static double GetPercentTree(const T& map) { - size_t total_tree_size = 0; - for (map_index_t b = 0; b < map.num_buckets_; ++b) { - if (map.TableEntryIsTree(b)) { - total_tree_size += TableEntryToTree(map.table_[b])->size(); - } - } - return static_cast(total_tree_size) / - static_cast(map.size()); - } }; } // namespace protobuf } // namespace google::internal @@ -111,7 +91,6 @@ struct Ratios { double min_load; double avg_load; double max_load; - double percent_tree; }; template @@ -132,7 +111,6 @@ Ratios CollectMeanProbeLengths() { while (t.size() < min_max_sizes.max_load) t[elem()]; result.max_load = Peer::GetMeanProbeLength(t); - result.percent_tree = Peer::GetPercentTree(t); return result; } @@ -297,7 +275,6 @@ int main(int argc, char** argv) { print("min", &Ratios::min_load); print("avg", &Ratios::avg_load); print("max", &Ratios::max_load); - print("tree_percent", &Ratios::percent_tree); } absl::PrintF(" ],\n"); absl::PrintF(" \"context\": {\n"); diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc index 40233b75a7..7490e02392 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -87,6 +87,11 @@ struct MoveTestKey { MoveTestKey(MoveTestKey&& other) noexcept : data(other.data), copies(other.copies) {} + template + friend auto AbslHashValue(H state, const MoveTestKey& m) { + return H::combine(std::move(state), m.data); + } + friend bool operator==(const MoveTestKey& lhs, const MoveTestKey& rhs) { return lhs.data == rhs.data; } @@ -313,28 +318,10 @@ namespace google { namespace protobuf { namespace internal { -template <> -struct RealKeyToVariantKey { - VariantKey operator()(const MoveTestKey& value) const { - return VariantKey(value.data); - } -}; - -template <> -struct RealKeyToVariantKeyAlternative { - VariantKey operator()(const MoveTestKey& value) const { - return VariantKey(value.data); - } -}; - template <> struct TransparentSupport { using hash = absl::Hash; - static bool Equals(const MoveTestKey& a, const MoveTestKey& b) { - return a == b; - } - template using key_arg = MoveTestKey; @@ -505,7 +492,6 @@ static int k0 = 812398771; static int k1 = 1312938717; static int k2 = 1321555333; - TEST_F(MapImplTest, CopyIteratorStressTest) { std::vector::iterator> v; const int kIters = 1e5;