Skip to content

Remove the num_same_spans_ span-return histogram from CentralFreeList. #720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
+0 −167
Draft

Remove the num_same_spans_ span-return histogram from CentralFreeList. #720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 0 additions & 56 deletions tcmalloc/central_freelist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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_;
Expand Down Expand Up @@ -614,7 +594,6 @@ inline void CentralFreeList<Forwarder>::InsertRange(absl::Span<void*> 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.
Expand All @@ -636,7 +615,6 @@ inline void CentralFreeList<Forwarder>::InsertRange(absl::Span<void*> batch) {
}
const size_t step = j - i;
const absl::Span<Span::ObjIdx> b{&idx[i], step};
++runs;
#endif

Span* span = ReleaseToSpans(b, spans[i], object_size, size_reciprocal,
Expand All @@ -648,12 +626,6 @@ inline void CentralFreeList<Forwarder>::InsertRange(absl::Span<void*> 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<unsigned int>(same_span))]
.LossyAdd(1);
#endif

RecordMultiSpansDeallocated(free_count);
UpdateObjectCounts(batch.size());
Expand Down Expand Up @@ -855,35 +827,7 @@ inline size_t CentralFreeList<Forwarder>::NumSpansWith(
return objects_to_spans_[bucket].value();
}

template <class Forwarder>
inline void CentralFreeList<Forwarder>::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 <class Forwarder>
inline void CentralFreeList<Forwarder>::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 <class Forwarder>
inline void CentralFreeList<Forwarder>::PrintSpanUtilStats(Printer& out) {
Expand Down
103 changes: 0 additions & 103 deletions tcmalloc/central_freelist_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,16 +378,6 @@ class CentralFreeListTestPeer {
template <typename Forwarder>
using CFL = CentralFreeList<Forwarder>;

template <typename Forwarder>
static size_t num_same_spans(const CentralFreeList<Forwarder>& 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<StaticForwarder>;
Expand Down Expand Up @@ -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<void*> 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
Expand Down Expand Up @@ -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<Span*> 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<unsigned int>(
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<unsigned int>(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
Expand Down
8 changes: 0 additions & 8 deletions tcmalloc/global_stats.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down
Loading