diff --git a/src/google/protobuf/compiler/cpp/unittest.inc b/src/google/protobuf/compiler/cpp/unittest.inc index a6552e1e1dc4..a417bbfb09d9 100644 --- a/src/google/protobuf/compiler/cpp/unittest.inc +++ b/src/google/protobuf/compiler/cpp/unittest.inc @@ -27,8 +27,9 @@ #include #include -#include "google/protobuf/compiler/cpp/unittest.h" +#include "absl/base/attributes.h" #include "absl/strings/cord.h" +#include "google/protobuf/compiler/cpp/unittest.h" #include "absl/strings/string_view.h" #ifndef _MSC_VER // We exclude this large proto because it's too large for @@ -482,25 +483,11 @@ TEST(GENERATED_MESSAGE_TEST_NAME, ADLSwap) { UNITTEST::TestAllTypes message1, message2; TestUtil::SetAllFields(&message1); - // Note the address of one of the repeated fields, to verify it was swapped - // rather than copied. - const int32_t* addr = &message1.repeated_int32().Get(0); -#ifdef PROTOBUF_FORCE_COPY_IN_SWAP - const int32_t value = *addr; -#endif - using std::swap; swap(message1, message2); TestUtil::ExpectAllFieldsSet(message2); TestUtil::ExpectClear(message1); - -#ifdef PROTOBUF_FORCE_COPY_IN_SWAP - EXPECT_NE(addr, &message2.repeated_int32().Get(0)); - EXPECT_EQ(value, message2.repeated_int32().Get(0)); -#else - EXPECT_EQ(addr, &message2.repeated_int32().Get(0)); -#endif } TEST(GENERATED_MESSAGE_TEST_NAME, CopyConstructor) { diff --git a/src/google/protobuf/extension_set_unittest.cc b/src/google/protobuf/extension_set_unittest.cc index 114f516d87b8..9c06ceb4dbc9 100644 --- a/src/google/protobuf/extension_set_unittest.cc +++ b/src/google/protobuf/extension_set_unittest.cc @@ -11,6 +11,7 @@ #include "google/protobuf/extension_set.h" +#include #include #include @@ -841,10 +842,12 @@ TEST(ExtensionSetTest, SpaceUsedExcludingSelf) { const size_t old_capacity = \ message->GetRepeatedExtension(unittest::repeated_##type##_extension) \ .Capacity(); \ - EXPECT_GE( \ - old_capacity, \ - (RepeatedFieldLowerClampLimit())); \ + if (sizeof(cpptype) > 1) { \ + EXPECT_GE( \ + old_capacity, \ + (RepeatedFieldLowerClampLimit())); \ + } \ for (int i = 0; i < 16; ++i) { \ message->AddExtension(unittest::repeated_##type##_extension, value); \ } \ diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index 5ff41a4c15e6..17e1baf5a62b 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -23,6 +23,8 @@ #include #include +#include +#include #include #include #include @@ -113,6 +115,120 @@ struct HeapRep { }; }; +// We use small object optimization (SOO) to store elements inline when possible +// for small repeated fields. We do so in order to avoid memory indirections. + +// SOO data is stored in the same space as the size/capacity ints. +enum { kSooCapacityBytes = 2 * sizeof(int) }; + +// Arena/elements pointers are aligned to at least kSooPtrAlignment bytes so we +// can use the lower bits to encode whether we're in SOO mode and if so, the +// SOO size. NOTE: we also tried using all kSooPtrMask bits to encode SOO size +// and use all ones as a sentinel value for non-SOO mode, but that was slower in +// benchmarks/loadtests. +enum { kSooPtrAlignment = sizeof(void*) }; +// The mask for the size bits in SOO mode, and also a sentinel value indicating +// that the field is not in SOO mode. +enum { kSooPtrMask = ~(kSooPtrAlignment - 1) }; +// This bit is 0 when in SOO mode and 1 when in non-SOO mode. +enum { kNotSooBit = kSooPtrAlignment >> 1 }; +// These bits are used to encode the size when in SOO mode (sizes are 0-3). +enum { kSooSizeMask = kNotSooBit - 1 }; + +// The number of elements that can be stored in the SOO rep. When +// kSooPtrAlignment == 8, this is 1 for int64_t, 2 for int32_t, 3 for bool, and +// 0 for absl::Cord. +constexpr int SooCapacityElements(size_t element_size) { + return std::min(kSooCapacityBytes / element_size, kSooSizeMask); +} + +struct LongSooRep { + // Returns char* rather than void* so callers can do pointer arithmetic. + char* elements() const { + auto ret = reinterpret_cast(elements_int & kSooPtrMask); + ABSL_DCHECK_NE(ret, nullptr); + return ret; + } + + uintptr_t elements_int; + int size; + int capacity; +}; +struct ShortSooRep { + constexpr ShortSooRep() = default; + explicit ShortSooRep(Arena* arena) + : arena_and_size(reinterpret_cast(arena)) { + ABSL_DCHECK_EQ(size(), 0); + } + + int size() const { return arena_and_size & kSooSizeMask; } + bool is_soo() const { return (arena_and_size & kNotSooBit) == 0; } + + uintptr_t arena_and_size = 0; + union { + char data[kSooCapacityBytes]; + // NOTE: in some language versions, we can't have a constexpr constructor + // if we don't initialize all fields, but `data` doesn't need to be + // initialized so initialize an empty dummy variable instead. + std::true_type dummy = {}; + }; +}; +struct SooRep { + constexpr SooRep() : short_rep() {} + explicit SooRep(Arena* arena) : short_rep(arena) {} + + bool is_soo() const { + static_assert(sizeof(LongSooRep) == sizeof(ShortSooRep), ""); + static_assert(offsetof(SooRep, long_rep) == offsetof(SooRep, short_rep), + ""); + static_assert(offsetof(LongSooRep, elements_int) == + offsetof(ShortSooRep, arena_and_size), + ""); + return short_rep.is_soo(); + } + Arena* soo_arena() const { + ABSL_DCHECK(is_soo()); + return reinterpret_cast(short_rep.arena_and_size & kSooPtrMask); + } + int size(bool is_soo) const { + ABSL_DCHECK_EQ(is_soo, this->is_soo()); +#if !defined(__clang__) && defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif + return is_soo ? short_rep.size() : long_rep.size; +#if !defined(__clang__) && defined(__GNUC__) +#pragma GCC diagnostic pop +#endif + } + void set_size(bool is_soo, int size) { + ABSL_DCHECK_EQ(is_soo, this->is_soo()); + if (is_soo) { + ABSL_DCHECK_LE(size, kSooSizeMask); + short_rep.arena_and_size &= kSooPtrMask; + short_rep.arena_and_size |= size; + } else { + long_rep.size = size; + } + } + // Initializes the SooRep in non-SOO mode with the given capacity and heap + // allocation. + void set_non_soo(bool was_soo, int capacity, void* elements) { + ABSL_DCHECK_EQ(was_soo, is_soo()); + ABSL_DCHECK_NE(elements, nullptr); + ABSL_DCHECK_EQ(reinterpret_cast(elements) % kSooPtrAlignment, + uintptr_t{0}); + if (was_soo) long_rep.size = short_rep.size(); + long_rep.capacity = capacity; + long_rep.elements_int = reinterpret_cast(elements) | kNotSooBit; + } + + union { + LongSooRep long_rep; + ShortSooRep short_rep; + }; +}; + } // namespace internal // RepeatedField is used to represent repeated fields of a primitive type (in @@ -318,10 +434,7 @@ class RepeatedField final // Gets the Arena on which this RepeatedField stores its elements. // Note: this can be inaccurate for split default fields so we make this // function non-const. - inline Arena* GetArena() { - return Capacity() == 0 ? static_cast(arena_or_elements_) - : heap_rep()->arena; - } + inline Arena* GetArena() { return GetArena(is_soo()); } // For internal use only. // @@ -339,15 +452,35 @@ class RepeatedField final friend class Arena; + static constexpr int kSooCapacityElements = + internal::SooCapacityElements(sizeof(Element)); + static constexpr int kInitialSize = 0; static PROTOBUF_CONSTEXPR const size_t kHeapRepHeaderSize = sizeof(HeapRep); RepeatedField(Arena* arena, const RepeatedField& rhs); RepeatedField(Arena* arena, RepeatedField&& rhs); + inline Arena* GetArena(bool is_soo) const { + return is_soo ? soo_rep_.soo_arena() : heap_rep()->arena; + } - void set_size(int s) { size_ = s; } - void set_capacity(int c) { capacity_ = c; } + bool is_soo() const { return soo_rep_.is_soo(); } + int size(bool is_soo) const { return soo_rep_.size(is_soo); } + int Capacity(bool is_soo) const { +#if !defined(__clang__) && defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wmaybe-uninitialized" +#endif + return is_soo ? kSooCapacityElements : soo_rep_.long_rep.capacity; +#if !defined(__clang__) && defined(__GNUC__) +#pragma GCC diagnostic pop +#endif + } + void set_size(bool is_soo, int size) { + ABSL_DCHECK_LE(size, Capacity(is_soo)); + soo_rep_.set_size(is_soo, size); + } // Swaps entire contents with "other". Should be called only if the caller can // guarantee that both repeated fields are on the same arena or are on the @@ -392,63 +525,83 @@ class RepeatedField final // the old container from `old_size` to `Capacity()` (unpoison memory) // directly before it is being released, and annotate the new container from // `Capacity()` to `old_size` (poison unused memory). - void Grow(int old_size, int new_size); - void GrowNoAnnotate(int old_size, int new_size); + void Grow(bool was_soo, int old_size, int new_size); + void GrowNoAnnotate(bool was_soo, int old_size, int new_size); // Annotates a change in size of this instance. This function should be called - // with (capacity, old_size) after new memory has been allocated and - // filled from previous memory), and called with (old_size, capacity) - // right before (previously annotated) memory is released. + // with (capacity, old_size) after new memory has been allocated and filled + // from previous memory, and UnpoisonBuffer() should be called right before + // (previously annotated) memory is released. void AnnotateSize(int old_size, int new_size) const { if (old_size != new_size) { - ABSL_ANNOTATE_CONTIGUOUS_CONTAINER( - unsafe_elements(), unsafe_elements() + Capacity(), - unsafe_elements() + old_size, unsafe_elements() + new_size); + ABSL_ATTRIBUTE_UNUSED const bool is_soo = this->is_soo(); + ABSL_ATTRIBUTE_UNUSED const Element* elem = unsafe_elements(is_soo); + ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(elem, elem + Capacity(is_soo), + elem + old_size, elem + new_size); if (new_size < old_size) { ABSL_ANNOTATE_MEMORY_IS_UNINITIALIZED( - unsafe_elements() + new_size, - (old_size - new_size) * sizeof(Element)); + elem + new_size, (old_size - new_size) * sizeof(Element)); } } } - // Replaces size with new_size and returns the previous value of size. This - // function is intended to be the only place where size is modified, with the - // exception of `AddInputIterator()` where the size of added items is not - // known in advance. - inline int ExchangeCurrentSize(int new_size) { - const int prev_size = size(); + // Unpoisons the memory buffer. + void UnpoisonBuffer() const { + AnnotateSize(size(), Capacity()); + if (is_soo()) { + // We need to manually unpoison the SOO buffer because in reflection for + // split repeated fields, we poison the whole SOO buffer even when we + // don't actually use the whole SOO buffer (e.g. for RepeatedField). + PROTOBUF_UNPOISON_MEMORY_REGION(soo_rep_.short_rep.data, + sizeof(soo_rep_.short_rep.data)); + } + } + + // Replaces size with new_size and returns the previous value of + // size. This function is intended to be the only place where + // size is modified, with the exception of `AddInputIterator()` + // where the size of added items is not known in advance. + inline int ExchangeCurrentSize(bool is_soo, int new_size) { + const int prev_size = size(is_soo); AnnotateSize(prev_size, new_size); - set_size(new_size); + set_size(is_soo, new_size); return prev_size; } // Returns a pointer to elements array. // pre-condition: Capacity() > 0. - Element* elements() const { - ABSL_DCHECK_GT(Capacity(), 0); - // Because of above pre-condition this cast is safe. - return unsafe_elements(); + Element* elements(bool is_soo) { + ABSL_DCHECK_GT(Capacity(is_soo), 0); + return unsafe_elements(is_soo); + } + const Element* elements(bool is_soo) const { + return const_cast(this)->elements(is_soo); } - // Returns a pointer to elements array if it exists; otherwise either null or - // an invalid pointer is returned. This only happens for empty repeated - // fields, where you can't dereference this pointer anyway (it's empty). - Element* unsafe_elements() const { - return static_cast(arena_or_elements_); + // Returns a pointer to elements array if it exists; otherwise an invalid + // pointer is returned. This only happens for empty repeated fields, where you + // can't dereference this pointer anyway (it's empty). + Element* unsafe_elements(bool is_soo) { + return is_soo ? reinterpret_cast(soo_rep_.short_rep.data) + : reinterpret_cast(soo_rep_.long_rep.elements()); + } + const Element* unsafe_elements(bool is_soo) const { + return const_cast(this)->unsafe_elements(is_soo); } // Returns a pointer to the HeapRep struct. - // pre-condition: the HeapRep must have been allocated, ie elements() is safe. + // pre-condition: the HeapRep must have been allocated, ie !is_soo(). HeapRep* heap_rep() const { - return reinterpret_cast(reinterpret_cast(elements()) - + ABSL_DCHECK(!is_soo()); + return reinterpret_cast(soo_rep_.long_rep.elements() - kHeapRepHeaderSize); } // Internal helper to delete all elements and deallocate the storage. template void InternalDeallocate() { - const size_t bytes = Capacity() * sizeof(Element) + kHeapRepHeaderSize; + ABSL_DCHECK(!is_soo()); + const size_t bytes = Capacity(false) * sizeof(Element) + kHeapRepHeaderSize; if (heap_rep()->arena == nullptr) { internal::SizedDelete(heap_rep(), bytes); } else if (!in_destructor) { @@ -468,62 +621,70 @@ class RepeatedField final // empty (common case), and add only an 8-byte header to the elements array // when non-empty. We make sure to place the size fields directly in the // RepeatedField class to avoid costly cache misses due to the indirection. - int size_; - int capacity_; - // If capacity_ == 0 this points to an Arena otherwise it points to the - // elements member of a HeapRep struct. Using this invariant allows the - // storage of the arena pointer without an extra allocation in the - // constructor. - void* arena_or_elements_; + internal::SooRep soo_rep_{}; }; // implementation ==================================================== template -constexpr RepeatedField::RepeatedField() - : size_(0), capacity_(0), arena_or_elements_(nullptr) { +constexpr RepeatedField::RepeatedField() { StaticValidityCheck(); +#ifdef __cpp_lib_is_constant_evaluated + if (!std::is_constant_evaluated()) { + AnnotateSize(kSooCapacityElements, 0); + } +#endif // __cpp_lib_is_constant_evaluated } template -inline RepeatedField::RepeatedField(Arena* arena) - : size_(0), capacity_(0), arena_or_elements_(arena) { +inline RepeatedField::RepeatedField(Arena* arena) : soo_rep_(arena) { StaticValidityCheck(); + AnnotateSize(kSooCapacityElements, 0); } template inline RepeatedField::RepeatedField(Arena* arena, const RepeatedField& rhs) - : size_(0), capacity_(0), arena_or_elements_(arena) { + : soo_rep_(arena) { StaticValidityCheck(); - if (auto size = rhs.size()) { - Grow(0, size); - ExchangeCurrentSize(size); - UninitializedCopyN(rhs.elements(), size, unsafe_elements()); + AnnotateSize(kSooCapacityElements, 0); + const bool rhs_is_soo = rhs.is_soo(); + if (auto size = rhs.size(rhs_is_soo)) { + bool is_soo = true; + if (size > kSooCapacityElements) { + Grow(is_soo, 0, size); + is_soo = false; + } + ExchangeCurrentSize(is_soo, size); + UninitializedCopyN(rhs.elements(rhs_is_soo), size, unsafe_elements(is_soo)); } } template template -RepeatedField::RepeatedField(Iter begin, Iter end) - : size_(0), capacity_(0), arena_or_elements_(nullptr) { +RepeatedField::RepeatedField(Iter begin, Iter end) { StaticValidityCheck(); + AnnotateSize(kSooCapacityElements, 0); Add(begin, end); } template RepeatedField::~RepeatedField() { StaticValidityCheck(); + const bool is_soo = this->is_soo(); #ifndef NDEBUG // Try to trigger segfault / asan failure in non-opt builds if arena_ // lifetime has ended before the destructor. - auto arena = GetArena(); + auto arena = GetArena(is_soo); if (arena) (void)arena->SpaceAllocated(); #endif - if (Capacity() > 0) { - Destroy(unsafe_elements(), unsafe_elements() + size()); - InternalDeallocate(); + const int size = this->size(is_soo); + if (size > 0) { + Element* elem = unsafe_elements(is_soo); + Destroy(elem, elem + size); } + UnpoisonBuffer(); + if (!is_soo) InternalDeallocate(); } template @@ -555,9 +716,10 @@ inline RepeatedField& RepeatedField::operator=( // We don't just call Swap(&other) here because it would perform 3 copies if // the two fields are on different arenas. if (this != &other) { - if (GetArena() != other.GetArena() + const Arena* arena = GetArena(); + if (arena != other.GetArena() #ifdef PROTOBUF_FORCE_COPY_IN_MOVE - || GetArena() == nullptr + || arena == nullptr #endif // !PROTOBUF_FORCE_COPY_IN_MOVE ) { CopyFrom(other); @@ -575,36 +737,44 @@ inline bool RepeatedField::empty() const { template inline int RepeatedField::size() const { - return size_; + return size(is_soo()); } template inline int RepeatedField::Capacity() const { - return capacity_; + return Capacity(is_soo()); } template inline void RepeatedField::AddAlreadyReserved(Element value) { - ABSL_DCHECK_LT(size(), Capacity()); - void* p = elements() + ExchangeCurrentSize(size() + 1); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); + ABSL_DCHECK_LT(old_size, Capacity(is_soo)); + void* p = elements(is_soo) + ExchangeCurrentSize(is_soo, old_size + 1); ::new (p) Element(std::move(value)); } template inline Element* RepeatedField::AddAlreadyReserved() ABSL_ATTRIBUTE_LIFETIME_BOUND { - ABSL_DCHECK_LT(size(), Capacity()); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); + ABSL_DCHECK_LT(old_size, Capacity(is_soo)); // new (p) compiles into nothing: this is intentional as this // function is documented to return uninitialized data for trivial types. - void* p = elements() + ExchangeCurrentSize(size() + 1); + void* p = elements(is_soo) + ExchangeCurrentSize(is_soo, old_size + 1); return ::new (p) Element; } template inline Element* RepeatedField::AddNAlreadyReserved(int n) ABSL_ATTRIBUTE_LIFETIME_BOUND { - ABSL_DCHECK_GE(Capacity() - size(), n) << Capacity() << ", " << size(); - Element* p = unsafe_elements() + ExchangeCurrentSize(size() + n); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); + ABSL_ATTRIBUTE_UNUSED const int capacity = Capacity(is_soo); + ABSL_DCHECK_GE(capacity - old_size, n) << capacity << ", " << old_size; + Element* p = + unsafe_elements(is_soo) + ExchangeCurrentSize(is_soo, old_size + n); for (Element *begin = p, *end = p + n; begin != end; ++begin) { new (static_cast(begin)) Element; } @@ -614,15 +784,20 @@ inline Element* RepeatedField::AddNAlreadyReserved(int n) template inline void RepeatedField::Resize(int new_size, const Element& value) { ABSL_DCHECK_GE(new_size, 0); - const int old_size = size(); + bool is_soo = this->is_soo(); + const int old_size = size(is_soo); if (new_size > old_size) { - if (new_size > Capacity()) Grow(old_size, new_size); - Element* first = elements() + ExchangeCurrentSize(new_size); - std::uninitialized_fill(first, elements() + new_size, value); + if (new_size > Capacity(is_soo)) { + Grow(is_soo, old_size, new_size); + is_soo = false; + } + Element* elem = elements(is_soo); + Element* first = elem + ExchangeCurrentSize(is_soo, new_size); + std::uninitialized_fill(first, elem + new_size, value); } else if (new_size < old_size) { - Element* elem = unsafe_elements(); + Element* elem = unsafe_elements(is_soo); Destroy(elem + new_size, elem + old_size); - ExchangeCurrentSize(new_size); + ExchangeCurrentSize(is_soo, new_size); } } @@ -631,7 +806,7 @@ inline const Element& RepeatedField::Get(int index) const ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_DCHECK_GE(index, 0); ABSL_DCHECK_LT(index, size()); - return elements()[index]; + return elements(is_soo())[index]; } template @@ -639,7 +814,7 @@ inline const Element& RepeatedField::at(int index) const ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_CHECK_GE(index, 0); ABSL_CHECK_LT(index, size()); - return elements()[index]; + return elements(is_soo())[index]; } template @@ -647,7 +822,7 @@ inline Element& RepeatedField::at(int index) ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_CHECK_GE(index, 0); ABSL_CHECK_LT(index, size()); - return elements()[index]; + return elements(is_soo())[index]; } template @@ -655,7 +830,7 @@ inline Element* RepeatedField::Mutable(int index) ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_DCHECK_GE(index, 0); ABSL_DCHECK_LT(index, size()); - return &elements()[index]; + return &elements(is_soo())[index]; } template @@ -665,69 +840,92 @@ inline void RepeatedField::Set(int index, const Element& value) { template inline void RepeatedField::Add(Element value) { - const int old_size = size(); - int capacity = Capacity(); - Element* elem = unsafe_elements(); + bool is_soo = this->is_soo(); + const int old_size = size(is_soo); + int capacity = Capacity(is_soo); + Element* elem = unsafe_elements(is_soo); if (ABSL_PREDICT_FALSE(old_size == capacity)) { - Grow(old_size, old_size + 1); - capacity = Capacity(); - elem = unsafe_elements(); + Grow(is_soo, old_size, old_size + 1); + is_soo = false; + capacity = Capacity(is_soo); + elem = unsafe_elements(is_soo); } int new_size = old_size + 1; - void* p = elem + ExchangeCurrentSize(new_size); + void* p = elem + ExchangeCurrentSize(is_soo, new_size); ::new (p) Element(std::move(value)); // The below helps the compiler optimize dense loops. - ABSL_ASSUME(new_size == size_); - ABSL_ASSUME(elem == arena_or_elements_); - ABSL_ASSUME(capacity == capacity_); + // Note: we can't call functions in PROTOBUF_ASSUME so use local variables. + ABSL_ATTRIBUTE_UNUSED const int final_is_soo = this->is_soo(); + PROTOBUF_ASSUME(is_soo == final_is_soo); + ABSL_ATTRIBUTE_UNUSED const int final_size = size(is_soo); + PROTOBUF_ASSUME(new_size == final_size); + ABSL_ATTRIBUTE_UNUSED Element* const final_elements = unsafe_elements(is_soo); + PROTOBUF_ASSUME(elem == final_elements); + ABSL_ATTRIBUTE_UNUSED const int final_capacity = Capacity(is_soo); + PROTOBUF_ASSUME(capacity == final_capacity); } template inline Element* RepeatedField::Add() ABSL_ATTRIBUTE_LIFETIME_BOUND { - const int old_size = size(); + bool is_soo = this->is_soo(); + const int old_size = size(is_soo); if (ABSL_PREDICT_FALSE(old_size == Capacity())) { - Grow(old_size, old_size + 1); + Grow(is_soo, old_size, old_size + 1); + is_soo = false; } - void* p = unsafe_elements() + ExchangeCurrentSize(old_size + 1); + void* p = unsafe_elements(is_soo) + ExchangeCurrentSize(is_soo, old_size + 1); return ::new (p) Element; } template template inline void RepeatedField::AddForwardIterator(Iter begin, Iter end) { - const int old_size = size(); - int capacity = Capacity(); - Element* elem = unsafe_elements(); + bool is_soo = this->is_soo(); + const int old_size = size(is_soo); + int capacity = Capacity(is_soo); + Element* elem = unsafe_elements(is_soo); int new_size = old_size + static_cast(std::distance(begin, end)); if (ABSL_PREDICT_FALSE(new_size > capacity)) { - Grow(old_size, new_size); - elem = unsafe_elements(); - capacity = Capacity(); + Grow(is_soo, old_size, new_size); + is_soo = false; + elem = unsafe_elements(is_soo); + capacity = Capacity(is_soo); } - UninitializedCopy(begin, end, elem + ExchangeCurrentSize(new_size)); + UninitializedCopy(begin, end, elem + ExchangeCurrentSize(is_soo, new_size)); // The below helps the compiler optimize dense loops. - ABSL_ASSUME(new_size == size_); - ABSL_ASSUME(elem == arena_or_elements_); - ABSL_ASSUME(capacity == capacity_); + // Note: we can't call functions in PROTOBUF_ASSUME so use local variables. + ABSL_ATTRIBUTE_UNUSED const int final_is_soo = this->is_soo(); + PROTOBUF_ASSUME(is_soo == final_is_soo); + ABSL_ATTRIBUTE_UNUSED const int final_size = size(is_soo); + PROTOBUF_ASSUME(new_size == final_size); + ABSL_ATTRIBUTE_UNUSED Element* const final_elements = unsafe_elements(is_soo); + PROTOBUF_ASSUME(elem == final_elements); + ABSL_ATTRIBUTE_UNUSED const int final_capacity = Capacity(is_soo); + PROTOBUF_ASSUME(capacity == final_capacity); } template template inline void RepeatedField::AddInputIterator(Iter begin, Iter end) { - Element* elem = unsafe_elements(); - Element* first = elem + size(); - Element* last = elem + Capacity(); - AnnotateSize(size(), Capacity()); + bool is_soo = this->is_soo(); + int size = this->size(is_soo); + int capacity = Capacity(is_soo); + Element* elem = unsafe_elements(is_soo); + Element* first = elem + size; + Element* last = elem + capacity; + UnpoisonBuffer(); while (begin != end) { if (ABSL_PREDICT_FALSE(first == last)) { - int size = first - elem; - GrowNoAnnotate(size, size + 1); - elem = unsafe_elements(); + size = first - elem; + GrowNoAnnotate(is_soo, size, size + 1); + is_soo = false; + elem = unsafe_elements(is_soo); + capacity = Capacity(is_soo); first = elem + size; - last = elem + Capacity(); + last = elem + capacity; } ::new (static_cast(first)) Element(*begin); ++begin; @@ -735,8 +933,8 @@ inline void RepeatedField::AddInputIterator(Iter begin, Iter end) { } const int new_size = first - elem; - set_size(new_size); - AnnotateSize(Capacity(), new_size); + set_size(is_soo, new_size); + AnnotateSize(capacity, new_size); } template @@ -753,10 +951,11 @@ inline void RepeatedField::Add(Iter begin, Iter end) { template inline void RepeatedField::RemoveLast() { - const int old_size = size(); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); ABSL_DCHECK_GT(old_size, 0); - elements()[old_size - 1].~Element(); - ExchangeCurrentSize(old_size - 1); + elements(is_soo)[old_size - 1].~Element(); + ExchangeCurrentSize(is_soo, old_size - 1); } template @@ -764,9 +963,10 @@ void RepeatedField::ExtractSubrange(int start, int num, Element* elements) { ABSL_DCHECK_GE(start, 0); ABSL_DCHECK_GE(num, 0); - const int old_size = size(); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); ABSL_DCHECK_LE(start + num, old_size); - Element* elem = unsafe_elements(); + Element* elem = unsafe_elements(is_soo); // Save the values of the removed elements if requested. if (elements != nullptr) { @@ -783,19 +983,23 @@ void RepeatedField::ExtractSubrange(int start, int num, template inline void RepeatedField::Clear() { - Element* elem = unsafe_elements(); - Destroy(elem, elem + size()); - ExchangeCurrentSize(0); + const bool is_soo = this->is_soo(); + Element* elem = unsafe_elements(is_soo); + Destroy(elem, elem + size(is_soo)); + ExchangeCurrentSize(is_soo, 0); } template inline void RepeatedField::MergeFrom(const RepeatedField& other) { ABSL_DCHECK_NE(&other, this); - if (auto other_size = other.size()) { + const bool other_is_soo = other.is_soo(); + if (auto other_size = other.size(other_is_soo)) { const int old_size = size(); Reserve(old_size + other_size); - Element* dst = elements() + ExchangeCurrentSize(old_size + other_size); - UninitializedCopyN(other.elements(), other_size, dst); + const bool is_soo = this->is_soo(); + Element* dst = + elements(is_soo) + ExchangeCurrentSize(is_soo, old_size + other_size); + UninitializedCopyN(other.elements(other_is_soo), other_size, dst); } } @@ -832,13 +1036,13 @@ inline typename RepeatedField::iterator RepeatedField::erase( template inline Element* RepeatedField::mutable_data() ABSL_ATTRIBUTE_LIFETIME_BOUND { - return unsafe_elements(); + return unsafe_elements(is_soo()); } template inline const Element* RepeatedField::data() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return unsafe_elements(); + return unsafe_elements(is_soo()); } template @@ -846,27 +1050,31 @@ inline void RepeatedField::InternalSwap( RepeatedField* PROTOBUF_RESTRICT other) { ABSL_DCHECK(this != other); - // Swap all fields at once. - static_assert(std::is_standard_layout>::value, - "offsetof() requires standard layout before c++17"); - static constexpr size_t kOffset = offsetof(RepeatedField, size_); - internal::memswaparena_or_elements_) - kOffset>( - reinterpret_cast(this) + kOffset, - reinterpret_cast(other) + kOffset); + // We need to unpoison during the swap in case we're in SOO mode. + UnpoisonBuffer(); + other->UnpoisonBuffer(); + + internal::memswap( + reinterpret_cast(&this->soo_rep_), + reinterpret_cast(&other->soo_rep_)); + + AnnotateSize(Capacity(), size()); + other->AnnotateSize(other->Capacity(), other->size()); } template void RepeatedField::Swap(RepeatedField* other) { if (this == other) return; + Arena* arena = GetArena(); + Arena* other_arena = other->GetArena(); #ifdef PROTOBUF_FORCE_COPY_IN_SWAP - if (GetArena() != nullptr && GetArena() == other->GetArena()) { + if (arena != nullptr && arena == other_arena) { #else // PROTOBUF_FORCE_COPY_IN_SWAP - if (GetArena() == other->GetArena()) { + if (arena == other_arena) { #endif // !PROTOBUF_FORCE_COPY_IN_SWAP InternalSwap(other); } else { - RepeatedField temp(other->GetArena()); + RepeatedField temp(other_arena); temp.MergeFrom(*this); CopyFrom(*other); other->UnsafeArenaSwap(&temp); @@ -882,45 +1090,51 @@ void RepeatedField::UnsafeArenaSwap(RepeatedField* other) { template void RepeatedField::SwapElements(int index1, int index2) { + Element* elem = elements(is_soo()); using std::swap; // enable ADL with fallback - swap(elements()[index1], elements()[index2]); + swap(elem[index1], elem[index2]); } template inline typename RepeatedField::iterator RepeatedField::begin() ABSL_ATTRIBUTE_LIFETIME_BOUND { - return iterator(unsafe_elements()); + return iterator(unsafe_elements(is_soo())); } template inline typename RepeatedField::const_iterator RepeatedField::begin() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return const_iterator(unsafe_elements()); + return const_iterator(unsafe_elements(is_soo())); } template inline typename RepeatedField::const_iterator RepeatedField::cbegin() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return const_iterator(unsafe_elements()); + return const_iterator(unsafe_elements(is_soo())); } template inline typename RepeatedField::iterator RepeatedField::end() ABSL_ATTRIBUTE_LIFETIME_BOUND { - return iterator(unsafe_elements() + size()); + const bool is_soo = this->is_soo(); + return iterator(unsafe_elements(is_soo) + size(is_soo)); } template inline typename RepeatedField::const_iterator RepeatedField::end() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return const_iterator(unsafe_elements() + size()); + const bool is_soo = this->is_soo(); + return const_iterator(unsafe_elements(is_soo) + size(is_soo)); } template inline typename RepeatedField::const_iterator RepeatedField::cend() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return const_iterator(unsafe_elements() + size()); + const bool is_soo = this->is_soo(); + return const_iterator(unsafe_elements(is_soo) + size(is_soo)); } template inline size_t RepeatedField::SpaceUsedExcludingSelfLong() const { const int capacity = Capacity(); - return capacity > 0 ? capacity * sizeof(Element) + kHeapRepHeaderSize : 0; + return capacity > kSooCapacityElements + ? capacity * sizeof(Element) + kHeapRepHeaderSize + : 0; } namespace internal { @@ -928,10 +1142,7 @@ namespace internal { // requested 'new_size'. The result is clamped to the closed interval: // [internal::kMinRepeatedFieldAllocationSize, // std::numeric_limits::max()] -// Requires: -// new_size > capacity && -// (capacity == 0 || -// capacity >= kRepeatedFieldLowerClampLimit) +// Requires: new_size > capacity template inline int CalculateReserveSize(int capacity, int new_size) { constexpr int lower_limit = @@ -945,6 +1156,15 @@ inline int CalculateReserveSize(int capacity, int new_size) { if (PROTOBUF_PREDICT_FALSE(capacity > kMaxSizeBeforeClamp)) { return std::numeric_limits::max(); } + constexpr int kSooCapacityElements = SooCapacityElements(sizeof(T)); + if (kSooCapacityElements > 0 && kSooCapacityElements < lower_limit) { + // In this case, we need to set capacity to 0 here to ensure power-of-two + // sized allocations. + if (capacity < lower_limit) capacity = 0; + } else { + ABSL_DCHECK(capacity == 0 || capacity >= lower_limit) + << capacity << " " << lower_limit; + } // We want to double the number of bytes, not the number of elements, to try // to stay within power-of-two allocations. // The allocation has kHeapRepHeaderSize + sizeof(T) * capacity. @@ -955,22 +1175,25 @@ inline int CalculateReserveSize(int capacity, int new_size) { template void RepeatedField::Reserve(int new_size) { - if (ABSL_PREDICT_FALSE(new_size > Capacity())) { - Grow(size(), new_size); + const bool was_soo = is_soo(); + if (ABSL_PREDICT_FALSE(new_size > Capacity(was_soo))) { + Grow(was_soo, size(was_soo), new_size); } } // Avoid inlining of Reserve(): new, copy, and delete[] lead to a significant // amount of code bloat. template -PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(int old_size, +PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(bool was_soo, + int old_size, int new_size) { - ABSL_DCHECK_GT(new_size, Capacity()); + const int old_capacity = Capacity(was_soo); + ABSL_DCHECK_GT(new_size, old_capacity); HeapRep* new_rep; Arena* arena = GetArena(); new_size = internal::CalculateReserveSize( - Capacity(), new_size); + old_capacity, new_size); ABSL_DCHECK_LE(static_cast(new_size), (std::numeric_limits::max() - kHeapRepHeaderSize) / @@ -994,25 +1217,22 @@ PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(int old_size, } new_rep->arena = arena; - if (Capacity() > 0) { - if (old_size > 0) { - Element* pnew = static_cast(new_rep->elements()); - Element* pold = elements(); - // TODO: add absl::is_trivially_relocatable - if (std::is_trivial::value) { - memcpy(static_cast(pnew), pold, old_size * sizeof(Element)); - } else { - for (Element* end = pnew + old_size; pnew != end; ++pnew, ++pold) { - ::new (static_cast(pnew)) Element(std::move(*pold)); - pold->~Element(); - } + if (old_size > 0) { + Element* pnew = static_cast(new_rep->elements()); + Element* pold = elements(was_soo); + // TODO: add absl::is_trivially_relocatable + if (std::is_trivial::value) { + memcpy(static_cast(pnew), pold, old_size * sizeof(Element)); + } else { + for (Element* end = pnew + old_size; pnew != end; ++pnew, ++pold) { + ::new (static_cast(pnew)) Element(std::move(*pold)); + pold->~Element(); } } - InternalDeallocate(); } + if (!was_soo) InternalDeallocate(); - set_capacity(new_size); - arena_or_elements_ = static_cast(new_rep->elements()); + soo_rep_.set_non_soo(was_soo, new_size, new_rep->elements()); } // Ideally we would be able to use: @@ -1021,21 +1241,22 @@ PROTOBUF_NOINLINE void RepeatedField::GrowNoAnnotate(int old_size, // However, as explained in b/266411038#comment9, this causes issues // in shared libraries for Youtube (and possibly elsewhere). template -PROTOBUF_NOINLINE void RepeatedField::Grow(int old_size, +PROTOBUF_NOINLINE void RepeatedField::Grow(bool was_soo, int old_size, int new_size) { - AnnotateSize(old_size, Capacity()); - GrowNoAnnotate(old_size, new_size); + UnpoisonBuffer(); + GrowNoAnnotate(was_soo, old_size, new_size); AnnotateSize(Capacity(), old_size); } template inline void RepeatedField::Truncate(int new_size) { - const int old_size = size(); + const bool is_soo = this->is_soo(); + const int old_size = size(is_soo); ABSL_DCHECK_LE(new_size, old_size); if (new_size < old_size) { - Element* elem = unsafe_elements(); + Element* elem = unsafe_elements(is_soo); Destroy(elem + new_size, elem + old_size); - ExchangeCurrentSize(new_size); + ExchangeCurrentSize(is_soo, new_size); } } diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index 50e43f594e14..574db69560eb 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -201,12 +201,10 @@ TEST(RepeatedField, Small) { EXPECT_TRUE(field.empty()); EXPECT_EQ(field.size(), 0); - // Additional bytes are for 'struct Rep' header. - int expected_usage = - (sizeof(Arena*) > sizeof(int) ? sizeof(Arena*) / sizeof(int) : 3) * - sizeof(int) + - sizeof(Arena*); - EXPECT_GE(field.SpaceUsedExcludingSelf(), expected_usage); + if (sizeof(void*) == 8) { + // Usage should be 0 because this should fit in SOO space. + EXPECT_EQ(field.SpaceUsedExcludingSelf(), 0); + } } @@ -258,8 +256,11 @@ void CheckAllocationSizes(bool is_ptr) { ASSERT_EQ((1 << log2), last_alloc); } - // The byte size must be a multiple of 8. - ASSERT_EQ(rep->Capacity() * sizeof(T) % 8, 0); + // The byte size must be a multiple of 8 when not SOO. + const int capacity_bytes = rep->Capacity() * sizeof(T); + if (capacity_bytes > internal::kSooCapacityBytes) { + ASSERT_EQ(capacity_bytes % 8, 0); + } } } } @@ -481,14 +482,6 @@ TEST(RepeatedField, Resize) { EXPECT_TRUE(field.empty()); } -TEST(RepeatedField, ReserveNothing) { - RepeatedField field; - EXPECT_EQ(0, field.Capacity()); - - field.Reserve(-1); - EXPECT_EQ(0, field.Capacity()); -} - TEST(RepeatedField, ReserveLowerClamp) { int clamped_value = internal::CalculateReserveSize(0, 1); EXPECT_GE(clamped_value, sizeof(void*) / sizeof(bool)); @@ -899,9 +892,7 @@ TEST(RepeatedField, MoveConstruct) { RepeatedField source; source.Add(1); source.Add(2); - const int* data = source.data(); RepeatedField destination = std::move(source); - EXPECT_EQ(data, destination.data()); EXPECT_THAT(destination, ElementsAre(1, 2)); // This property isn't guaranteed but it's useful to have a test that would // catch changes in this area. @@ -928,14 +919,8 @@ TEST(RepeatedField, MoveAssign) { source.Add(2); RepeatedField destination; destination.Add(3); - const int* source_data = source.data(); - const int* destination_data = destination.data(); destination = std::move(source); - EXPECT_EQ(source_data, destination.data()); EXPECT_THAT(destination, ElementsAre(1, 2)); - // This property isn't guaranteed but it's useful to have a test that would - // catch changes in this area. - EXPECT_EQ(destination_data, source.data()); EXPECT_THAT(source, ElementsAre(3)); } { @@ -945,9 +930,7 @@ TEST(RepeatedField, MoveAssign) { source->Add(2); RepeatedField* destination = Arena::Create>(&arena); destination->Add(3); - const int* source_data = source->data(); *destination = std::move(*source); - EXPECT_EQ(source_data, destination->data()); EXPECT_THAT(*destination, ElementsAre(1, 2)); EXPECT_THAT(*source, ElementsAre(3)); } @@ -999,9 +982,7 @@ TEST(RepeatedField, MoveAssign) { RepeatedField& alias = field; field.Add(1); field.Add(2); - const int* data = field.data(); field = std::move(alias); - EXPECT_EQ(data, field.data()); EXPECT_THAT(field, ElementsAre(1, 2)); } { @@ -1009,9 +990,7 @@ TEST(RepeatedField, MoveAssign) { RepeatedField* field = Arena::Create>(&arena); field->Add(1); field->Add(2); - const int* data = field->data(); *field = std::move(*field); - EXPECT_EQ(data, field->data()); EXPECT_THAT(*field, ElementsAre(1, 2)); } } @@ -1346,6 +1325,20 @@ TEST(RepeatedField, Cleanups) { EXPECT_THAT(growth.cleanups, testing::UnorderedElementsAre(ptr)); } +TEST(RepeatedField, InitialSooCapacity) { + if (sizeof(void*) == 8) { + EXPECT_EQ(RepeatedField().Capacity(), 3); + EXPECT_EQ(RepeatedField().Capacity(), 2); + EXPECT_EQ(RepeatedField().Capacity(), 1); + EXPECT_EQ(RepeatedField().Capacity(), 0); + } else { + EXPECT_EQ(RepeatedField().Capacity(), 1); + EXPECT_EQ(RepeatedField().Capacity(), 1); + EXPECT_EQ(RepeatedField().Capacity(), 1); + EXPECT_EQ(RepeatedField().Capacity(), 0); + } +} + // =================================================================== // RepeatedPtrField tests. These pretty much just mirror the RepeatedField // tests above.