From 92a23e80024333dbb7f9cb760aaad0a2fcf4c7d5 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 6 Jan 2025 14:24:57 -0800 Subject: [PATCH] Move part of the `TypeDefinedMapFieldBase` implementation into `MapFieldBase` implemented on top of `UntypedMapBase` visitation. Reduces code duplication in large binaries. More to come in future changes. PiperOrigin-RevId: 712658107 --- src/google/protobuf/map.cc | 73 ++++++++++++++++++++++++++++- src/google/protobuf/map.h | 4 ++ src/google/protobuf/map_field.cc | 5 ++ src/google/protobuf/map_field.h | 2 +- src/google/protobuf/map_field_inl.h | 8 ---- 5 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/google/protobuf/map.cc b/src/google/protobuf/map.cc index ea2716d5ab..328302c42c 100644 --- a/src/google/protobuf/map.cc +++ b/src/google/protobuf/map.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include "absl/base/optimization.h" @@ -30,6 +29,78 @@ namespace internal { NodeBase* const kGlobalEmptyTable[kGlobalEmptyTableSize] = {}; +void UntypedMapBase::UntypedMergeFrom(const UntypedMapBase& other) { + if (other.empty()) return; + + // Do the merging in steps to avoid Key*Value number of instantiations and + // reduce code duplication per instantation. + NodeBase* nodes = nullptr; + + // First, allocate all the nodes without types. + for (size_t i = 0; i < other.num_elements_; ++i) { + NodeBase* new_node = AllocNode(); + new_node->next = nodes; + nodes = new_node; + } + + // Then, copy the values. + VisitValueType([&](auto value_type) { + using Value = typename decltype(value_type)::type; + NodeBase* out_node = nodes; + + // Get the ClassData once to avoid redundant virtual function calls. + const internal::ClassData* class_data = + std::is_same_v + ? GetClassData(*other.GetValue(other.begin().node_)) + : nullptr; + + for (auto it = other.begin(); !it.Equals(EndIterator()); it.PlusPlus()) { + Value* out = GetValue(out_node); + out_node = out_node->next; + auto& in = *other.GetValue(it.node_); + if constexpr (std::is_same_v) { + class_data->PlacementNew(out, arena())->CheckTypeAndMergeFrom(in); + } else { + Arena::CreateInArenaStorage(out, this->arena_, in); + } + } + }); + + // Finally, copy the keys and insert the nodes. + VisitKeyType([&](auto key_type) { + using Key = typename decltype(key_type)::type; + for (auto it = other.begin(); !it.Equals(EndIterator()); it.PlusPlus()) { + NodeBase* node = nodes; + nodes = nodes->next; + const Key& in = *other.GetKey(it.node_); + Key* out = GetKey(node); + if (!internal::InitializeMapKey(out, in, this->arena_)) { + Arena::CreateInArenaStorage(out, this->arena_, in); + } + + static_cast*>(this)->InsertOrReplaceNode( + static_cast::KeyNode*>(node)); + } + }); +} + +void UntypedMapBase::UntypedSwap(UntypedMapBase& other) { + if (arena() == other.arena()) { + InternalSwap(&other); + } else { + UntypedMapBase tmp(arena_, type_info_); + InternalSwap(&tmp); + + ABSL_DCHECK(empty()); + UntypedMergeFrom(other); + + other.ClearTable(true, nullptr); + other.UntypedMergeFrom(tmp); + + if (arena_ == nullptr) tmp.ClearTable(false, nullptr); + } +} + void UntypedMapBase::DeleteNode(NodeBase* node) { const auto destroy = absl::Overload{ [](std::string* str) { str->~basic_string(); }, diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 1f6e5a6cf4..b9ee5ffd43 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -379,6 +379,9 @@ class PROTOBUF_EXPORT UntypedMapBase { std::swap(arena_, other->arena_); } + void UntypedMergeFrom(const UntypedMapBase& other); + void UntypedSwap(UntypedMapBase& other); + static size_type max_size() { return std::numeric_limits::max(); } @@ -674,6 +677,7 @@ class KeyMapBase : public UntypedMapBase { using KeyNode = internal::KeyNode; protected: + friend UntypedMapBase; friend class MapFieldBase; friend class TcParser; friend struct MapTestPeer; diff --git a/src/google/protobuf/map_field.cc b/src/google/protobuf/map_field.cc index e57e79c64c..8d030b8127 100644 --- a/src/google/protobuf/map_field.cc +++ b/src/google/protobuf/map_field.cc @@ -36,6 +36,11 @@ MapFieldBase::~MapFieldBase() { delete maybe_payload(); } +void MapFieldBase::SwapImpl(MapFieldBase& lhs, MapFieldBase& rhs) { + MapFieldBase::SwapPayload(lhs, rhs); + lhs.GetMapRaw().UntypedSwap(rhs.GetMapRaw()); +} + template auto VisitMapKey(const MapKey& map_key, Map& map, F f) { switch (map_key.type()) { diff --git a/src/google/protobuf/map_field.h b/src/google/protobuf/map_field.h index dab3529db9..c16a1f7f74 100644 --- a/src/google/protobuf/map_field.h +++ b/src/google/protobuf/map_field.h @@ -513,6 +513,7 @@ class PROTOBUF_EXPORT MapFieldBase : public MapFieldBaseForParse { const MapKey& map_key, MapValueRef* val); static bool DeleteMapValueImpl(MapFieldBase& self, const MapKey& map_key); + static void SwapImpl(MapFieldBase& lhs, MapFieldBase& rhs); private: friend class ContendedMapCleanTest; @@ -640,7 +641,6 @@ class TypeDefinedMapFieldBase : public MapFieldBase { using Iter = typename Map::const_iterator; static void MergeFromImpl(MapFieldBase& base, const MapFieldBase& other); - static void SwapImpl(MapFieldBase& lhs, MapFieldBase& rhs); // map_ is inside an anonymous union so we can explicitly control its // destruction diff --git a/src/google/protobuf/map_field_inl.h b/src/google/protobuf/map_field_inl.h index a42acd77a7..a6116321c6 100644 --- a/src/google/protobuf/map_field_inl.h +++ b/src/google/protobuf/map_field_inl.h @@ -34,14 +34,6 @@ namespace google { namespace protobuf { namespace internal { // ------------------------TypeDefinedMapFieldBase--------------- -template -void TypeDefinedMapFieldBase::SwapImpl(MapFieldBase& lhs, - MapFieldBase& rhs) { - MapFieldBase::SwapPayload(lhs, rhs); - static_cast(lhs).map_.swap( - static_cast(rhs).map_); -} - template void TypeDefinedMapFieldBase::MergeFromImpl(MapFieldBase& base, const MapFieldBase& other) {