From f176b22e6f2e09bba06d65fa287370bc07cc1c87 Mon Sep 17 00:00:00 2001 From: Chris Kennelly Date: Thu, 9 Jul 2026 10:47:55 -0700 Subject: [PATCH] Remove Span::Fragmentation. We no longer generate fragmentation profiles, and the possibility of concurrent access to the Span outside of locks necessitates making `allocated_` an atomic variable. PiperOrigin-RevId: 945203205 --- tcmalloc/central_freelist_test.cc | 41 ------------------------------- tcmalloc/span.cc | 24 ------------------ tcmalloc/span.h | 4 --- tcmalloc/span_test.cc | 10 -------- 4 files changed, 79 deletions(-) diff --git a/tcmalloc/central_freelist_test.cc b/tcmalloc/central_freelist_test.cc index c01738475..0411820be 100644 --- a/tcmalloc/central_freelist_test.cc +++ b/tcmalloc/central_freelist_test.cc @@ -1130,47 +1130,6 @@ TEST_P(CentralFreeListTest, PassSpanDensityToPageheap) { test_function(e.objects_per_span(), AccessDensityPrediction::kDense); } -TEST_P(CentralFreeListTest, SpanFragmentation) { -#if ABSL_HAVE_HWADDRESS_SANITIZER - GTEST_SKIP() - << "Skipping under HWASan, which uses the top bits of the pointer."; -#endif - - // This test is primarily exercising Span itself to model how tcmalloc.cc uses - // it, but this gives us a self-contained (and sanitizable) implementation of - // the CentralFreeList. - TypeParam e(GetParam().size, GetParam().bytes, GetParam().num_to_move); - // Allocate one object from the CFL to allocate a span. - void* initial; - int got = e.central_freelist().RemoveRange(absl::MakeSpan(&initial, 1)); - ASSERT_EQ(got, 1); - - Span* const span = e.central_freelist().forwarder().MapObjectToSpan(initial); - const size_t object_size = - e.central_freelist().forwarder().class_to_size(TypeParam::kSizeClass); - - ThreadManager fragmentation; - fragmentation.Start(1, [&](int) { - if (e.objects_per_span() != 1) { - benchmark::DoNotOptimize(span->Fragmentation(object_size)); - } - }); - - ThreadManager cfl; - cfl.Start(1, [&](int) { - void* next; - int got = e.central_freelist().RemoveRange(absl::MakeSpan(&next, 1)); - e.central_freelist().InsertRange(absl::MakeSpan(&next, got)); - }); - - absl::SleepFor(absl::Milliseconds(50)); - - fragmentation.Stop(); - cfl.Stop(); - - e.central_freelist().InsertRange(absl::MakeSpan(&initial, 1)); -} - TEST_P(CentralFreeListTest, SpanLifetimeWithLongLivedSpans) { #if ABSL_HAVE_HWADDRESS_SANITIZER GTEST_SKIP() diff --git a/tcmalloc/span.cc b/tcmalloc/span.cc index 943c2da95..a8e2881cf 100644 --- a/tcmalloc/span.cc +++ b/tcmalloc/span.cc @@ -79,30 +79,6 @@ SampledAllocation* Span::UnsampleSlow() { return sampled_allocation; } -double Span::Fragmentation(size_t object_size) const { - if (object_size == 0) { - // Avoid crashes in production mode code, but report in tests. - TC_ASSERT_NE(object_size, 0); - return 0; - } - const size_t span_objects = bytes_in_span() / object_size; - const size_t live = allocated_.load(std::memory_order_relaxed); - if (live == 0) { - // Avoid crashes in production mode code, but report in tests. - TC_ASSERT_NE(live, 0); - return 0; - } - // Assume that all in-use objects in this span are spread evenly - // through this span. So charge the free space in span evenly - // to each of the live objects. - // A note on units here: StackTraceTable::AddTrace(1, *t) - // represents usage (of whatever kind: heap space, allocation, - // fragmentation) of 1 object of size t->allocated_size. - // So we want to report here the number of objects we are "responsible" - // for pinning - NOT bytes. - return static_cast(span_objects - live) / live; -} - // Freelist organization. // // Partially full spans in CentralFreeList contain a list of free objects diff --git a/tcmalloc/span.h b/tcmalloc/span.h index 147417439..a7a90998f 100644 --- a/tcmalloc/span.h +++ b/tcmalloc/span.h @@ -183,10 +183,6 @@ class ABSL_CACHELINE_ALIGNED Span final : public SpanList::Elem { // Total memory bytes in the span. size_t bytes_in_span() const; - // Returns internal fragmentation of the span. - // REQUIRES: this is a SMALL_OBJECT span. - double Fragmentation(size_t object_size) const; - // Returns number of objects allocated in the span. uint16_t Allocated() const { return allocated_.load(std::memory_order_relaxed); diff --git a/tcmalloc/span_test.cc b/tcmalloc/span_test.cc index 86514906a..22e84e1ac 100644 --- a/tcmalloc/span_test.cc +++ b/tcmalloc/span_test.cc @@ -129,11 +129,6 @@ TEST_P(SpanTest, FreelistBasic) { for (;;) { size_t n = span_.FreelistPopBatch(absl::MakeSpan(batch, want), size_); popped += n; - EXPECT_NEAR( - span_.Fragmentation(size_), - static_cast(objects_per_span_) / static_cast(popped) - - 1., - 1e-5); EXPECT_EQ(span_.FreelistEmpty(size_, objects_per_span_), popped == objects_per_span_); for (size_t i = 0; i < n; ++i) { @@ -193,11 +188,6 @@ TEST_P(SpanTest, FreelistBasicObjIdx) { for (;;) { size_t n = span_.FreelistPopBatch(absl::MakeSpan(batch, want), size_); popped += n; - EXPECT_NEAR( - span_.Fragmentation(size_), - static_cast(objects_per_span_) / static_cast(popped) - - 1., - 1e-5); EXPECT_EQ(span_.FreelistEmpty(size_, objects_per_span_), popped == objects_per_span_); for (size_t i = 0; i < n; ++i) {