0

Allow (and use) std::hardware_destructive_interference_size.

Bug: none
Change-Id: Ic3282588f4c6e75c7fcbff6f9c64827b5c18a10d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5767325
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Owners-Override: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1350762}
This commit is contained in:
Peter Kasting
2024-09-04 13:56:46 +00:00
committed by Chromium LUCI CQ
parent 45cd057118
commit fc838e8cc8
6 changed files with 38 additions and 45 deletions
base/allocator/partition_allocator/src/partition_alloc
components/media_router/common/providers/cast/channel
styleguide/c++

@@ -6,6 +6,7 @@
#define PARTITION_ALLOC_PARTITION_ADDRESS_SPACE_H_ #define PARTITION_ALLOC_PARTITION_ADDRESS_SPACE_H_
#include <cstddef> #include <cstddef>
#include <new>
#include <utility> #include <utility>
#include "partition_alloc/address_pool_manager_types.h" #include "partition_alloc/address_pool_manager_types.h"
@@ -436,7 +437,8 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) PartitionAddressSpace {
static constexpr uintptr_t kUninitializedPoolBaseAddress = static constexpr uintptr_t kUninitializedPoolBaseAddress =
static_cast<uintptr_t>(-1); static_cast<uintptr_t>(-1);
struct alignas(kPartitionCachelineSize) PA_THREAD_ISOLATED_ALIGN PoolSetup { struct alignas(std::hardware_destructive_interference_size)
PA_THREAD_ISOLATED_ALIGN PoolSetup {
// Before PartitionAddressSpace::Init(), no allocation are allocated from a // Before PartitionAddressSpace::Init(), no allocation are allocated from a
// reserved address space. Therefore, set *_pool_base_address_ initially to // reserved address space. Therefore, set *_pool_base_address_ initially to
// -1, so that PartitionAddressSpace::IsIn*Pool() always returns false. // -1, so that PartitionAddressSpace::IsIn*Pool() always returns false.
@@ -466,7 +468,9 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) PartitionAddressSpace {
static_assert(sizeof(PoolSetup) % SystemPageSize() == 0, static_assert(sizeof(PoolSetup) % SystemPageSize() == 0,
"PoolSetup has to fill a page(s)"); "PoolSetup has to fill a page(s)");
#else #else
static_assert(sizeof(PoolSetup) % kPartitionCachelineSize == 0, static_assert(sizeof(PoolSetup) %
std::hardware_destructive_interference_size ==
0,
"PoolSetup has to fill a cacheline(s)"); "PoolSetup has to fill a cacheline(s)");
#endif #endif

@@ -75,15 +75,6 @@ using internal::FreeFlags;
namespace internal { namespace internal {
// Size of a cache line. Not all CPUs in the world have a 64 bytes cache line
// size, but as of 2021, most do. This is in particular the case for almost all
// x86_64 and almost all ARM CPUs supported by Chromium. As this is used for
// static alignment, we cannot query the CPU at runtime to determine the actual
// alignment, so use 64 bytes everywhere. Since this is only used to avoid false
// sharing, getting this wrong only results in lower performance, not incorrect
// code.
constexpr size_t kPartitionCachelineSize = 64;
// Underlying partition storage pages (`PartitionPage`s) are a power-of-2 size. // Underlying partition storage pages (`PartitionPage`s) are a power-of-2 size.
// It is typical for a `PartitionPage` to be based on multiple system pages. // It is typical for a `PartitionPage` to be based on multiple system pages.
// Most references to "page" refer to `PartitionPage`s. // Most references to "page" refer to `PartitionPage`s.

@@ -35,6 +35,7 @@
#include <cstddef> #include <cstddef>
#include <cstdint> #include <cstdint>
#include <limits> #include <limits>
#include <new>
#include <optional> #include <optional>
#include <utility> #include <utility>
@@ -225,7 +226,7 @@ struct alignas(64) PA_COMPONENT_EXPORT(PARTITION_ALLOC) PartitionRoot {
// //
// Careful! PartitionAlloc's performance is sensitive to its layout. Please // Careful! PartitionAlloc's performance is sensitive to its layout. Please
// put the fast-path objects in the struct below. // put the fast-path objects in the struct below.
struct alignas(internal::kPartitionCachelineSize) Settings { struct alignas(std::hardware_destructive_interference_size) Settings {
// Chromium-style: Complex constructor needs an explicit out-of-line // Chromium-style: Complex constructor needs an explicit out-of-line
// constructor. // constructor.
Settings(); Settings();
@@ -283,7 +284,7 @@ struct alignas(64) PA_COMPONENT_EXPORT(PARTITION_ALLOC) PartitionRoot {
// Not used on the fastest path (thread cache allocations), but on the fast // Not used on the fastest path (thread cache allocations), but on the fast
// path of the central allocator. // path of the central allocator.
alignas(internal::kPartitionCachelineSize) internal::Lock lock_; alignas(std::hardware_destructive_interference_size) internal::Lock lock_;
Bucket buckets[internal::kNumBuckets] = {}; Bucket buckets[internal::kNumBuckets] = {};
Bucket sentinel_bucket{}; Bucket sentinel_bucket{};

@@ -9,6 +9,7 @@
#include <cstdint> #include <cstdint>
#include <limits> #include <limits>
#include <memory> #include <memory>
#include <new>
#include <optional> #include <optional>
#include "partition_alloc/build_config.h" #include "partition_alloc/build_config.h"
@@ -604,7 +605,7 @@ PA_ALWAYS_INLINE void ThreadCache::PutInBucket(Bucket& bucket,
// % 16. Its distance to the next cacheline is // % 16. Its distance to the next cacheline is
// `64 - ((slot_start & 63) / 16) * 16` // `64 - ((slot_start & 63) / 16) * 16`
static_assert( static_assert(
internal::kPartitionCachelineSize == 64, std::hardware_destructive_interference_size == 64,
"The computation below assumes that cache lines are 64 bytes long."); "The computation below assumes that cache lines are 64 bytes long.");
int distance_to_next_cacheline_in_16_bytes = 4 - ((slot_start >> 4) & 3); int distance_to_next_cacheline_in_16_bytes = 4 - ((slot_start >> 4) & 3);
int slot_size_remaining_in_16_bytes = bucket.slot_size / 16; int slot_size_remaining_in_16_bytes = bucket.slot_size / 16;

@@ -12,6 +12,7 @@
#include <cstdint> #include <cstdint>
#include <cstring> #include <cstring>
#include <new>
#include <optional> #include <optional>
#include <ostream> #include <ostream>
#include <string_view> #include <string_view>
@@ -367,8 +368,7 @@ class EnumTable {
private: private:
#ifdef ARCH_CPU_64_BITS #ifdef ARCH_CPU_64_BITS
// Align the data on a cache line boundary. alignas(std::hardware_destructive_interference_size)
alignas(64)
#endif #endif
std::initializer_list<Entry> data_; std::initializer_list<Entry> data_;
bool is_sorted_; bool is_sorted_;

@@ -574,35 +574,6 @@ Overlaps with utilities in `base/strings/string_number_conversions.h`, which are
easier to use correctly. easier to use correctly.
*** ***
### std::hardware_{con,de}structive_interference_size <sup>[banned]</sup>
```c++
struct SharedData {
ReadOnlyFrequentlyUsed data;
alignas(std::hardware_destructive_interference_size) std::atomic<size_t> counter;
};
```
**Description:** The `std::hardware_destructive_interference_size` constant is
useful to avoid false sharing (destructive interference) between variables that
would otherwise occupy the same cacheline. In contrast,
`std::hardware_constructive_interference_size` is helpful to promote true
sharing (constructive interference), e.g. to support better locality for
non-contended data.
**Documentation:**
[`std::hardware_destructive_interference_size`](https://en.cppreference.com/w/cpp/thread/hardware_destructive_interference_size),
[`std::hardware_constructive_interference_size`](https://en.cppreference.com/w/cpp/thread/hardware_destructive_interference_size)
**Notes:**
*** promo
Banned for now since these are
[not supported yet](https://github.com/llvm/llvm-project/issues/60174). Allow
once supported.
[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/cwktrFxxUY4)
***
### std::in_place{_type,_index}[_t] <sup>[banned]</sup> ### std::in_place{_type,_index}[_t] <sup>[banned]</sup>
```c++ ```c++
@@ -1206,6 +1177,31 @@ avoiding the need to use the `erase(remove(...` paradigm.
[Migration bug](https://crbug.com/1414639) [Migration bug](https://crbug.com/1414639)
*** ***
### std::hardware_{con,de}structive_interference_size <sup>[allowed]</sup>
```c++
struct SharedData {
ReadOnlyFrequentlyUsed data;
alignas(std::hardware_destructive_interference_size) std::atomic<size_t> counter;
};
```
**Description:** The `std::hardware_destructive_interference_size` constant is
useful to avoid false sharing (destructive interference) between variables that
would otherwise occupy the same cacheline. In contrast,
`std::hardware_constructive_interference_size` is helpful to promote true
sharing (constructive interference), e.g. to support better locality for
non-contended data.
**Documentation:**
[`std::hardware_destructive_interference_size`](https://en.cppreference.com/w/cpp/thread/hardware_destructive_interference_size),
[`std::hardware_constructive_interference_size`](https://en.cppreference.com/w/cpp/thread/hardware_destructive_interference_size)
**Notes:**
*** promo
[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/cwktrFxxUY4)
***
### std::is_[un]bounded_array <sup>[allowed]</sup> ### std::is_[un]bounded_array <sup>[allowed]</sup>
```c++ ```c++