From bfbc95d41e52245e98eeb1100f3fbeea3e341efb Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 21 Sep 2023 12:06:12 -0700 Subject: [PATCH] Faster swap. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arena never changes, so no need to swap it. Also explicitly instantiated memswap template to reduce code size. ``` name old INSTRUCTIONS/op new INSTRUCTIONS/op delta BM_ProtoSwap 356 ± 0% 300 ± 0% -15.73% (p=0.000 n=100+100) BM_ProtoSwap 67.0 ± 0% 63.0 ± 0% -5.97% (p=0.000 n=100+100) BM_ProtoSwap 31.0 ± 0% 31.0 ± 0% ~ (all samples are equal) BM_ProtoSwap 52.0 ± 0% 52.0 ± 0% ~ (all samples are equal) ``` PiperOrigin-RevId: 567379442 --- src/google/protobuf/repeated_ptr_field.cc | 4 +++ src/google/protobuf/repeated_ptr_field.h | 38 ++++++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc index f9bf16db31..e9d70f0b59 100644 --- a/src/google/protobuf/repeated_ptr_field.cc +++ b/src/google/protobuf/repeated_ptr_field.cc @@ -147,6 +147,10 @@ void InternalOutOfLineDeleteMessageLite(MessageLite* message) { delete message; } +template PROTOBUF_EXPORT_TEMPLATE_DEFINE void +memswap::value>( + char* PROTOBUF_RESTRICT, char* PROTOBUF_RESTRICT); + } // namespace internal } // namespace protobuf diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index d8f8689b0f..9cde2cae0f 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -127,6 +127,14 @@ struct IsMovable : std::integral_constant::value && std::is_move_assignable::value> {}; +// A trait that tells offset of `T::arena_`. +// +// Do not use this struct - it exists for internal use only. +template +struct ArenaOffsetHelper { + constexpr static size_t value = offsetof(T, arena_); +}; + // This is the common base class for RepeatedPtrFields. It deals only in void* // pointers. Users should not use this interface directly. // @@ -160,15 +168,15 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { internal::GenericTypeHandler, Handler>::type; constexpr RepeatedPtrFieldBase() - : arena_(nullptr), + : tagged_rep_or_elem_(nullptr), current_size_(0), total_size_(kSSOCapacity), - tagged_rep_or_elem_(nullptr) {} + arena_(nullptr) {} explicit RepeatedPtrFieldBase(Arena* arena) - : arena_(arena), + : tagged_rep_or_elem_(nullptr), current_size_(0), total_size_(kSSOCapacity), - tagged_rep_or_elem_(nullptr) {} + arena_(arena) {} RepeatedPtrFieldBase(const RepeatedPtrFieldBase&) = delete; RepeatedPtrFieldBase& operator=(const RepeatedPtrFieldBase&) = delete; @@ -305,8 +313,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { inline void InternalSwap(RepeatedPtrFieldBase* PROTOBUF_RESTRICT rhs) { ABSL_DCHECK(this != rhs); - // Swap all fields at once. - internal::memswap( + // Swap all fields except arena pointer at once. + internal::memswap::value>( reinterpret_cast(this), reinterpret_cast(rhs)); } @@ -664,6 +672,11 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { friend class internal::TcParser; // TODO: Remove this friend. + // Expose offset of `arena_` without exposing the member itself. + // Used to optimize code size of `InternalSwap` method. + template + friend struct ArenaOffsetHelper; + // The reflection implementation needs to call protected methods directly, // reinterpreting pointers as being to Message instead of a specific Message // subclass. @@ -824,10 +837,10 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // misses due to the indirection, because these fields are checked frequently. // Placing all fields directly in the RepeatedPtrFieldBase instance would cost // significant performance for memory-sensitive workloads. - Arena* arena_; + void* tagged_rep_or_elem_; int current_size_; int total_size_; - void* tagged_rep_or_elem_; + Arena* arena_; }; void InternalOutOfLineDeleteMessageLite(MessageLite* message); @@ -2136,6 +2149,15 @@ UnsafeArenaAllocatedRepeatedPtrFieldBackInserter( mutable_field); } + +namespace internal { +// Size optimization for `memswap` - supplied below N is used by every +// `RepeatedPtrField`. +extern template PROTOBUF_EXPORT_TEMPLATE_DECLARE void +memswap::value>( + char* PROTOBUF_RESTRICT, char* PROTOBUF_RESTRICT); +} // namespace internal + } // namespace protobuf } // namespace google