diff --git a/src/google/protobuf/arena_impl.h b/src/google/protobuf/arena_impl.h index d02ad93985ed..76727688b558 100644 --- a/src/google/protobuf/arena_impl.h +++ b/src/google/protobuf/arena_impl.h @@ -55,6 +55,13 @@ namespace google { namespace protobuf { namespace internal { +// To prevent sharing cache lines between threads +#ifdef __cpp_aligned_new +enum { kCacheAlignment = 64 }; +#else +enum { kCacheAlignment = alignof(max_align_t) }; // do the best we can +#endif + inline constexpr size_t AlignUpTo8(size_t n) { // Align n to next multiple of 8 (from Hacker's Delight, Chapter 3.) return (n + 7) & static_cast(-8); @@ -497,10 +504,10 @@ class PROTOBUF_EXPORT ThreadSafeArena { // have fallback function calls in tail position. This substantially improves // code for the happy path. PROTOBUF_NDEBUG_INLINE bool MaybeAllocateAligned(size_t n, void** out) { - SerialArena* a; + SerialArena* arena; if (PROTOBUF_PREDICT_TRUE(!alloc_policy_.should_record_allocs() && - GetSerialArenaFromThreadCache(&a))) { - return a->MaybeAllocateAligned(n, out); + GetSerialArenaFromThreadCache(&arena))) { + return arena->MaybeAllocateAligned(n, out); } return false; } @@ -564,7 +571,7 @@ class PROTOBUF_EXPORT ThreadSafeArena { // fast path optimizes the case where a single thread uses multiple arenas. ThreadCache* tc = &thread_cache(); SerialArena* serial = hint_.load(std::memory_order_acquire); - if (PROTOBUF_PREDICT_TRUE(serial != NULL && serial->owner() == tc)) { + if (PROTOBUF_PREDICT_TRUE(serial != nullptr && serial->owner() == tc)) { *arena = serial; return true; } @@ -602,7 +609,7 @@ class PROTOBUF_EXPORT ThreadSafeArena { #ifdef _MSC_VER #pragma warning(disable : 4324) #endif - struct alignas(64) ThreadCache { + struct alignas(kCacheAlignment) ThreadCache { #if defined(GOOGLE_PROTOBUF_NO_THREADLOCAL) // If we are using the ThreadLocalStorage class to store the ThreadCache, // then the ThreadCache's default constructor has to be responsible for @@ -610,7 +617,7 @@ class PROTOBUF_EXPORT ThreadSafeArena { ThreadCache() : next_lifecycle_id(0), last_lifecycle_id_seen(-1), - last_serial_arena(NULL) {} + last_serial_arena(nullptr) {} #endif // Number of per-thread lifecycle IDs to reserve. Must be power of two. @@ -633,7 +640,7 @@ class PROTOBUF_EXPORT ThreadSafeArena { #ifdef _MSC_VER #pragma warning(disable : 4324) #endif - struct alignas(64) CacheAlignedLifecycleIdGenerator { + struct alignas(kCacheAlignment) CacheAlignedLifecycleIdGenerator { std::atomic id; }; static CacheAlignedLifecycleIdGenerator lifecycle_id_generator_; diff --git a/src/google/protobuf/arenastring.cc b/src/google/protobuf/arenastring.cc index d886ddad66a5..3887c940a3e1 100644 --- a/src/google/protobuf/arenastring.cc +++ b/src/google/protobuf/arenastring.cc @@ -187,7 +187,7 @@ std::string* ArenaStringPtr::Release() { if (IsDefault()) return nullptr; std::string* released = tagged_ptr_.Get(); - if (!tagged_ptr_.IsAllocated()) { + if (tagged_ptr_.IsArena()) { released = tagged_ptr_.IsMutable() ? new std::string(std::move(*released)) : new std::string(*released); } @@ -216,9 +216,7 @@ void ArenaStringPtr::SetAllocated(std::string* value, Arena* arena) { } void ArenaStringPtr::Destroy() { - if (tagged_ptr_.IsAllocated()) { - delete tagged_ptr_.Get(); - } + delete tagged_ptr_.GetIfAllocated(); } void ArenaStringPtr::ClearToEmpty() { diff --git a/src/google/protobuf/arenastring.h b/src/google/protobuf/arenastring.h index b8d31fec015b..6bc8395f233f 100644 --- a/src/google/protobuf/arenastring.h +++ b/src/google/protobuf/arenastring.h @@ -96,13 +96,12 @@ class PROTOBUF_EXPORT LazyString { class TaggedStringPtr { public: - // Bit flags qualifying string properties. We can use up to 3 bits as - // ptr_ is guaranteed and enforced to be aligned on 8 byte boundaries. + // Bit flags qualifying string properties. We can use 2 bits as + // ptr_ is guaranteed and enforced to be aligned on 4 byte boundaries. enum Flags { kArenaBit = 0x1, // ptr is arena allocated - kAllocatedBit = 0x2, // ptr is heap allocated - kMutableBit = 0x4, // ptr contents are fully mutable - kMask = 0x7 // Bit mask + kMutableBit = 0x2, // ptr contents are fully mutable + kMask = 0x3 // Bit mask }; // Composed logical types @@ -112,7 +111,7 @@ class TaggedStringPtr { // Allocated strings are mutable and (as the name implies) owned. // A heap allocated string must be deleted. - kAllocated = kAllocatedBit | kMutableBit, + kAllocated = kMutableBit, // Mutable arena strings are strings where the string instance is owned // by the arena, but the string contents itself are owned by the string @@ -166,8 +165,16 @@ class TaggedStringPtr { // Returns true if the current string is an immutable default value. inline bool IsDefault() const { return (as_int() & kMask) == kDefault; } - // Returns true if the current string is a heap allocated mutable value. - inline bool IsAllocated() const { return as_int() & kAllocatedBit; } + // If the current string is a heap-allocated mutable value, returns a pointer + // to it. Returns nullptr otherwise. + inline std::string *GetIfAllocated() const { + auto allocated = as_int() ^ kAllocated; + if (allocated & kMask) return nullptr; + + auto ptr = reinterpret_cast(allocated); + PROTOBUF_ASSUME(ptr != nullptr); + return ptr; + } // Returns true if the current string is an arena allocated value. // This means it's either a mutable or fixed size arena string. @@ -224,8 +231,8 @@ static_assert(std::is_trivial::value, // Because ArenaStringPtr is used in oneof unions, its constructor is a NOP and // the field is always manually initialized via method calls. // -// See TaggedPtr for more information about the types of string values being -// held, and the mutable and ownership invariants for each type. +// See TaggedStringPtr for more information about the types of string values +// being held, and the mutable and ownership invariants for each type. struct PROTOBUF_EXPORT ArenaStringPtr { ArenaStringPtr() = default; constexpr ArenaStringPtr(ExplicitlyConstructedArenaString* default_value,