From 9289b191eb8749b5c3299fdae2dfd92c409f058c Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Fri, 11 Sep 2026 13:28:57 -0700 Subject: [PATCH] Remove the num_same_spans_ span-return histogram from CentralFreeList. This telemetry was added under TODO(b/527641380) to characterize how many objects in a returned batch land on the same span, guiding CentralFreeList optimization work. It cost a per-size-class StatsCounter array plus per-batch bookkeeping in the InsertRange hot path (a run counter and a LossyAdd), and exposed the counts through text and pbtxt stats printers. The optimization investigation this instrumentation supported is complete, so delete the histogram, its supporting constant, the InsertRange accounting, the PrintSameSpanStats/PrintSameSpanStatsInPbtxt printers, their callers in global_stats, and the associated tests. PiperOrigin-RevId: 979988245 --- tcmalloc/central_freelist.h | 56 ---------------- tcmalloc/central_freelist_test.cc | 103 ------------------------------ tcmalloc/global_stats.cc | 8 --- 3 files changed, 167 deletions(-) diff --git a/tcmalloc/central_freelist.h b/tcmalloc/central_freelist.h index 03f6ea0ca..06ddc53a8 100644 --- a/tcmalloc/central_freelist.h +++ b/tcmalloc/central_freelist.h @@ -145,13 +145,6 @@ class CentralFreeList { public: using Forwarder = ForwarderT; - static constexpr size_t kSameSpanBucketCapacity = - absl::bit_width(kMaxObjectsToMove); - // num_same_spans_ is indexed by absl::bit_width(same_span) for same_span in - // [0, kMaxObjectsToMove - 1], which fits only when kMaxObjectsToMove is a - // power of two. - static_assert(absl::has_single_bit(kMaxObjectsToMove)); - constexpr CentralFreeList() : lock_(absl::base_internal::SCHEDULE_KERNEL_ONLY), size_class_(0), @@ -204,9 +197,7 @@ class CentralFreeList { void PrintSpanUtilStats(Printer& out); void PrintSpanLifetimeStats(Printer& out); void PrintNumSpansUsed(Printer& out); - void PrintSameSpanStats(Printer& out); void PrintSpanUtilStatsInPbtxt(PbtxtRegion& region); - void PrintSameSpanStatsInPbtxt(PbtxtRegion& region); void PrintSpanLifetimeStatsInPbtxt(PbtxtRegion& region); void PrintNumSpansUsedInPbtxt(PbtxtRegion& region); @@ -351,17 +342,6 @@ class CentralFreeList { // so writes are performed using LossyAdd for speed, the lock still // guarantees accuracy. -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - // Records histogram of how many consecutive objects fell on the same span for - // batches. - // - // Index in this array corresponds to absl::bit_width(same_span), yielding - // 8 buckets total because same_span has range [0, 127] (assuming - // kMaxObjectsToMove is 128). - // - // TODO(b/527641380): Delete this after wrapping up optimizations. - StatsCounter num_same_spans_[kSameSpanBucketCapacity]; -#endif // TCMALLOC_INTERNAL_LEGACY_LOCKING // Num free objects in cache entry StatsCounter counter_; @@ -614,7 +594,6 @@ inline void CentralFreeList::InsertRange(absl::Span batch) { idx[i] = spans[i]->PtrToIdx(batch[i], object_size); } } - int runs = 0; #endif // !TCMALLOC_INTERNAL_LEGACY_LOCKING // Safe to store free spans into freed up space in span array. @@ -636,7 +615,6 @@ inline void CentralFreeList::InsertRange(absl::Span batch) { } const size_t step = j - i; const absl::Span b{&idx[i], step}; - ++runs; #endif Span* span = ReleaseToSpans(b, spans[i], object_size, size_reciprocal, @@ -648,12 +626,6 @@ inline void CentralFreeList::InsertRange(absl::Span batch) { i += step; } -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const int same_span = batch.size() - runs; - TC_ASSERT_GE(same_span, 0); - num_same_spans_[absl::bit_width(static_cast(same_span))] - .LossyAdd(1); -#endif RecordMultiSpansDeallocated(free_count); UpdateObjectCounts(batch.size()); @@ -855,35 +827,7 @@ inline size_t CentralFreeList::NumSpansWith( return objects_to_spans_[bucket].value(); } -template -inline void CentralFreeList::PrintSameSpanStats(Printer& out) { -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - out.printf("class %3d [ %8zu bytes ] :", size_class_, object_size_); - for (int i = 0; i < kSameSpanBucketCapacity; ++i) { - out.printf(" %6zu", num_same_spans_[i].value()); - } - out.printf("\n"); -#endif // TCMALLOC_INTERNAL_LEGACY_LOCKING -} -template -inline void CentralFreeList::PrintSameSpanStatsInPbtxt( - PbtxtRegion& region) { -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - for (int i = 0; i < kSameSpanBucketCapacity; ++i) { - auto value = num_same_spans_[i].value(); - if (value == 0) { - continue; - } - PbtxtRegion histogram = region.CreateSubRegion("same_span_stats"); - int lower_bound = i == 0 ? 0 : (1 << (i - 1)); - int upper_bound = i == 0 ? 0 : ((1 << i) - 1); - histogram.PrintI64("lower_bound", lower_bound); - histogram.PrintI64("upper_bound", upper_bound); - histogram.PrintI64("value", value); - } -#endif // TCMALLOC_INTERNAL_LEGACY_LOCKING -} template inline void CentralFreeList::PrintSpanUtilStats(Printer& out) { diff --git a/tcmalloc/central_freelist_test.cc b/tcmalloc/central_freelist_test.cc index b01516860..2ddd4bfb7 100644 --- a/tcmalloc/central_freelist_test.cc +++ b/tcmalloc/central_freelist_test.cc @@ -378,16 +378,6 @@ class CentralFreeListTestPeer { template using CFL = CentralFreeList; - template - static size_t num_same_spans(const CentralFreeList& cfl, - size_t index) { -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - return cfl.num_same_spans_[absl::bit_width(index)].value(); -#else - return 0; -#endif - } - static void VerifyLegacyLayout() { #ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING using CFLType = CFL; @@ -495,37 +485,6 @@ TEST_P(CentralFreeListTest, IsolatedSmoke) { } } -TEST_P(CentralFreeListTest, SameSpanTracking) { -#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())); - if (e.objects_per_span() <= 1) { - GTEST_SKIP() << "Single-object spans skip CentralFreeList InsertRange"; - } - - EXPECT_CALL(e.forwarder(), AllocateSpan).Times(1); - - absl::FixedArray batch(e.batch_size()); - int allocated = e.central_freelist().RemoveRange( - absl::MakeSpan(&batch[0], e.batch_size())); - ASSERT_GT(allocated, 0); - - EXPECT_CALL(e.forwarder(), MapObjectsToSpans).Times(1); - EXPECT_CALL(e.forwarder(), DeallocateSpans).Times(testing::AtLeast(0)); - - e.central_freelist().InsertRange(absl::MakeSpan(&batch[0], allocated)); - -#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING - const int expected_same_span = allocated - 1; - EXPECT_GE(central_freelist_internal::CentralFreeListTestPeer::num_same_spans( - e.central_freelist(), expected_same_span), - 1); -#endif -} TEST_P(CentralFreeListTest, SpanUtilizationHistogram) { #if ABSL_HAVE_HWADDRESS_SANITIZER @@ -1237,68 +1196,6 @@ TEST_P(CentralFreeListTest, SpanAllocationTracker) { single_spans)))); } -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 = 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]; - const int got = - e.central_freelist().RemoveRange(absl::MakeSpan(batch, num_to_move)); - ASSERT_GT(got, 0); - - Span* spans[kMaxObjectsToMove]; - e.forwarder().MapObjectsToSpans(absl::MakeSpan(batch, got), spans, - e.kSizeClass); - absl::flat_hash_set pseudo_spans; - for (int i = 0; i < got; ++i) { - pseudo_spans.insert(spans[i]); - } - - e.central_freelist().InsertRange(absl::MakeSpan(batch, got)); - - // Check the stats after the first insertion. - { - 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( - got - pseudo_spans.size())); - const int count = first_batch ? 1 : 0; - absl::StrAppendFormat(&expected_stats, " %6d", count); - } - absl::StrAppend(&expected_stats, "\n"); - - std::string buffer = PrintToString(1024 * 1024, [&](Printer& printer) { - e.central_freelist().PrintSameSpanStats(printer); - }); - EXPECT_EQ(buffer, expected_stats) << got; - } - { - std::string expected_pbtxt = ""; - if (e.objects_per_span() > 1) { - int same_span_val = got - pseudo_spans.size(); - int bucket = absl::bit_width(static_cast(same_span_val)); - int lower_bound = bucket == 0 ? 0 : (1 << (bucket - 1)); - int upper_bound = bucket == 0 ? 0 : ((1 << bucket) - 1); - expected_pbtxt = absl::StrFormat( - " same_span_stats { lower_bound: %d upper_bound: %d value: 1}", - lower_bound, upper_bound); - } - - std::string buffer_pbtxt = - PrintToString(1024 * 1024, [&](PbtxtRegion& region) { - e.central_freelist().PrintSameSpanStatsInPbtxt(region); - }); - EXPECT_EQ(buffer_pbtxt, expected_pbtxt) << got; - } -} TEST_P(CentralFreeListTest, MultipleSpans) { #if ABSL_HAVE_HWADDRESS_SANITIZER diff --git a/tcmalloc/global_stats.cc b/tcmalloc/global_stats.cc index d065aa0d5..32eb95608 100644 --- a/tcmalloc/global_stats.cc +++ b/tcmalloc/global_stats.cc @@ -529,12 +529,6 @@ void DumpStats(Printer& out, int level) { tc_globals.central_freelist(size_class).PrintNumSpansUsed(out); } - out.printf("------------------------------------------------\n"); - out.printf("Central cache freelist: Same-span returns\n"); - out.printf("------------------------------------------------\n"); - for (int size_class = 1; size_class < kNumClasses; ++size_class) { - tc_globals.central_freelist(size_class).PrintSameSpanStats(out); - } tc_globals.transfer_cache().Print(tc_globals.per_size_class_counts(), out); tc_globals.sharded_transfer_cache().Print( @@ -823,8 +817,6 @@ void DumpStatsInPbtxt(Printer& out, int level) { tc_globals.central_freelist(size_class) .PrintSpanLifetimeStatsInPbtxt(entry); tc_globals.central_freelist(size_class).PrintNumSpansUsedInPbtxt(entry); - - tc_globals.central_freelist(size_class).PrintSameSpanStatsInPbtxt(entry); } tc_globals.transfer_cache().PrintInPbtxt(tc_globals.per_size_class_counts(),