Commit dd5c3642 authored by Mark Mentovai's avatar Mark Mentovai Committed by Commit Bot

PartitionAlloc: Set the thread cache threshold by size, not bucket index

The bucket index is needed internally, but it can be computed from the
threshold size rather than being hard-coded.

This was partially fixed by 94069728
(https://chromium-review.googlesource.com/c/2428896) which added a
constant for the threshold size which can be checked by PA_CHECK.
Unfortunately, that new CHECK was violated on mac-arm64, where the
environment reports that the system allocator guarantees a minimum
8-byte alignment, thus the bucket corresponding to allocation size 512
occurs at a higher index than other 64-bit platforms, where the
environment reports a minimum 16-byte alignment. It’s possible that the
alignment reported on mac-arm64 is a bug, because the system allocator
does in fact impose a 16-byte minimum alignment, but perhaps it believes
that while this is true, it’s only guaranteeing 8-byte alignment.

Regardless, the new PA_CHECK did catch that the thread cache threshold
was not tuned as expected on mac-arm64. It seems to be intended to have
used 512 everywhere, but 256 was being used on mac-arm64 instead.

Bug: 1135726
Change-Id: Ic820842d2a6eb669a0a65afb68c34638adf598d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463445
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: default avatarBenoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816239}
parent 7c2b8cc3
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include <cstdint> #include <cstdint>
#include <memory> #include <memory>
#include "base/allocator/partition_allocator/checked_ptr_support.h"
#include "base/allocator/partition_allocator/partition_alloc_forward.h" #include "base/allocator/partition_allocator/partition_alloc_forward.h"
#include "base/allocator/partition_allocator/partition_cookie.h" #include "base/allocator/partition_allocator/partition_cookie.h"
#include "base/allocator/partition_allocator/partition_freelist_entry.h" #include "base/allocator/partition_allocator/partition_freelist_entry.h"
...@@ -19,7 +18,6 @@ ...@@ -19,7 +18,6 @@
#include "base/no_destructor.h" #include "base/no_destructor.h"
#include "base/partition_alloc_buildflags.h" #include "base/partition_alloc_buildflags.h"
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "build/build_config.h"
namespace base { namespace base {
...@@ -95,6 +93,10 @@ class BASE_EXPORT ThreadCacheRegistry { ...@@ -95,6 +93,10 @@ class BASE_EXPORT ThreadCacheRegistry {
#define GET_COUNTER(counter) 0 #define GET_COUNTER(counter) 0
#endif // defined(PA_ENABLE_THREAD_CACHE_STATISTICS) #endif // defined(PA_ENABLE_THREAD_CACHE_STATISTICS)
ALWAYS_INLINE static constexpr int ConstexprLog2(size_t n) {
return n < 1 ? -1 : (n < 2 ? 0 : (1 + ConstexprLog2(n >> 1)));
}
// Per-thread cache. *Not* threadsafe, must only be accessed from a single // Per-thread cache. *Not* threadsafe, must only be accessed from a single
// thread. // thread.
// //
...@@ -170,14 +172,11 @@ class BASE_EXPORT ThreadCache { ...@@ -170,14 +172,11 @@ class BASE_EXPORT ThreadCache {
}; };
// TODO(lizeb): Optimize the threshold. // TODO(lizeb): Optimize the threshold.
#if defined(ARCH_CPU_64_BITS) || ENABLE_TAG_FOR_MTE_CHECKED_PTR
static constexpr size_t kBucketCount = 41;
#else
static constexpr size_t kBucketCount = 49;
#endif
// Checked in ThreadCache::Init(), not with static_assert() as the size is not
// set at compile-time.
static constexpr size_t kSizeThreshold = 512; static constexpr size_t kSizeThreshold = 512;
static constexpr size_t kBucketCount =
((ConstexprLog2(kSizeThreshold) - kMinBucketedOrder + 1)
<< kNumBucketsPerOrderBits) +
1;
static_assert( static_assert(
kBucketCount < kNumBuckets, kBucketCount < kNumBuckets,
"Cannot have more cached buckets than what the allocator supports"); "Cannot have more cached buckets than what the allocator supports");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment