diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 48a8115ad7..4902ce2d70 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -253,6 +253,7 @@ cc_library( "//src/google/protobuf:__subpackages__", ], deps = [ + ":arena_align", ":arena_allocation_policy", ":arena_cleanup", ":arena_config", diff --git a/src/google/protobuf/arena.h b/src/google/protobuf/arena.h index 29c5a106f5..b60b7d0fc9 100644 --- a/src/google/protobuf/arena.h +++ b/src/google/protobuf/arena.h @@ -49,6 +49,7 @@ using type_info = ::type_info; #endif #include +#include "google/protobuf/arena_align.h" #include "google/protobuf/arena_config.h" #include "google/protobuf/arena_impl.h" #include "google/protobuf/port.h" @@ -291,15 +292,16 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final { // Allocates memory with the specific size and alignment. void* AllocateAligned(size_t size, size_t align = 8) { - if (align <= 8) { - return Allocate(internal::AlignUpTo8(size)); + if (align <= internal::ArenaAlignDefault::align) { + return Allocate(internal::ArenaAlignDefault::Ceil(size)); } else { // We are wasting space by over allocating align - 8 bytes. Compared // to a dedicated function that takes current alignment in consideration. // Such a scheme would only waste (align - 8)/2 bytes on average, but // requires a dedicated function in the outline arena allocation // functions. Possibly re-evaluate tradeoffs later. - return internal::AlignTo(Allocate(size + align - 8), align); + auto align_as = internal::ArenaAlignAs(align); + return align_as.Ceil(Allocate(align_as.Padded(size))); } } @@ -662,15 +664,16 @@ class PROTOBUF_EXPORT PROTOBUF_ALIGNAS(8) Arena final { } void* AllocateAlignedForArray(size_t n, size_t align) { - if (align <= 8) { - return AllocateForArray(internal::AlignUpTo8(n)); + if (align <= internal::ArenaAlignDefault::align) { + return AllocateForArray(internal::ArenaAlignDefault::Ceil(n)); } else { // We are wasting space by over allocating align - 8 bytes. Compared // to a dedicated function that takes current alignment in consideration. // Such a scheme would only waste (align - 8)/2 bytes on average, but // requires a dedicated function in the outline arena allocation // functions. Possibly re-evaluate tradeoffs later. - return internal::AlignTo(AllocateForArray(n + align - 8), align); + auto align_as = internal::ArenaAlignAs(align); + return align_as.Ceil(AllocateForArray(align_as.Padded(n))); } } diff --git a/src/google/protobuf/arena_align.h b/src/google/protobuf/arena_align.h index 8881a93a71..573caf3a96 100644 --- a/src/google/protobuf/arena_align.h +++ b/src/google/protobuf/arena_align.h @@ -35,14 +35,21 @@ // // Ceil(size_t n) - rounds `n` up to the nearest `align` boundary. // Floor(size_t n) - rounds `n` down to the nearest `align` boundary. -// Ceil(T* P) - rounds `p` up to the nearest `align` boundary. +// Padded(size_t n) - returns the unaligned size to align 'n' bytes. (1) + +// Ceil(T* P) - rounds `p` up to the nearest `align` boundary. (2) // IsAligned(size_t n) - returns true if `n` is aligned to `align` // IsAligned(T* p) - returns true if `p` is aligned to `align` // CheckAligned(T* p) - returns `p`. Checks alignment of `p` in debug. // -// Additionally there is an optimized `CeilDefaultAligned(T*)` method which is -// equivalent to `Ceil(ArenaAlignDefault().CheckAlign(p))` but more efficiently -// implemented as a 'check only' for ArenaAlignDefault. +// 1) `Padded(n)` returns the minimum size needed to align an object of size 'n' +// into a memory area that is default aligned. For example, allocating 'n' +// bytes aligned at 32 bytes requires a size of 'n + 32 - 8' to align at 32 +// bytes for any 8 byte boundary. +// +// 2) There is an optimized `CeilDefaultAligned(T*)` method which is equivalent +// to `Ceil(ArenaAlignDefault::CheckAlign(p))` but more efficiently +// implemented as a 'check only' for ArenaAlignDefault. // // These classes allow for generic arena logic using 'alignment policies'. // @@ -50,10 +57,14 @@ // // template // void* NaiveAlloc(size_t n, Align align) { -// align.CheckAligned(n); -// uint8_t* ptr = align.CeilDefaultAligned(ptr_); -// ptr_ += n; -// return ptr; +// ABSL_ASSERT(align.IsAligned(n)); +// const size_t required = align.Padded(n); +// if (required <= static_cast(ptr_ - limit_)) { +// uint8_t* ptr = align.CeilDefaultAligned(ptr_); +// ptr_ = ptr + n; +// return ptr; +// } +// return nullptr; // } // // void CallSites() { @@ -80,31 +91,41 @@ namespace internal { struct ArenaAlignDefault { PROTOBUF_EXPORT static constexpr size_t align = 8; // NOLINT - static constexpr bool IsAligned(size_t n) { return (n & (align - 1)) == 0; } + static constexpr bool IsAligned(size_t n) { return (n & (align - 1)) == 0U; } template - static bool IsAligned(T* ptr) { - return (reinterpret_cast(ptr) & (align - 1)) == 0; + static inline PROTOBUF_ALWAYS_INLINE bool IsAligned(T* ptr) { + return (reinterpret_cast(ptr) & (align - 1)) == 0U; + } + + static inline PROTOBUF_ALWAYS_INLINE constexpr size_t Ceil(size_t n) { + return (n + align - 1) & -align; + } + static inline PROTOBUF_ALWAYS_INLINE constexpr size_t Floor(size_t n) { + return (n & ~(align - 1)); } - static constexpr size_t Ceil(size_t n) { return (n + align - 1) & -align; } - static constexpr size_t Floor(size_t n) { return (n & ~(align - 1)); } + static inline PROTOBUF_ALWAYS_INLINE size_t Padded(size_t n) { + ABSL_ASSERT(IsAligned(n)); + return n; + } template - T* Ceil(T* ptr) const { + static inline PROTOBUF_ALWAYS_INLINE T* Ceil(T* ptr) { uintptr_t intptr = reinterpret_cast(ptr); return reinterpret_cast((intptr + align - 1) & -align); } template - T* CeilDefaultAligned(T* ptr) const { - return ArenaAlignDefault().CheckAligned(ptr); + static inline PROTOBUF_ALWAYS_INLINE T* CeilDefaultAligned(T* ptr) { + ABSL_ASSERT(IsAligned(ptr)); + return ptr; } // Address sanitizer enabled alignment check template - static T* CheckAligned(T* ptr) { - GOOGLE_ABSL_DCHECK(IsAligned(ptr)) << static_cast(ptr); + static inline PROTOBUF_ALWAYS_INLINE T* CheckAligned(T* ptr) { + ABSL_ASSERT(IsAligned(ptr)); return ptr; } }; @@ -114,16 +135,24 @@ struct ArenaAlign { size_t align; - constexpr bool IsAligned(size_t n) const { return (n & (align - 1)) == 0; } + constexpr bool IsAligned(size_t n) const { return (n & (align - 1)) == 0U; } template bool IsAligned(T* ptr) const { - return (reinterpret_cast(ptr) & (align - 1)) == 0; + return (reinterpret_cast(ptr) & (align - 1)) == 0U; } constexpr size_t Ceil(size_t n) const { return (n + align - 1) & -align; } constexpr size_t Floor(size_t n) const { return (n & ~(align - 1)); } + constexpr size_t Padded(size_t n) const { + // TODO(mvels): there are direct callers of AllocateAligned() that violate + // `size` being a multiple of `align`: that should be an error / assert. + // ABSL_ASSERT(IsAligned(n)); + ABSL_ASSERT(ArenaAlignDefault::IsAligned(align)); + return n + align - ArenaAlignDefault::align; + } + template T* Ceil(T* ptr) const { uintptr_t intptr = reinterpret_cast(ptr); @@ -132,24 +161,55 @@ struct ArenaAlign { template T* CeilDefaultAligned(T* ptr) const { - return Ceil(ArenaAlignDefault().CheckAligned(ptr)); + ABSL_ASSERT(ArenaAlignDefault::IsAligned(ptr)); + return Ceil(ptr); } // Address sanitizer enabled alignment check template T* CheckAligned(T* ptr) const { - GOOGLE_ABSL_DCHECK(IsAligned(ptr)) << static_cast(ptr); + ABSL_ASSERT(IsAligned(ptr)); return ptr; } }; inline ArenaAlign ArenaAlignAs(size_t align) { // align must be a non zero power of 2 >= 8 - GOOGLE_ABSL_DCHECK_NE(align, 0); + GOOGLE_ABSL_DCHECK_NE(align, 0U); GOOGLE_ABSL_DCHECK(absl::has_single_bit(align)) << "Invalid alignment " << align; return ArenaAlign{align}; } +template +struct AlignFactory { + static_assert(align > ArenaAlignDefault::align, "Not over-aligned"); + static_assert((align & (align - 1)) == 0U, "Not power of 2"); + static constexpr ArenaAlign Create() { return ArenaAlign{align}; } +}; + +template +struct AlignFactory { + static_assert(align <= ArenaAlignDefault::align, "Over-aligned"); + static_assert((align & (align - 1)) == 0U, "Not power of 2"); + static constexpr ArenaAlignDefault Create() { return ArenaAlignDefault{}; } +}; + +// Returns an `ArenaAlignDefault` instance for `align` less than or equal to the +// default alignment, and `AlignAs(align)` for over-aligned values of `align`. +// The purpose is to take advantage of invoking functions accepting a template +// overloaded 'Align align` argument reducing the alignment operations on +// `ArenaAlignDefault` implementations to no-ops. +template +inline constexpr auto ArenaAlignAs() { + return AlignFactory::Create(); +} + +// Returns ArenaAlignAs +template +inline constexpr auto ArenaAlignOf() { + return ArenaAlignAs(); +} + } // namespace internal } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/arena_align_test.cc b/src/google/protobuf/arena_align_test.cc index e17cb33209..f594e2236b 100644 --- a/src/google/protobuf/arena_align_test.cc +++ b/src/google/protobuf/arena_align_test.cc @@ -67,6 +67,16 @@ TEST(ArenaAlignDefault, Ceil) { EXPECT_THAT(align_default.Ceil(16), Eq(16)); } +TEST(ArenaAlignDefault, Padded) { + auto align_default = ArenaAlignDefault(); + EXPECT_THAT(align_default.Padded(0), Eq(0)); + EXPECT_THAT(align_default.Padded(8), Eq(8)); + EXPECT_THAT(align_default.Padded(64), Eq(64)); +#ifdef PROTOBUF_HAS_DEATH_TEST + EXPECT_DEBUG_DEATH(align_default.Padded(1), ".*"); +#endif // PROTOBUF_HAS_DEATH_TEST +} + TEST(ArenaAlignDefault, CeilPtr) { alignas(8) char p[17] = {0}; auto align_default = ArenaAlignDefault(); @@ -147,6 +157,18 @@ TEST(ArenaAlign, Ceil) { EXPECT_THAT(align_64.Ceil(128), Eq(128)); } +TEST(ArenaAlign, Padded) { + auto align_64 = ArenaAlignAs(64); + EXPECT_THAT(align_64.Padded(64), Eq(64 + 64 - ArenaAlignDefault::align)); + EXPECT_THAT(align_64.Padded(128), Eq(128 + 64 - ArenaAlignDefault::align)); +#ifdef PROTOBUF_HAS_DEATH_TEST + // TODO(mvels): there are direct callers of AllocateAligned() that violate + // `size` being a multiple of `align`: that should be an error / assert. + // EXPECT_DEBUG_DEATH(align_64.Padded(16), ".*"); + EXPECT_DEBUG_DEATH(ArenaAlignAs(2).Padded(8), ".*"); +#endif // PROTOBUF_HAS_DEATH_TEST +} + TEST(ArenaAlign, CeilPtr) { alignas(64) char p[129] = {0}; auto align_64 = ArenaAlignAs(64); diff --git a/src/google/protobuf/arena_impl.h b/src/google/protobuf/arena_impl.h index 043f21f67f..0d39c19c88 100644 --- a/src/google/protobuf/arena_impl.h +++ b/src/google/protobuf/arena_impl.h @@ -41,10 +41,11 @@ #include #include "google/protobuf/stubs/common.h" -#include "absl/base/attributes.h" #include "google/protobuf/stubs/logging.h" #include "absl/numeric/bits.h" +#include "absl/strings/cord.h" #include "absl/synchronization/mutex.h" +#include "google/protobuf/arena_align.h" #include "google/protobuf/arena_allocation_policy.h" #include "google/protobuf/arena_cleanup.h" #include "google/protobuf/arena_config.h" @@ -59,29 +60,6 @@ namespace google { namespace protobuf { namespace internal { -inline PROTOBUF_ALWAYS_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); -} - -inline PROTOBUF_ALWAYS_INLINE constexpr size_t AlignUpTo(size_t n, size_t a) { - // We are wasting space by over allocating align - 8 bytes. Compared to a - // dedicated function that takes current alignment in consideration. Such a - // scheme would only waste (align - 8)/2 bytes on average, but requires a - // dedicated function in the outline arena allocation functions. Possibly - // re-evaluate tradeoffs later. - return a <= 8 ? AlignUpTo8(n) : n + a - 8; -} - -inline PROTOBUF_ALWAYS_INLINE void* AlignTo(void* p, size_t a) { - if (a <= 8) { - return p; - } else { - auto u = reinterpret_cast(p); - return reinterpret_cast((u + a - 1) & (~a + 1)); - } -} - // Arena blocks are variable length malloc-ed objects. The following structure // describes the common header for all blocks. struct ArenaBlock { @@ -172,7 +150,7 @@ class PROTOBUF_EXPORT SerialArena { // from it. template void* AllocateAligned(size_t n) { - GOOGLE_ABSL_DCHECK_EQ(internal::AlignUpTo8(n), n); // Must be already aligned. + GOOGLE_ABSL_DCHECK(internal::ArenaAlignDefault::IsAligned(n)); GOOGLE_ABSL_DCHECK_GE(limit_, ptr()); if (alloc_client == AllocationClient::kArray) { @@ -188,6 +166,22 @@ class PROTOBUF_EXPORT SerialArena { } private: + static inline PROTOBUF_ALWAYS_INLINE constexpr size_t AlignUpTo(size_t n, + size_t a) { + // We are wasting space by over allocating align - 8 bytes. Compared to a + // dedicated function that takes current alignment in consideration. Such a + // scheme would only waste (align - 8)/2 bytes on average, but requires a + // dedicated function in the outline arena allocation functions. Possibly + // re-evaluate tradeoffs later. + return a <= 8 ? ArenaAlignDefault::Ceil(n) : ArenaAlignAs(a).Padded(n); + } + + static inline PROTOBUF_ALWAYS_INLINE void* AlignTo(void* p, size_t a) { + return (a <= ArenaAlignDefault::align) + ? ArenaAlignDefault::CeilDefaultAligned(p) + : ArenaAlignAs(a).CeilDefaultAligned(p); + } + void* AllocateFromExisting(size_t n) { PROTOBUF_UNPOISON_MEMORY_REGION(ptr(), n); void* ret = ptr(); @@ -248,7 +242,7 @@ class PROTOBUF_EXPORT SerialArena { public: // Allocate space if the current region provides enough space. bool MaybeAllocateAligned(size_t n, void** out) { - GOOGLE_ABSL_DCHECK_EQ(internal::AlignUpTo8(n), n); // Must be already aligned. + GOOGLE_ABSL_DCHECK(internal::ArenaAlignDefault::IsAligned(n)); GOOGLE_ABSL_DCHECK_GE(limit_, ptr()); if (PROTOBUF_PREDICT_FALSE(!HasSpace(n))) return false; *out = AllocateFromExisting(n); @@ -264,7 +258,7 @@ class PROTOBUF_EXPORT SerialArena { static_assert(!std::is_trivially_destructible::value, "This function is only for non-trivial types."); - constexpr int aligned_size = AlignUpTo8(sizeof(T)); + constexpr int aligned_size = ArenaAlignDefault::Ceil(sizeof(T)); constexpr auto destructor = cleanup::arena_destruct_object; size_t required = aligned_size + cleanup::Size(destructor); if (PROTOBUF_PREDICT_FALSE(!HasSpace(required))) { @@ -300,7 +294,7 @@ class PROTOBUF_EXPORT SerialArena { void (*destructor)(void*)) { n = AlignUpTo(n, align); PROTOBUF_UNPOISON_MEMORY_REGION(ptr(), n); - void* ret = internal::AlignTo(ptr(), align); + void* ret = ArenaAlignAs(align).CeilDefaultAligned(ptr()); set_ptr(ptr() + n); GOOGLE_ABSL_DCHECK_GE(limit_, ptr()); AddCleanupFromExisting(ret, destructor); @@ -384,7 +378,8 @@ class PROTOBUF_EXPORT SerialArena { inline void Init(ArenaBlock* b, size_t offset); public: - static constexpr size_t kBlockHeaderSize = AlignUpTo8(sizeof(ArenaBlock)); + static constexpr size_t kBlockHeaderSize = + ArenaAlignDefault::Ceil(sizeof(ArenaBlock)); }; // This class provides the core Arena memory allocation library. Different @@ -603,7 +598,7 @@ class PROTOBUF_EXPORT ThreadSafeArena { static constexpr size_t kSerialArenaSize = (sizeof(SerialArena) + 7) & static_cast(-8); static constexpr size_t kAllocPolicySize = - AlignUpTo8(sizeof(AllocationPolicy)); + ArenaAlignDefault::Ceil(sizeof(AllocationPolicy)); static constexpr size_t kMaxCleanupNodeSize = 16; static_assert(kBlockHeaderSize % 8 == 0, "kBlockHeaderSize must be a multiple of 8.");