From be9eb4bccf48f4d6dcebe27f90102cbb1826c63a Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 8 Oct 2024 08:32:10 -0700 Subject: [PATCH] Refactor the code to consistently use the "base" implementation for copying objects in the slow path. It reduces binary size. Some calls already took care of doing so, but it is easy to miss. Remove the now unused NewFromPrototype and Merge functions from the traits. PiperOrigin-RevId: 683634339 --- src/google/protobuf/message.cc | 11 ------ src/google/protobuf/message_lite.cc | 10 ----- src/google/protobuf/repeated_ptr_field.cc | 9 +++++ src/google/protobuf/repeated_ptr_field.h | 47 ++++------------------- 4 files changed, 16 insertions(+), 61 deletions(-) diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index 3870efcd90..6020fc6ef1 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -502,17 +502,6 @@ template <> // Note: force noinline to workaround MSVC compiler bug with /Zc:inline, issue // #240 PROTOBUF_NOINLINE -#endif - Message* - GenericTypeHandler::NewFromPrototype(const Message* prototype, - Arena* arena) { - return prototype->New(arena); -} -template <> -#if defined(_MSC_VER) && (_MSC_VER >= 1800) -// Note: force noinline to workaround MSVC compiler bug with /Zc:inline, issue -// #240 -PROTOBUF_NOINLINE #endif Arena* GenericTypeHandler::GetArena(Message* value) { diff --git a/src/google/protobuf/message_lite.cc b/src/google/protobuf/message_lite.cc index 1a2c6bd88b..88dc8a0f9b 100644 --- a/src/google/protobuf/message_lite.cc +++ b/src/google/protobuf/message_lite.cc @@ -713,16 +713,6 @@ absl::Cord MessageLite::SerializePartialAsCord() const { namespace internal { -MessageLite* NewFromPrototypeHelper(const MessageLite* prototype, - Arena* arena) { - return prototype->New(arena); -} -template <> -void GenericTypeHandler::Merge(const MessageLite& from, - MessageLite* to) { - to->CheckTypeAndMergeFrom(from); -} - // Non-inline variants of std::string specializations for // various InternalMetadata routines. template <> diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc index 517230563c..58d85ea9f6 100644 --- a/src/google/protobuf/repeated_ptr_field.cc +++ b/src/google/protobuf/repeated_ptr_field.cc @@ -33,6 +33,15 @@ namespace protobuf { namespace internal { +MessageLite* CloneSlow(Arena* arena, const MessageLite& value) { + auto* msg = value.New(arena); + msg->CheckTypeAndMergeFrom(value); + return msg; +} +std::string* CloneSlow(Arena* arena, const std::string& value) { + return Arena::Create(arena, value); +} + void** RepeatedPtrFieldBase::InternalExtend(int extend_amount) { ABSL_DCHECK(extend_amount > 0); constexpr size_t kPtrSize = sizeof(rep()->elements[0]); diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index 12ccb38025..605927f8c2 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -89,6 +89,11 @@ struct ArenaOffsetHelper { static constexpr size_t value = offsetof(T, arena_); }; +// Copies the object in the arena. +// Used in the slow path. Out-of-line for lower binary size cost. +PROTOBUF_EXPORT MessageLite* CloneSlow(Arena* arena, const MessageLite& value); +PROTOBUF_EXPORT std::string* CloneSlow(Arena* arena, const std::string& value); + // Defined further below. template class GenericTypeHandler; @@ -107,10 +112,8 @@ class GenericTypeHandler; // // static Type* New(Arena* arena); // static Type* New(Arena* arena, Type&& value); -// static Type* NewFromPrototype(const Type* prototype, Arena* arena); // static void Delete(Type*, Arena* arena); // static void Clear(Type*); -// static void Merge(const Type& from, Type* to); // // // Only needs to be implemented if SpaceUsedExcludingSelf() is called. // static int SpaceUsedLong(const Type&); @@ -331,10 +334,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template static inline Value* copy(const Value* value) { - using H = CommonHandler; - auto* new_value = H::NewFromPrototype(value, nullptr); - H::Merge(*value, new_value); - return cast(new_value); + return cast(CloneSlow(nullptr, *value)); } // Used for constructing iterators. @@ -499,9 +499,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { my_arena->Own(value); } else if (my_arena != value_arena) { ABSL_DCHECK(value_arena != nullptr); - auto* new_value = TypeHandler::NewFromPrototype(value, my_arena); - H::Merge(*value, new_value); - value = new_value; + value = cast(CloneSlow(my_arena, *value)); } UnsafeArenaAddAllocated(value); @@ -824,10 +822,6 @@ class GenericTypeHandler { static inline Type* New(Arena* arena, Type&& value) { return Arena::Create(arena, std::move(value)); } - static inline Type* NewFromPrototype(const Type* /*prototype*/, - Arena* arena = nullptr) { - return New(arena); - } static inline void Delete(Type* value, Arena* arena) { if (arena != nullptr) return; #ifdef __cpp_if_constexpr @@ -843,44 +837,20 @@ class GenericTypeHandler { #endif } static inline void Clear(Type* value) { value->Clear(); } - static void Merge(const Type& from, Type* to); static inline size_t SpaceUsedLong(const Type& value) { return value.SpaceUsedLong(); } }; -// NewFromPrototypeHelper() is not defined inline here, as we will need to do a -// virtual function dispatch anyways to go from Message* to call New/Merge. (The -// additional helper is needed as a workaround for MSVC.) -PROTOBUF_EXPORT MessageLite* NewFromPrototypeHelper( - const MessageLite* prototype, Arena* arena); - -template <> -inline MessageLite* GenericTypeHandler::NewFromPrototype( - const MessageLite* prototype, Arena* arena) { - return NewFromPrototypeHelper(prototype, arena); -} template <> inline Arena* GenericTypeHandler::GetArena(MessageLite* value) { return value->GetArena(); } -template -PROTOBUF_NOINLINE inline void GenericTypeHandler::Merge( - const GenericType& from, GenericType* to) { - to->MergeFrom(from); -} -template <> -PROTOBUF_EXPORT void GenericTypeHandler::Merge( - const MessageLite& from, MessageLite* to); - // Message specialization bodies defined in message.cc. This split is necessary // to allow proto2-lite (which includes this header) to be independent of // Message. template <> -PROTOBUF_EXPORT Message* GenericTypeHandler::NewFromPrototype( - const Message* prototype, Arena* arena); -template <> PROTOBUF_EXPORT Arena* GenericTypeHandler::GetArena(Message* value); PROTOBUF_EXPORT void* NewStringElement(Arena* arena); @@ -899,9 +869,6 @@ class GenericTypeHandler { static PROTOBUF_NOINLINE Type* New(Arena* arena, Type&& value) { return Arena::Create(arena, std::move(value)); } - static inline Type* NewFromPrototype(const Type*, Arena* arena) { - return New(arena); - } static inline void Delete(Type* value, Arena* arena) { if (arena == nullptr) { delete value;