From 88173f34dd13ffe6f6d66e4f77df30bec8e95a71 Mon Sep 17 00:00:00 2001 From: Ben Reisner Date: Tue, 1 Sep 2026 10:15:44 -0700 Subject: [PATCH] Experiment with conditionally appending/prepending a span to a bucket depending on its bucket index. For each iteration, we unconditionally remove a span from a bucket and add it to either the front or back of the linked list, depending on its bucket index. For buckets with fewer allocations, we add to the back of the list to deprioritize them. PiperOrigin-RevId: 974565473 --- tcmalloc/central_freelist.h | 38 ++++- tcmalloc/central_freelist_benchmark.cc | 4 +- tcmalloc/central_freelist_fuzz.cc | 18 ++- tcmalloc/central_freelist_test.cc | 186 ++++++++++++++++++++++--- tcmalloc/experiment_config.h | 2 + tcmalloc/global_stats.cc | 11 ++ tcmalloc/hinted_tracker_lists.h | 8 +- tcmalloc/mock_central_freelist.h | 4 +- tcmalloc/mock_static_forwarder.h | 5 +- tcmalloc/mock_transfer_cache.h | 15 +- tcmalloc/parameters.cc | 16 +++ tcmalloc/parameters.h | 3 + tcmalloc/tcmalloc_variants.cmake | 8 ++ tcmalloc/testing/get_stats_test.cc | 18 +++ tcmalloc/transfer_cache.h | 18 ++- tcmalloc/transfer_cache_benchmark.cc | 9 +- tcmalloc/transfer_cache_internals.h | 13 +- tcmalloc/variants.bzl | 6 + 18 files changed, 330 insertions(+), 52 deletions(-) diff --git a/tcmalloc/central_freelist.h b/tcmalloc/central_freelist.h index 21f39f259..7beeb057e 100644 --- a/tcmalloc/central_freelist.h +++ b/tcmalloc/central_freelist.h @@ -135,6 +135,10 @@ static constexpr size_t kFewObjectsAllocMaxLimit = 16; static constexpr size_t kSpansUsedStatBuckets = absl::bit_width(kMaxObjectsToMove); +enum class CflSubbucketPrioritization : bool { + kDisabled = false, + kEnabled = true +}; // Data kept per size-class in central cache. template @@ -155,13 +159,16 @@ class CentralFreeList { #endif pages_per_span_(0), nonempty_(), - use_all_buckets_for_few_object_spans_(false) { + use_all_buckets_for_few_object_spans_(false), + cfl_subbucket_prioritization_(CflSubbucketPrioritization::kDisabled) { } CentralFreeList(const CentralFreeList&) = delete; CentralFreeList& operator=(const CentralFreeList&) = delete; - void Init(size_t size_class) ABSL_LOCKS_EXCLUDED(lock_); + void Init(size_t size_class, + CflSubbucketPrioritization cfl_subbucket_prioritization) + ABSL_LOCKS_EXCLUDED(lock_); // These methods all do internal locking. @@ -395,12 +402,15 @@ class CentralFreeList { bool use_all_buckets_for_few_object_spans_; + CflSubbucketPrioritization cfl_subbucket_prioritization_; + ABSL_ATTRIBUTE_NO_UNIQUE_ADDRESS Forwarder forwarder_; }; // Like a constructor and hence we disable thread safety analysis. template -inline void CentralFreeList::Init(size_t size_class) +inline void CentralFreeList::Init( + size_t size_class, CflSubbucketPrioritization cfl_subbucket_prioritization) ABSL_NO_THREAD_SAFETY_ANALYSIS { size_class_ = size_class; object_size_ = forwarder_.class_to_size(size_class); @@ -427,6 +437,7 @@ inline void CentralFreeList::Init(size_t size_class) TC_ASSERT_LE(absl::bit_width(objects_per_span_), kSpanUtilBucketCapacity); num_to_move_ = forwarder_.num_objects_to_move(size_class); + cfl_subbucket_prioritization_ = cfl_subbucket_prioritization; } template @@ -476,10 +487,24 @@ inline Span* CentralFreeList::ReleaseToSpans( // we remove it from the previous list and add it to the desired list indexed // by cur_index. const uint8_t cur_index = IndexFor(cur_allocated, cur_bitwidth); - if (kDeferredNonEmpty && ABSL_PREDICT_FALSE(was_empty)) { - nonempty_.Add(span, cur_index); + + if (cfl_subbucket_prioritization_ == CflSubbucketPrioritization::kEnabled) { + const uint8_t relative_index = cur_index % kNumLists; + const uint8_t mid = kNumLists / 2; + const bool use_prepend = relative_index <= mid; + + if (kDeferredNonEmpty && ABSL_PREDICT_FALSE(was_empty)) { + nonempty_.Add(span, cur_index, use_prepend); + } else { + nonempty_.Remove(span, prev_index); + nonempty_.Add(span, cur_index, use_prepend); + } span->set_nonempty_index(cur_index); - } else if (ABSL_PREDICT_FALSE(cur_index != prev_index)) { + } else { + if (kDeferredNonEmpty && ABSL_PREDICT_FALSE(was_empty)) { + nonempty_.Add(span, cur_index); + span->set_nonempty_index(cur_index); + } else if (ABSL_PREDICT_FALSE(cur_index != prev_index)) { #ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING nonempty_.Remove(span, prev_index); nonempty_.Add(span, cur_index); @@ -487,6 +512,7 @@ inline Span* CentralFreeList::ReleaseToSpans( nonempty_.Move(span, prev_index, cur_index); #endif span->set_nonempty_index(cur_index); + } } return nullptr; } diff --git a/tcmalloc/central_freelist_benchmark.cc b/tcmalloc/central_freelist_benchmark.cc index 47b384f44..4ae2a47a1 100644 --- a/tcmalloc/central_freelist_benchmark.cc +++ b/tcmalloc/central_freelist_benchmark.cc @@ -176,7 +176,9 @@ class BenchmarkEnv { size_t num_objects_to_move) { cache_.forwarder().Init(class_size, span_bytes, num_objects_to_move, kPageSize); - cache_.Init(kSizeClass); + cache_.Init( + kSizeClass, + central_freelist_internal::CflSubbucketPrioritization::kDisabled); } CentralFreeList& central_freelist() { return cache_; } diff --git a/tcmalloc/central_freelist_fuzz.cc b/tcmalloc/central_freelist_fuzz.cc index 0fb31dbe3..6786fdc4b 100644 --- a/tcmalloc/central_freelist_fuzz.cc +++ b/tcmalloc/central_freelist_fuzz.cc @@ -51,8 +51,11 @@ struct State { CentralFreelistEnv env; std::vector objects; - State(size_t object_size, Length num_pages, size_t num_objects_to_move) - : env(object_size, Bytes(num_pages.in_bytes()), num_objects_to_move) {} + State(size_t object_size, Length num_pages, size_t num_objects_to_move, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization) + : env(object_size, Bytes(num_pages.in_bytes()), num_objects_to_move, + cfl_subbucket_prioritization) {} ~State(); @@ -194,12 +197,15 @@ void AbslStringify(Sink& sink, const Instruction& i) { } void FuzzCFL(size_t object_size, Length num_pages, size_t num_objects_to_move, - const std::vector& instructions) { + const std::vector& instructions, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization) { // TODO(271282540): Add support for multiple size classes for fuzzing. if (!SizeMap::IsValidSizeClass(object_size, num_pages, num_objects_to_move)) { return; } - State state(object_size, num_pages, num_objects_to_move); + State state(object_size, num_pages, num_objects_to_move, + cfl_subbucket_prioritization); for (const auto& instruction : instructions) { std::visit([&](const auto& arg) { arg.Perform(state); }, instruction); @@ -226,7 +232,9 @@ auto GetInstructionDomain() { FUZZ_TEST(CentralFreeListTest, FuzzCFL) .WithDomains(fuzztest::InRange(0, kMaxSize), AnyLength(), fuzztest::Arbitrary(), - fuzztest::VectorOf(GetInstructionDomain())); + fuzztest::VectorOf(GetInstructionDomain()), + fuzztest::Arbitrary< + central_freelist_internal::CflSubbucketPrioritization>()); } // namespace } // namespace tcmalloc::tcmalloc_internal diff --git a/tcmalloc/central_freelist_test.cc b/tcmalloc/central_freelist_test.cc index c6847937d..385b2f73e 100644 --- a/tcmalloc/central_freelist_test.cc +++ b/tcmalloc/central_freelist_test.cc @@ -435,7 +435,8 @@ namespace { using central_freelist_internal::kNumLists; using TypeParam = FakeCentralFreeListEnvironment< central_freelist_internal::CentralFreeList>; -using CentralFreeListTest = ::testing::TestWithParam; +using CentralFreeListTest = ::testing::TestWithParam>; TEST_P(CentralFreeListTest, IsolatedSmoke) { #if ABSL_HAVE_HWADDRESS_SANITIZER @@ -443,7 +444,8 @@ TEST_P(CentralFreeListTest, IsolatedSmoke) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); EXPECT_CALL(e.forwarder(), AllocateSpan).Times(1); absl::FixedArray batch(e.batch_size()); @@ -500,7 +502,8 @@ TEST_P(CentralFreeListTest, SameSpanTracking) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); if (e.objects_per_span() <= 1) { GTEST_SKIP() << "Single-object spans skip CentralFreeList InsertRange"; } @@ -531,7 +534,8 @@ TEST_P(CentralFreeListTest, SpanUtilizationHistogram) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); constexpr size_t kNumSpans = 10; // Request kNumSpans spans. @@ -638,7 +642,8 @@ TEST_P(CentralFreeListTest, SinglePopulate) { // Make sure that we allocate up to kObjectsPerSpan objects in both the span // prioritization states. - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); // Try to fetch sufficiently large number of objects at startup. const int num_objects_to_fetch = kMaxObjectsToMove; std::vector objects(num_objects_to_fetch, nullptr); @@ -714,7 +719,8 @@ TEST_P(CentralFreeListTest, BitwidthIndexedNonEmptyLists) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); if (e.objects_per_span() <= 2 * kNumLists) { GTEST_SKIP() << "Skipping test as one hot encoding used for few object spans."; @@ -732,7 +738,8 @@ TEST_P(CentralFreeListTest, DirectIndexedEncodedNonEmptyLists) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); if (e.objects_per_span() > 2 * kNumLists) { GTEST_SKIP() << "Skipping test as one hot encoding not required."; } @@ -755,7 +762,8 @@ TEST_P(CentralFreeListTest, SpanPriority) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); // If the number of objects per span is less than 2, we do not use more than // one nonempty_ lists. So, we can not prioritize the spans based on how many @@ -851,12 +859,134 @@ TEST_P(CentralFreeListTest, SpanPriority) { // Return rest of the objects. for (int span = 0; span < kNumSpans; ++span) { - for (int i = 0; i < objects[span].size(); ++i) { + for (size_t i = 0; i < objects[span].size(); ++i) { e.central_freelist().InsertRange({&objects[span][i], 1}); } } } +// Checks if spans in the same nonempty_ bucket are ordered correctly based on +// their subbucket designation. Spans routing to the append buckets should +// exhibit FIFO ordering, while spans routing to the prepend buckets should be +// LIFO. +TEST_P(CentralFreeListTest, SpanReinsertionOrder) { +#if ABSL_HAVE_HWADDRESS_SANITIZER + GTEST_SKIP() + << "Skipping under HWASan, which uses the top bits of the pointer."; +#endif + + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); + + const int objects_per_span = e.objects_per_span(); + if (objects_per_span < 3 || kNumLists < 2) return; + + constexpr int kNumSpans = 2; + absl::FixedArray> original_objects(kNumSpans); + void* batch[kMaxObjectsToMove]; + + // Completely drain all objects from kNumSpans (2) spans so both spans become + // empty, recording allocated objects to track span membership later. + const size_t to_fetch = objects_per_span; + for (int span = 0; span < kNumSpans; ++span) { + size_t fetched = 0; + while (fetched < to_fetch) { + const size_t n = to_fetch - fetched; + int got = e.central_freelist().RemoveRange( + absl::MakeSpan(batch, std::min(n, e.batch_size()))); + for (int i = 0; i < got; ++i) original_objects[span].push_back(batch[i]); + fetched += got; + } + } + + auto IndexFor = [objects_per_span](int allocated) -> size_t { + if (objects_per_span <= 2 * kNumLists) { + if (allocated <= kNumLists) return kNumLists - allocated; + return 0; + } else { + size_t bitwidth = absl::bit_width(static_cast(allocated)); + return kNumLists - std::min(bitwidth, kNumLists); + } + }; + + constexpr uint8_t mid = kNumLists / 2; + + struct TestCase { + int target_objects; + bool expects_prepend; + }; + + bool experiment_disabled = + std::get<1>(GetParam()) == + central_freelist_internal::CflSubbucketPrioritization::kDisabled; + + std::vector test_cases; + std::array bucket_tested = {false}; + // Find an allocation count for each unique bucket index, + // determining whether spans in that bucket are expected to be prepended + // (LIFO) or appended (FIFO). + for (int allocated = 1; allocated < objects_per_span; ++allocated) { + size_t idx = IndexFor(allocated); + if (!bucket_tested[idx]) { + bucket_tested[idx] = true; + bool expects_prepend = experiment_disabled || (idx % kNumLists <= mid); + test_cases.emplace_back(allocated, expects_prepend); + } + } + + // Reverse test_cases so target_objects is in descending order. + // This makes `to_return` increase in each iteration, allowing us to + // return objects incrementally without resetting. + std::reverse(test_cases.begin(), test_cases.end()); + + size_t returned_so_far = 0; + // Test each bucket: incrementally return objects to span 0 first, then + // span 1 second. Span 0 enters the target bucket first and span 1 second, so + // LIFO order yields span 1 and FIFO order yields span 0. + for (const auto& tc : test_cases) { + size_t target_returned = objects_per_span - tc.target_objects; + size_t additional_to_return = target_returned - returned_so_far; + + // Return only the additional objects to reach the target state. + for (int span = 0; span < kNumSpans; ++span) { + for (size_t i = 0; i < additional_to_return; ++i) { + e.central_freelist().InsertRange( + {&original_objects[span] + [objects_per_span - 1 - (returned_so_far + i)], + 1}); + } + } + returned_so_far = target_returned; + + // Now both spans should be identically bucketed. Pull 1. + int got = e.central_freelist().RemoveRange(absl::MakeSpan(batch, 1)); + EXPECT_EQ(got, 1); + + bool drew_from_span_1 = false; + for (void* ptr : original_objects[1]) { + if (ptr == batch[0]) drew_from_span_1 = true; + } + + if (tc.expects_prepend) { + EXPECT_TRUE(drew_from_span_1) + << "Expected LIFO for bucket " << tc.target_objects; + } else { + EXPECT_FALSE(drew_from_span_1) + << "Expected FIFO for bucket " << tc.target_objects; + } + + // Return the drawn object + e.central_freelist().InsertRange({batch, static_cast(got)}); + } + + // Cleanup: return all objects to the page heap at the very end + for (int span = 0; span < kNumSpans; ++span) { + for (size_t i = 0; i < objects_per_span - returned_so_far; ++i) { + e.central_freelist().InsertRange({&original_objects[span][i], 1}); + } + } +} + struct SpanLifetimes { absl::flat_hash_map live; absl::flat_hash_map completed; @@ -935,7 +1065,8 @@ TEST_P(CentralFreeListTest, HookTracing) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); static int insert_count = 0; static int remove_count = 0; @@ -970,7 +1101,8 @@ TEST_P(CentralFreeListTest, SpanLifetime) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); // Skip the check for objects_per_span = 1 since such spans skip most of the // central freelist's logic. if (e.objects_per_span() == 1) { @@ -1016,7 +1148,8 @@ TEST_P(CentralFreeListTest, SpanAllocationTracker) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); const int objects_per_span = e.objects_per_span(); if (objects_per_span == 1) return; @@ -1090,8 +1223,9 @@ TEST_P(CentralFreeListTest, SameSpans) { #ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING GTEST_SKIP() << "Stats are non-functional when optimization is not enabled."; #endif - const int num_to_move = GetParam().num_to_move; - TypeParam e(GetParam().size, GetParam().bytes, num_to_move); + const int num_to_move = std::get<0>(GetParam()).num_to_move; + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + num_to_move, std::get<1>(GetParam())); // Roundtrip a batch. void* batch[kMaxObjectsToMove]; @@ -1111,8 +1245,9 @@ TEST_P(CentralFreeListTest, SameSpans) { // Check the stats after the first insertion. { - std::string expected_stats = absl::StrFormat( - "class %3d [ %8zu bytes ] :", e.kSizeClass, GetParam().size); + std::string expected_stats = + absl::StrFormat("class %3d [ %8zu bytes ] :", e.kSizeClass, + std::get<0>(GetParam()).size); for (int i = 0; i < CentralFreeList::kSameSpanBucketCapacity; ++i) { const bool first_batch = e.objects_per_span() > 1 && i == absl::bit_width(static_cast( @@ -1153,7 +1288,8 @@ TEST_P(CentralFreeListTest, MultipleSpans) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); std::vector all_objects; constexpr size_t kNumSpans = 10; @@ -1239,7 +1375,8 @@ TEST_P(CentralFreeListTest, PassSpanDensityToPageheap) { << "Skipping under HWASan, which uses the top bits of the pointer."; #endif - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); + TypeParam e(std::get<0>(GetParam()).size, std::get<0>(GetParam()).bytes, + std::get<0>(GetParam()).num_to_move, std::get<1>(GetParam())); ASSERT_GE(e.objects_per_span(), 1); auto test_function = [&](size_t num_objects, AccessDensityPrediction density) { @@ -1261,10 +1398,15 @@ TEST_P(CentralFreeListTest, PassSpanDensityToPageheap) { test_function(1, AccessDensityPrediction::kDense); test_function(e.objects_per_span(), AccessDensityPrediction::kDense); } -INSTANTIATE_TEST_SUITE_P(CentralFreeList, CentralFreeListTest, - // We skip the first size class since it is set to 0. - testing::ValuesIn(kSizeClasses.classes.begin() + 1, - kSizeClasses.classes.end())); +INSTANTIATE_TEST_SUITE_P( + CentralFreeList, CentralFreeListTest, + testing::Combine( + // We skip the first size class since it is set to 0. + testing::ValuesIn(kSizeClasses.classes.begin() + 1, + kSizeClasses.classes.end()), + testing::Values( + central_freelist_internal::CflSubbucketPrioritization::kDisabled, + central_freelist_internal::CflSubbucketPrioritization::kEnabled))); } // namespace } // namespace tcmalloc_internal diff --git a/tcmalloc/experiment_config.h b/tcmalloc/experiment_config.h index 70adeae5d..48dcecda1 100644 --- a/tcmalloc/experiment_config.h +++ b/tcmalloc/experiment_config.h @@ -30,6 +30,7 @@ enum class Experiment : int { TCMALLOC_SONIC_MADVISE_SAMPLED_ALLOCATIONS_HOLDBACK, // TODO: b/540945006 - Complete experiment. TCMALLOC_SONIC_MADV_NOHUGEPAGE_REGIONS, // TODO: b/527907199 - Complete experiment. TEST_ONLY_MM_VCPU, // TODO: b/245776120 - Complete experiment. + TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION, // TODO: b/505486241 - Complete experiment. TEST_ONLY_TCMALLOC_HEAP_PARTITIONING, // TODO: b/446814339 - Complete experiment. TEST_ONLY_TCMALLOC_POW2_SIZECLASS, TEST_ONLY_TCMALLOC_RELEASE_STALE_PAGES, // TODO: b/527473378 - Complete experiment. @@ -60,6 +61,7 @@ inline constexpr ExperimentConfig experiments[] = { {Experiment::TCMALLOC_SONIC_MADVISE_SAMPLED_ALLOCATIONS_HOLDBACK, "TCMALLOC_SONIC_MADVISE_SAMPLED_ALLOCATIONS_HOLDBACK", /*brittle=*/false, /*force_disable=*/false, /*rollout_lower_bound=*/0, /*rollout_upper_bound=*/0.5}, {Experiment::TCMALLOC_SONIC_MADV_NOHUGEPAGE_REGIONS, "TCMALLOC_SONIC_MADV_NOHUGEPAGE_REGIONS", /*brittle=*/false, /*force_disable=*/false, /*rollout_lower_bound=*/0, /*rollout_upper_bound=*/0.01}, {Experiment::TEST_ONLY_MM_VCPU, "TEST_ONLY_MM_VCPU"}, + {Experiment::TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION, "TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION"}, {Experiment::TEST_ONLY_TCMALLOC_HEAP_PARTITIONING, "TEST_ONLY_TCMALLOC_HEAP_PARTITIONING"}, {Experiment::TEST_ONLY_TCMALLOC_POW2_SIZECLASS, "TEST_ONLY_TCMALLOC_POW2_SIZECLASS", /*brittle=*/true}, {Experiment::TEST_ONLY_TCMALLOC_RELEASE_STALE_PAGES, "TEST_ONLY_TCMALLOC_RELEASE_STALE_PAGES"}, diff --git a/tcmalloc/global_stats.cc b/tcmalloc/global_stats.cc index a1725cb9f..1ce5aae01 100644 --- a/tcmalloc/global_stats.cc +++ b/tcmalloc/global_stats.cc @@ -697,6 +697,12 @@ void DumpStats(Printer& out, int level) { out.printf("PARAMETER tcmalloc_release_drained_slab_metadata %d\n", Parameters::release_drained_slab_metadata()); + out.printf( + "PARAMETER tcmalloc_cfl_subbucket_prioritization %d\n", + Parameters::cfl_subbucket_prioritization() == + central_freelist_internal::CflSubbucketPrioritization::kEnabled + ? 1 + : 0); } } @@ -960,6 +966,11 @@ void DumpStatsInPbtxt(Printer& out, int level) { region.PrintBool("tcmalloc_release_drained_slab_metadata", Parameters::release_drained_slab_metadata()); + + region.PrintBool( + "tcmalloc_cfl_subbucket_prioritization", + Parameters::cfl_subbucket_prioritization() == + central_freelist_internal::CflSubbucketPrioritization::kEnabled); } bool GetNumericProperty(const char* name_data, size_t name_size, diff --git a/tcmalloc/hinted_tracker_lists.h b/tcmalloc/hinted_tracker_lists.h index ba9e33af7..5e6c74723 100644 --- a/tcmalloc/hinted_tracker_lists.h +++ b/tcmalloc/hinted_tracker_lists.h @@ -76,10 +76,14 @@ class HintedTrackerLists { // Adds pointer to the nonempty_[i] list. // REQUIRES: i < N && pt != nullptr. void Add(TrackerType* absl_nonnull pt TCMALLOC_CAPTURED_BY_THIS, - const size_t i) { + const size_t i, bool prepend = true) { TC_ASSERT_LT(i, N); TC_ASSERT_NE(pt, nullptr); - lists_[i].prepend(pt); + if (prepend) { + lists_[i].prepend(pt); + } else { + lists_[i].append(pt); + } ++size_; nonempty_.SetBit(i); } diff --git a/tcmalloc/mock_central_freelist.h b/tcmalloc/mock_central_freelist.h index 030b74875..94edca7f4 100644 --- a/tcmalloc/mock_central_freelist.h +++ b/tcmalloc/mock_central_freelist.h @@ -43,7 +43,9 @@ class FakeCentralFreeListBase { FakeCentralFreeListBase(const FakeCentralFreeListBase&) = delete; FakeCentralFreeListBase& operator=(const FakeCentralFreeListBase&) = delete; - static constexpr void Init(size_t) {} + static constexpr void Init( + size_t, central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization) {} }; // CentralFreeList implementation that backs onto the system's malloc. diff --git a/tcmalloc/mock_static_forwarder.h b/tcmalloc/mock_static_forwarder.h index 737a0d2af..a77a8293a 100644 --- a/tcmalloc/mock_static_forwarder.h +++ b/tcmalloc/mock_static_forwarder.h @@ -29,6 +29,7 @@ #include "absl/synchronization/mutex.h" #include "absl/time/time.h" #include "absl/types/span.h" +#include "tcmalloc/central_freelist.h" #include "tcmalloc/common.h" #include "tcmalloc/internal/config.h" #include "tcmalloc/internal/hook_list.h" @@ -239,11 +240,13 @@ class FakeCentralFreeListEnvironment { explicit FakeCentralFreeListEnvironment( size_t class_size, Bytes span_bytes, size_t num_objects_to_move, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization, size_t page_size = kPageSize, double clock_frequency = absl::ToDoubleNanoseconds(absl::Seconds(2))) { forwarder().Init(class_size, span_bytes, num_objects_to_move, page_size, clock_frequency); - cache_.Init(kSizeClass); + cache_.Init(kSizeClass, cfl_subbucket_prioritization); } ~FakeCentralFreeListEnvironment() { EXPECT_EQ(cache_.length(), 0); } diff --git a/tcmalloc/mock_transfer_cache.h b/tcmalloc/mock_transfer_cache.h index 84dd02c88..d19bf33af 100644 --- a/tcmalloc/mock_transfer_cache.h +++ b/tcmalloc/mock_transfer_cache.h @@ -145,8 +145,13 @@ class FakeTransferCacheEnvironment { ::tcmalloc::tcmalloc_internal::kMaxObjectsToMove; static constexpr int kBatchSize = Manager::num_objects_to_move(1); - FakeTransferCacheEnvironment() : manager_(), cache_(&manager_, 1) { Init(); } - + FakeTransferCacheEnvironment() + : manager_(), + cache_( + &manager_, 1, + central_freelist_internal::CflSubbucketPrioritization::kDisabled) { + Init(); + } ~FakeTransferCacheEnvironment() { Drain(); } bool Shrink() { return cache_.ShrinkCache(kSizeClass); } @@ -240,10 +245,12 @@ class ThreeSizeClassManager : public FakeTransferCacheManager { ThreeSizeClassManager() { for (int i = 0; i < 3; ++i) { caches_[i] = std::make_unique( - this, i); + this, i, + central_freelist_internal::CflSubbucketPrioritization::kDisabled); } caches_[kColdSizeClass] = std::make_unique( - this, kColdSizeClass); + this, kColdSizeClass, + central_freelist_internal::CflSubbucketPrioritization::kDisabled); } constexpr static size_t class_to_size(int size_class) { diff --git a/tcmalloc/parameters.cc b/tcmalloc/parameters.cc index b1c0113ce..1d2d5306f 100644 --- a/tcmalloc/parameters.cc +++ b/tcmalloc/parameters.cc @@ -374,6 +374,22 @@ ReleaseStalePages Parameters::release_stale_pages() { return v.load(std::memory_order_relaxed); } +central_freelist_internal::CflSubbucketPrioritization +Parameters::cfl_subbucket_prioritization() { + ABSL_CONST_INIT static absl::once_flag flag; + ABSL_CONST_INIT static std::atomic< + central_freelist_internal::CflSubbucketPrioritization> + v{central_freelist_internal::CflSubbucketPrioritization::kDisabled}; + absl::base_internal::LowLevelCallOnce(&flag, [&]() { + v.store( + central_freelist_internal::CflSubbucketPrioritization{ + IsExperimentActive( + Experiment::TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION)}, + std::memory_order_relaxed); + }); + return v.load(std::memory_order_relaxed); +} + int32_t Parameters::max_per_cpu_cache_size() { return tc_globals.cpu_cache().CacheLimit(); } diff --git a/tcmalloc/parameters.h b/tcmalloc/parameters.h index ddcb5ccd8..aaaae3215 100644 --- a/tcmalloc/parameters.h +++ b/tcmalloc/parameters.h @@ -228,6 +228,9 @@ class Parameters { // TODO: b/527473378 - Remove this function once the experiment is cleaned up. static ReleaseStalePages release_stale_pages(); + static central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization(); + private: friend void ::TCMalloc_Internal_SetBackgroundReleaseRate(size_t v); friend void ::TCMalloc_Internal_SetGuardedSamplingInterval(int64_t v); diff --git a/tcmalloc/tcmalloc_variants.cmake b/tcmalloc/tcmalloc_variants.cmake index da45788fe..f352cf7d2 100644 --- a/tcmalloc/tcmalloc_variants.cmake +++ b/tcmalloc/tcmalloc_variants.cmake @@ -323,6 +323,14 @@ function(tcmalloc_cc_test_variants) DEPS ${TCMALLOC_DEPS} $ ) set_tests_properties(${TCMALLOC_NAME}_tcmalloc_madv_sampled_holdback PROPERTIES ENVIRONMENT "BORG_EXPERIMENTS=TCMALLOC_SONIC_MADVISE_SAMPLED_ALLOCATIONS_HOLDBACK;TEST_TMPDIR=${CMAKE_CURRENT_BINARY_DIR};TEST_SRCDIR=${CMAKE_SOURCE_DIR}") + tcmalloc_cc_test(NAME ${TCMALLOC_NAME}_tcmalloc_cfl_subbucket_prioritization + SRCS ${TCMALLOC_SRCS} + HDRS ${TCMALLOC_HDRS} + COPTS ${TCMALLOC_COPTS} + LINKOPTS ${TCMALLOC_LINKOPTS} + DEPS ${TCMALLOC_DEPS} $ + ) + set_tests_properties(${TCMALLOC_NAME}_tcmalloc_cfl_subbucket_prioritization PROPERTIES ENVIRONMENT "BORG_EXPERIMENTS=TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION;TEST_TMPDIR=${CMAKE_CURRENT_BINARY_DIR};TEST_SRCDIR=${CMAKE_SOURCE_DIR}") endfunction() function(tcmalloc_cc_binary_variants) diff --git a/tcmalloc/testing/get_stats_test.cc b/tcmalloc/testing/get_stats_test.cc index c97790df8..344a98e4b 100644 --- a/tcmalloc/testing/get_stats_test.cc +++ b/tcmalloc/testing/get_stats_test.cc @@ -181,6 +181,13 @@ TEST_F(GetStatsTest, Pbtxt) { EXPECT_THAT(buf, HasSubstr("tcmalloc_release_drained_slab_metadata: false")); + if (IsExperimentActive( + Experiment::TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION)) { + EXPECT_THAT(buf, HasSubstr("tcmalloc_cfl_subbucket_prioritization: true")); + } else { + EXPECT_THAT(buf, HasSubstr("tcmalloc_cfl_subbucket_prioritization: false")); + } + sized_delete(alloc, kSize); } @@ -322,6 +329,17 @@ TEST_F(GetStatsTest, Parameters) { EXPECT_THAT( buf, HasSubstr(R"(PARAMETER tcmalloc_release_drained_slab_metadata 0)")); + + if (IsExperimentActive( + Experiment::TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION)) { + EXPECT_THAT( + buf, + HasSubstr(R"(PARAMETER tcmalloc_cfl_subbucket_prioritization 1)")); + } else { + EXPECT_THAT( + buf, + HasSubstr(R"(PARAMETER tcmalloc_cfl_subbucket_prioritization 0)")); + } } Parameters::set_hpaa_subrelease(true); diff --git a/tcmalloc/transfer_cache.h b/tcmalloc/transfer_cache.h index 5b05d55ed..016af0ee8 100644 --- a/tcmalloc/transfer_cache.h +++ b/tcmalloc/transfer_cache.h @@ -105,7 +105,12 @@ class ProdCpuLayout { // Forwards calls to the unsharded TransferCache. class BackingTransferCache { public: - void Init(int size_class) { size_class_ = size_class; } + void Init(int size_class, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization = central_freelist_internal:: + CflSubbucketPrioritization::kDisabled) { + size_class_ = size_class; + } void InsertRange(absl::Span batch) const; [[nodiscard]] int RemoveRange(absl::Span batch) const; int size_class() const { return size_class_; } @@ -380,8 +385,10 @@ class ShardedTransferCacheManagerBase { : LargeCacheCapacity(size_class); new (&new_caches[size_class]) TransferCache(owner_, capacity.capacity > 0 ? size_class : 0, - {capacity.capacity, capacity.max_capacity}); - new_caches[size_class].freelist().Init(size_class); + {capacity.capacity, capacity.max_capacity}, + Parameters::cfl_subbucket_prioritization()); + new_caches[size_class].freelist().Init( + size_class, Parameters::cfl_subbucket_prioritization()); } shard.transfer_caches = new_caches; active_shards_.fetch_add(1, std::memory_order_relaxed); @@ -478,7 +485,8 @@ class TransferCacheManager : public StaticForwarder { void InitCaches() { for (int i = 0; i < kNumClasses; ++i) { - new (&cache_[i].tc) TransferCache(this, i); + new (&cache_[i].tc) + TransferCache(this, i, Parameters::cfl_subbucket_prioritization()); } } @@ -573,7 +581,7 @@ class TransferCacheManager { void Init() { for (int i = 0; i < kNumClasses; ++i) { - freelist_[i].Init(i); + freelist_[i].Init(i, Parameters::cfl_subbucket_prioritization()); } } diff --git a/tcmalloc/transfer_cache_benchmark.cc b/tcmalloc/transfer_cache_benchmark.cc index fbe2dede2..159d2e3d9 100644 --- a/tcmalloc/transfer_cache_benchmark.cc +++ b/tcmalloc/transfer_cache_benchmark.cc @@ -45,7 +45,14 @@ void BM_CrossThread(benchmark::State& state) { void* batch[kMaxObjectsToMove]; struct CrossThreadState { - CrossThreadState() : m{}, c{Cache(&m, 1), Cache(&m, 1)} {} + CrossThreadState() + : m{}, + c{Cache(&m, 1, + central_freelist_internal::CflSubbucketPrioritization:: + kDisabled), + Cache(&m, 1, + central_freelist_internal::CflSubbucketPrioritization:: + kDisabled)} {} FakeTransferCacheManager m; Cache c[2]; }; diff --git a/tcmalloc/transfer_cache_internals.h b/tcmalloc/transfer_cache_internals.h index a414fe3d6..99acad93a 100644 --- a/tcmalloc/transfer_cache_internals.h +++ b/tcmalloc/transfer_cache_internals.h @@ -85,15 +85,20 @@ class TransferCache { using Manager = TransferCacheManager; using FreeList = CentralFreeList; - TransferCache(Manager *owner, int size_class) - : TransferCache(owner, size_class, CapacityNeeded(size_class)) {} + TransferCache(Manager* owner, int size_class, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization) + : TransferCache(owner, size_class, CapacityNeeded(size_class), + cfl_subbucket_prioritization) {} struct Capacity { int capacity; int max_capacity; }; - TransferCache(Manager *owner, int size_class, Capacity capacity) + TransferCache(Manager* owner, int size_class, Capacity capacity, + central_freelist_internal::CflSubbucketPrioritization + cfl_subbucket_prioritization) : lock_(absl::base_internal::SCHEDULE_KERNEL_ONLY), low_water_mark_(0), slot_info_(SizeInfo({0, capacity.capacity})), @@ -101,7 +106,7 @@ class TransferCache { freelist_do_not_access_directly_(), owner_(owner), max_capacity_(capacity.max_capacity) { - freelist().Init(size_class); + freelist().Init(size_class, cfl_subbucket_prioritization); slots_ = max_capacity_ != 0 ? reinterpret_cast(owner_->Alloc( max_capacity_ * sizeof(void*))) : nullptr; diff --git a/tcmalloc/variants.bzl b/tcmalloc/variants.bzl index a46d71125..06e214e2d 100644 --- a/tcmalloc/variants.bzl +++ b/tcmalloc/variants.bzl @@ -295,6 +295,12 @@ test_variants = [ "deps": ["//tcmalloc:common_8k_pages"], "env": {"BORG_EXPERIMENTS": "TCMALLOC_SONIC_MADVISE_SAMPLED_ALLOCATIONS_HOLDBACK"}, }, + { + "name": "tcmalloc_cfl_subbucket_prioritization", + "malloc": "//tcmalloc", + "deps": ["//tcmalloc:common_8k_pages"], + "env": {"BORG_EXPERIMENTS": "TEST_ONLY_TCMALLOC_CFL_SUBBUCKET_PRIORITIZATION"}, + }, ] def create_tcmalloc_library(