diff --git a/rust/cpp.rs b/rust/cpp.rs index 8362bdb1f309..002567b2beeb 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -1017,10 +1017,6 @@ where fn to_view<'a>(key: Self::FfiKey) -> View<'a, Self>; - unsafe fn free(m: RawMap, prototype: MapValue); - - unsafe fn clear(m: RawMap, prototype: MapValue); - unsafe fn insert(m: RawMap, key: View<'_, Self>, value: MapValue) -> bool; unsafe fn get( @@ -1052,16 +1048,6 @@ macro_rules! generate_map_key_impl { $from_ffi(key) } - #[inline] - unsafe fn free(m: RawMap, prototype: MapValue) { - unsafe { [< proto2_rust_map_free_ $key >](m, prototype) } - } - - #[inline] - unsafe fn clear(m: RawMap, prototype: MapValue) { - unsafe { [< proto2_rust_map_clear_ $key >](m, prototype) } - } - #[inline] unsafe fn insert( m: RawMap, @@ -1126,13 +1112,13 @@ where unsafe fn map_free(_private: Private, map: &mut Map) { unsafe { - Key::free(map.as_raw(Private), Self::get_prototype()); + proto2_rust_map_free(map.as_raw(Private)); } } fn map_clear(mut map: MapMut) { unsafe { - Key::clear(map.as_raw(Private), Self::get_prototype()); + proto2_rust_map_clear(map.as_raw(Private)); } } @@ -1193,8 +1179,6 @@ where macro_rules! impl_map_primitives { (@impl $(($rust_type:ty, $cpp_type:ty) => [ - $free_thunk:ident, - $clear_thunk:ident, $insert_thunk:ident, $get_thunk:ident, $iter_get_thunk:ident, @@ -1202,14 +1186,6 @@ macro_rules! impl_map_primitives { ]),* $(,)?) => { $( extern "C" { - pub fn $free_thunk( - m: RawMap, - prototype: MapValue, - ); - pub fn $clear_thunk( - m: RawMap, - prototype: MapValue, - ); pub fn $insert_thunk( m: RawMap, key: $cpp_type, @@ -1235,8 +1211,6 @@ macro_rules! impl_map_primitives { paste!{ impl_map_primitives!(@impl $( ($rust_type, $cpp_type) => [ - [< proto2_rust_map_free_ $rust_type >], - [< proto2_rust_map_clear_ $rust_type >], [< proto2_rust_map_insert_ $rust_type >], [< proto2_rust_map_get_ $rust_type >], [< proto2_rust_map_iter_get_ $rust_type >], @@ -1260,6 +1234,8 @@ extern "C" { fn proto2_rust_thunk_UntypedMapIterator_increment(iter: &mut UntypedMapIterator); pub fn proto2_rust_map_new(key_prototype: MapValue, value_prototype: MapValue) -> RawMap; + pub fn proto2_rust_map_free(m: RawMap); + pub fn proto2_rust_map_clear(m: RawMap); pub fn proto2_rust_map_size(m: RawMap) -> usize; pub fn proto2_rust_map_iter(m: RawMap) -> UntypedMapIterator; } diff --git a/rust/cpp_kernel/map.cc b/rust/cpp_kernel/map.cc index 1c34cf2bfbf8..4f4786f945fb 100644 --- a/rust/cpp_kernel/map.cc +++ b/rust/cpp_kernel/map.cc @@ -279,25 +279,6 @@ size_t KeySize() { } } -template -void ClearMap(internal::UntypedMapBase* m, bool reset_table, - MapValue prototype) { - internal::MapNodeSizeInfoT size_info = GetSizeInfo(KeySize(), prototype); - if (internal::RustMapHelper::IsGlobalEmptyTable(m)) return; - uint8_t bits = 0; - if constexpr (std::is_same::value) { - bits |= internal::RustMapHelper::kKeyIsString; - } - if (prototype.tag == MapValueTag::kString) { - bits |= internal::RustMapHelper::kValueIsString; - } else if (prototype.tag == MapValueTag::kMessage) { - bits |= internal::RustMapHelper::kValueIsProto; - } - internal::RustMapHelper::ClearTable( - m, internal::RustMapHelper::ClearInput{size_info, bits, reset_table, - /* destroy_node = */ nullptr}); -} - } // namespace } // namespace rust } // namespace protobuf @@ -331,39 +312,39 @@ google::protobuf::internal::UntypedMapIterator proto2_rust_map_iter( return m->begin(); } -#define DEFINE_KEY_SPECIFIC_MAP_OPERATIONS(cpp_type, suffix) \ - void proto2_rust_map_free_##suffix(google::protobuf::internal::UntypedMapBase* m, \ - google::protobuf::rust::MapValue prototype) { \ - google::protobuf::rust::ClearMap(m, /* reset_table = */ false, prototype); \ - delete m; \ - } \ - void proto2_rust_map_clear_##suffix(google::protobuf::internal::UntypedMapBase* m, \ - google::protobuf::rust::MapValue prototype) { \ - google::protobuf::rust::ClearMap(m, /* reset_table = */ true, prototype); \ - } \ - bool proto2_rust_map_insert_##suffix(google::protobuf::internal::UntypedMapBase* m, \ - cpp_type key, \ - google::protobuf::rust::MapValue value) { \ - return google::protobuf::rust::Insert(m, key, value); \ - } \ - \ - bool proto2_rust_map_get_##suffix( \ - google::protobuf::internal::UntypedMapBase* m, google::protobuf::rust::MapValue prototype, \ - cpp_type key, google::protobuf::rust::MapValue* value) { \ - return google::protobuf::rust::Get(m, prototype, key, value); \ - } \ - \ - bool proto2_rust_map_remove_##suffix(google::protobuf::internal::UntypedMapBase* m, \ - google::protobuf::rust::MapValue prototype, \ - cpp_type key) { \ - return google::protobuf::rust::Remove(m, prototype, key); \ - } \ - \ - void proto2_rust_map_iter_get_##suffix( \ - const google::protobuf::internal::UntypedMapIterator* iter, \ - google::protobuf::rust::MapValue prototype, cpp_type* key, \ - google::protobuf::rust::MapValue* value) { \ - return google::protobuf::rust::IterGet(iter, prototype, key, value); \ +void proto2_rust_map_free(google::protobuf::internal::UntypedMapBase* m) { + m->ClearTable(false, nullptr); + delete m; +} + +void proto2_rust_map_clear(google::protobuf::internal::UntypedMapBase* m) { + m->ClearTable(true, nullptr); +} + +#define DEFINE_KEY_SPECIFIC_MAP_OPERATIONS(cpp_type, suffix) \ + bool proto2_rust_map_insert_##suffix(google::protobuf::internal::UntypedMapBase* m, \ + cpp_type key, \ + google::protobuf::rust::MapValue value) { \ + return google::protobuf::rust::Insert(m, key, value); \ + } \ + \ + bool proto2_rust_map_get_##suffix( \ + google::protobuf::internal::UntypedMapBase* m, google::protobuf::rust::MapValue prototype, \ + cpp_type key, google::protobuf::rust::MapValue* value) { \ + return google::protobuf::rust::Get(m, prototype, key, value); \ + } \ + \ + bool proto2_rust_map_remove_##suffix(google::protobuf::internal::UntypedMapBase* m, \ + google::protobuf::rust::MapValue prototype, \ + cpp_type key) { \ + return google::protobuf::rust::Remove(m, prototype, key); \ + } \ + \ + void proto2_rust_map_iter_get_##suffix( \ + const google::protobuf::internal::UntypedMapIterator* iter, \ + google::protobuf::rust::MapValue prototype, cpp_type* key, \ + google::protobuf::rust::MapValue* value) { \ + return google::protobuf::rust::IterGet(iter, prototype, key, value); \ } DEFINE_KEY_SPECIFIC_MAP_OPERATIONS(int32_t, i32) diff --git a/src/google/protobuf/map.cc b/src/google/protobuf/map.cc index 2b6ebcb5d92b..7299b887bc3c 100644 --- a/src/google/protobuf/map.cc +++ b/src/google/protobuf/map.cc @@ -29,7 +29,7 @@ namespace internal { NodeBase* const kGlobalEmptyTable[kGlobalEmptyTableSize] = {}; -void UntypedMapBase::ClearTable(const ClearInput input) { +void UntypedMapBase::ClearTableImpl(bool reset, void (*destroy)(NodeBase*)) { ABSL_DCHECK_NE(num_buckets_, kGlobalEmptyTableSize); if (alloc_.arena() == nullptr) { @@ -41,53 +41,43 @@ void UntypedMapBase::ClearTable(const ClearInput input) { NodeBase* next = node->next; absl::PrefetchToLocalCacheNta(next); destroy_node(node); - SizedDelete(node, SizeFromInfo(input.size_info)); + SizedDelete(node, type_info_.node_size); node = next; } } }; - switch (input.destroy_bits) { - case 0: - loop([](NodeBase*) {}); - break; - case kKeyIsString: - loop([](NodeBase* node) { - static_cast(node->GetVoidKey())->~basic_string(); - }); - break; - case kValueIsString: - loop([size_info = input.size_info](NodeBase* node) { - static_cast(node->GetVoidValue(size_info)) - ->~basic_string(); - }); - break; - case kKeyIsString | kValueIsString: - loop([size_info = input.size_info](NodeBase* node) { - static_cast(node->GetVoidKey())->~basic_string(); - static_cast(node->GetVoidValue(size_info)) - ->~basic_string(); - }); - break; - case kValueIsProto: - loop([size_info = input.size_info](NodeBase* node) { - static_cast(node->GetVoidValue(size_info)) - ->DestroyInstance(); - }); - break; - case kKeyIsString | kValueIsProto: - loop([size_info = input.size_info](NodeBase* node) { + + const auto dispatch_key = [&](auto value_handler) { + if (type_info_.key_type < TypeKind::kString) { + return loop(value_handler); + } else if (type_info_.key_type == TypeKind::kString) { + return loop([=](NodeBase* node) { static_cast(node->GetVoidKey())->~basic_string(); - static_cast(node->GetVoidValue(size_info)) - ->DestroyInstance(); + value_handler(node); }); - break; - case kUseDestructFunc: - loop(input.destroy_node); - break; + } else { + ABSL_CHECK(destroy != nullptr); + return loop(destroy); + } + }; + + if (type_info_.value_type < TypeKind::kString) { + dispatch_key([](NodeBase*) {}); + } else if (type_info_.value_type == TypeKind::kString) { + dispatch_key([&](NodeBase* node) { + GetValue(node)->~basic_string(); + }); + } else if (type_info_.value_type == TypeKind::kMessage) { + dispatch_key([&](NodeBase* node) { + GetValue(node)->DestroyInstance(); + }); + } else { + ABSL_CHECK(destroy != nullptr); + loop(destroy); } } - if (input.reset_table) { + if (reset) { std::fill(table_, table_ + num_buckets_, nullptr); num_elements_ = 0; index_of_first_non_null_ = num_buckets_; diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 1f17ffd4f6f0..be349d6197bd 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -457,6 +457,17 @@ class PROTOBUF_EXPORT UntypedMapBase { UntypedMapBase(const UntypedMapBase&) = delete; UntypedMapBase& operator=(const UntypedMapBase&) = delete; + template + T* GetValue(NodeBase* node) const { + return reinterpret_cast(reinterpret_cast(node) + + type_info_.value_offset); + } + + void ClearTable(bool reset, void (*destroy)(NodeBase*)) { + if (num_buckets_ == internal::kGlobalEmptyTableSize) return; + ClearTableImpl(reset, destroy); + } + protected: // 16 bytes is the minimum useful size for the array cache in the arena. enum : map_index_t { kMinTableSize = 16 / sizeof(void*) }; @@ -497,6 +508,8 @@ class PROTOBUF_EXPORT UntypedMapBase { map_index_t bucket; }; + void ClearTableImpl(bool reset, void (*destroy)(NodeBase*)); + // Returns whether we should insert after the head of the list. For // non-optimized builds, we randomly decide whether to insert right at the // head of the list or just after the head. This helps add a little bit of @@ -555,56 +568,21 @@ class PROTOBUF_EXPORT UntypedMapBase { return result; } - enum { - kKeyIsString = 1 << 0, - kValueIsString = 1 << 1, - kValueIsProto = 1 << 2, - kUseDestructFunc = 1 << 3, - }; - template - static constexpr uint8_t MakeDestroyBits() { - uint8_t result = 0; - if (!std::is_trivially_destructible::value) { - if (std::is_same::value) { - result |= kKeyIsString; - } else { - return kUseDestructFunc; - } - } - if (!std::is_trivially_destructible::value) { - if (std::is_same::value) { - result |= kValueIsString; - } else if (std::is_base_of::value) { - result |= kValueIsProto; - } else { - return kUseDestructFunc; - } - } - return result; - } - - struct ClearInput { - MapNodeSizeInfoT size_info; - uint8_t destroy_bits; - bool reset_table; - void (*destroy_node)(NodeBase*); - }; - template static void DestroyNode(NodeBase* node) { static_cast(node)->~Node(); } template - static constexpr ClearInput MakeClearInput(bool reset) { - constexpr auto bits = - MakeDestroyBits(); - return ClearInput{Node::size_info(), bits, reset, - bits & kUseDestructFunc ? DestroyNode : nullptr}; + static constexpr auto GetDestroyNode() { + return internal::UntypedMapBase::StaticTypeKind< + typename Node::key_type>() == TypeKind::kUnknown || + internal::UntypedMapBase::StaticTypeKind< + typename Node::mapped_type>() == TypeKind::kUnknown + ? DestroyNode + : nullptr; } - void ClearTable(ClearInput input); - // 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; @@ -907,7 +885,6 @@ bool InitializeMapKey(T*, K&&, Arena*) { class RustMapHelper { public: using NodeAndBucket = UntypedMapBase::NodeAndBucket; - using ClearInput = UntypedMapBase::ClearInput; static void GetSizeAndAlignment(const google::protobuf::MessageLite* m, uint16_t* size, uint8_t* alignment) { @@ -926,12 +903,6 @@ class RustMapHelper { return Map::Node::size_info(); } - enum { - kKeyIsString = UntypedMapBase::kKeyIsString, - kValueIsString = UntypedMapBase::kValueIsString, - kValueIsProto = UntypedMapBase::kValueIsProto, - }; - static NodeBase* AllocNode(UntypedMapBase* m, MapNodeSizeInfoT size_info) { return m->AllocNode(size_info); } @@ -963,10 +934,6 @@ class RustMapHelper { static void DestroyMessage(MessageLite* m) { m->DestroyInstance(); } - static void ClearTable(UntypedMapBase* m, ClearInput input) { - m->ClearTable(input); - } - static bool IsGlobalEmptyTable(const UntypedMapBase* m) { return m->num_buckets_ == kGlobalEmptyTableSize; } @@ -1049,9 +1016,7 @@ class Map : private internal::KeyMapBase> { // won't trigger for leaked maps that never get destructed. StaticValidityCheck(); - if (this->num_buckets_ != internal::kGlobalEmptyTableSize) { - this->ClearTable(this->template MakeClearInput(false)); - } + this->ClearTable(false, this->template GetDestroyNode()); } private: @@ -1391,8 +1356,7 @@ class Map : private internal::KeyMapBase> { } void clear() { - if (this->num_buckets_ == internal::kGlobalEmptyTableSize) return; - this->ClearTable(this->template MakeClearInput(true)); + this->ClearTable(true, this->template GetDestroyNode()); } // Assign