From f78ae869163eec33792c2308a3195bc824dfdeb1 Mon Sep 17 00:00:00 2001 From: Chris Kennelly Date: Sun, 23 Aug 2026 10:03:09 -0700 Subject: [PATCH] Add defaulted-off option to gate madvise'ing sampled allocations. Due to allocation reuse, we may trend towards the entire allocation being resident over time, even if it is not used. Consider: * Allocate 256KB. Write to it in its entirety. Free it. * Allocate 256KB again. Write to the first 128KB. Our residency telemetry will show the second allocation as fully resident, and many of those bytes (the upper half) as inactive. Our resident and zero telemetry is better equipped if it can answer accurately about the live allocation being profiled rather than the history of the address space. PiperOrigin-RevId: 969428495 --- tcmalloc/allocation_sampling.h | 45 ++++++- tcmalloc/global_stats.cc | 6 + tcmalloc/internal/parameter_accessors.h | 7 ++ tcmalloc/parameters.cc | 13 ++ tcmalloc/parameters.h | 11 ++ tcmalloc/testing/BUILD | 5 + tcmalloc/testing/get_stats_test.cc | 12 ++ tcmalloc/testing/heap_profiling_test.cc | 158 +++++++++++++++++++++++- tcmalloc/testing/testutil.h | 17 +++ 9 files changed, 268 insertions(+), 6 deletions(-) diff --git a/tcmalloc/allocation_sampling.h b/tcmalloc/allocation_sampling.h index 7db362dcc..8e123f79b 100644 --- a/tcmalloc/allocation_sampling.h +++ b/tcmalloc/allocation_sampling.h @@ -25,6 +25,8 @@ #include "tcmalloc/internal/config.h" #include "tcmalloc/internal/logging.h" #include "tcmalloc/internal/percpu.h" +#include "tcmalloc/internal/residency.h" +#include "tcmalloc/internal/util.h" #include "tcmalloc/malloc_extension.h" #include "tcmalloc/malloc_hook.h" #include "tcmalloc/malloc_hook_invoke.h" @@ -170,12 +172,48 @@ ABSL_ATTRIBUTE_NOINLINE sized_ptr_t SampleifyAllocation( // A span must be provided or created by this point. TC_ASSERT_NE(span, nullptr); + // Do not madvise guarded (GWP-ASan) allocations: GWP-ASan initializes magic + // canary bytes in the allocated page to detect buffer overflows on + // deallocation; releasing memory zeroes the page and corrupts the canaries. + if (Parameters::madvise_sampled_allocations() == + MadviseSampledAllocations::kEnabled && + alloc_with_status.status != Profile::Sample::GuardedStatus::Guarded) { + switch (GetMemoryTag(span->start_address())) { + case MemoryTag::kSampled: + case MemoryTag::kSampledP1: + case MemoryTag::kCold: { + // TODO(b/540945006): Reconsider whether to skip the first page. + const uintptr_t hardware_page_size = GetPageSize(); + uintptr_t start = reinterpret_cast(span->start_address()); + uintptr_t length = span->bytes_in_span(); + if (length <= hardware_page_size) { + break; + } + start += hardware_page_size; + length -= hardware_page_size; + + (void)state.system_allocator().Release(reinterpret_cast(start), + length); + break; + } + case MemoryTag::kNormal: + case MemoryTag::kNormalP1: + case MemoryTag::kMetadata: + break; + } + } + // TODO(b/414876446): Add entropy to the handles generated. stack_trace.sampled_alloc_handle = AllocHandle(state.sampled_alloc_handle_generator.fetch_add( 1, std::memory_order_relaxed) + 1); - stack_trace.span_start_address = span->start_address(); + // For guarded allocations under large page sizes, span->start_address() + // rounds down to a PROT_NONE guard page; record the object address instead + // so residency queries (e.g. mincore) inspect the accessible page. + stack_trace.span_start_address = (alloc_with_status.alloc != nullptr) + ? alloc_with_status.alloc + : span->start_address(); stack_trace.allocation_time = absl::Now(); stack_trace.guarded_status = alloc_with_status.status; stack_trace.allocation_type = policy.allocation_type(); @@ -195,9 +233,7 @@ ABSL_ATTRIBUTE_NOINLINE sized_ptr_t SampleifyAllocation( .weight = allocation_estimate, .stack = absl::MakeSpan(stack_trace.stack, stack_trace.depth), .allocation_time = stack_trace.allocation_time, - .ptr = (alloc_with_status.alloc != nullptr) - ? alloc_with_status.alloc - : stack_trace.span_start_address, + .ptr = stack_trace.span_start_address, .access_hint = stack_trace.access_hint, .access_allocated = stack_trace.cold_allocated ? MallocHook::Access::Cold : MallocHook::Access::Hot, @@ -213,6 +249,7 @@ ABSL_ATTRIBUTE_NOINLINE sized_ptr_t SampleifyAllocation( // heap profile, and won't need any information from Span::Sample() next. SampledAllocation* sampled_allocation = state.sampled_allocation_recorder().Register(std::move(stack_trace)); + // No pageheap_lock required. The span is freshly allocated and no one else // can access it. It is visible after we return from this allocation path. span->Sample(sampled_allocation); diff --git a/tcmalloc/global_stats.cc b/tcmalloc/global_stats.cc index d7e4e63cb..f15e543c5 100644 --- a/tcmalloc/global_stats.cc +++ b/tcmalloc/global_stats.cc @@ -637,6 +637,9 @@ void DumpStats(Printer& out, int level) { MadviseRegionsNoHugepage::kEnabled ? 1 : 0); + out.printf("PARAMETER tcmalloc_madvise_sampled_allocations %d\n", + Parameters::madvise_sampled_allocations() == + MadviseSampledAllocations::kEnabled); out.printf("PARAMETER tcmalloc_use_wider_slabs %d\n", tc_globals.cpu_cache().UseWiderSlabs() ? 1 : 0); out.printf("PARAMETER heap_partitioning %d\n", @@ -923,6 +926,9 @@ void DumpStatsInPbtxt(Printer& out, int level) { region.PrintBool("subrelease_unbacked_hugepages", Parameters::subrelease_unbacked_hugepages() == SubreleaseUnbackedMode::kEnabled); + region.PrintBool("tcmalloc_madvise_sampled_allocations", + Parameters::madvise_sampled_allocations() == + MadviseSampledAllocations::kEnabled); region.PrintBool("back_small_allocations", Parameters::back_small_allocations()); diff --git a/tcmalloc/internal/parameter_accessors.h b/tcmalloc/internal/parameter_accessors.h index 39cc50cf6..073e9813d 100644 --- a/tcmalloc/internal/parameter_accessors.h +++ b/tcmalloc/internal/parameter_accessors.h @@ -35,6 +35,8 @@ struct TracerSizeClassInfo { size_t num_objects_to_move; }; +enum class MadviseSampledAllocations : bool { kDisabled, kEnabled }; + } // namespace tcmalloc_internal } // namespace tcmalloc @@ -92,6 +94,11 @@ TCMalloc_Internal_SetHugePageFillerSkipSubreleaseLongInterval(absl::Duration v); ABSL_ATTRIBUTE_WEAK bool TCMalloc_Internal_GetMadviseColdRegionsNoHugepage(); ABSL_ATTRIBUTE_WEAK void TCMalloc_Internal_SetMadviseColdRegionsNoHugepage( bool v); +[[nodiscard]] ABSL_ATTRIBUTE_WEAK + tcmalloc::tcmalloc_internal::MadviseSampledAllocations + TCMalloc_Internal_GetMadviseSampledAllocations(); +ABSL_ATTRIBUTE_WEAK void TCMalloc_Internal_SetMadviseSampledAllocations( + tcmalloc::tcmalloc_internal::MadviseSampledAllocations v); ABSL_ATTRIBUTE_WEAK int64_t TCMalloc_Internal_GetEventTraceMemoryLimit(); ABSL_ATTRIBUTE_WEAK void TCMalloc_Internal_SetEventTraceMemoryLimit(int64_t v); ABSL_ATTRIBUTE_WEAK uint8_t TCMalloc_Internal_GetMinHotAccessHint(); diff --git a/tcmalloc/parameters.cc b/tcmalloc/parameters.cc index 8d6e0edf0..ce51208c7 100644 --- a/tcmalloc/parameters.cc +++ b/tcmalloc/parameters.cc @@ -231,6 +231,9 @@ ABSL_CONST_INIT std::atomic Parameters::back_size_threshold_bytes_( ABSL_CONST_INIT std::atomic Parameters::enable_unfiltered_collapse_( false); ABSL_CONST_INIT std::atomic Parameters::release_max_cold_pages_(false); +ABSL_CONST_INIT std::atomic + Parameters::madvise_sampled_allocations_( + MadviseSampledAllocations::kDisabled); ABSL_CONST_INIT std::atomic Parameters::event_trace_memory_limit_( 16 << 20); ABSL_CONST_INIT @@ -368,6 +371,7 @@ static bool want_disable_dynamic_slabs() { } // namespace tcmalloc_internal } // namespace tcmalloc +using tcmalloc::tcmalloc_internal::MadviseSampledAllocations; using tcmalloc::tcmalloc_internal::Parameters; using tcmalloc::tcmalloc_internal::tc_globals; @@ -663,6 +667,15 @@ void TCMalloc_Internal_SetMadviseColdRegionsNoHugepage(bool v) { std::memory_order_relaxed); } +MadviseSampledAllocations TCMalloc_Internal_GetMadviseSampledAllocations() { + return Parameters::madvise_sampled_allocations(); +} + +void TCMalloc_Internal_SetMadviseSampledAllocations( + MadviseSampledAllocations v) { + Parameters::madvise_sampled_allocations_.store(v, std::memory_order_relaxed); +} + int64_t TCMalloc_Internal_GetEventTraceMemoryLimit() { return Parameters::event_trace_memory_limit(); } diff --git a/tcmalloc/parameters.h b/tcmalloc/parameters.h index 91b8cccf8..9613d444e 100644 --- a/tcmalloc/parameters.h +++ b/tcmalloc/parameters.h @@ -148,6 +148,14 @@ class Parameters { TCMalloc_Internal_SetMadviseColdRegionsNoHugepage(value); } + [[nodiscard]] static MadviseSampledAllocations madvise_sampled_allocations() { + return madvise_sampled_allocations_.load(std::memory_order_relaxed); + } + + static void set_madvise_sampled_allocations(MadviseSampledAllocations value) { + TCMalloc_Internal_SetMadviseSampledAllocations(value); + } + static int64_t event_trace_memory_limit() { return event_trace_memory_limit_.load(std::memory_order_relaxed); } @@ -256,6 +264,8 @@ class Parameters { friend void ::TCMalloc_Internal_SetEnableUnfilteredCollapse(bool v); friend void ::TCMalloc_Internal_SetHugeRegionAdaptiveReleaseEnabled(bool v); friend void ::TCMalloc_Internal_SetReleaseMaxColdPages(bool v); + friend void ::TCMalloc_Internal_SetMadviseSampledAllocations( + tcmalloc::tcmalloc_internal::MadviseSampledAllocations v); friend void ::TCMalloc_Internal_SetEventTraceMemoryLimit(int64_t v); friend void ::TCMalloc_Internal_SetReleaseDrainedSlabMetadata(bool v); @@ -278,6 +288,7 @@ class Parameters { static std::atomic back_size_threshold_bytes_; static std::atomic enable_unfiltered_collapse_; static std::atomic release_max_cold_pages_; + static std::atomic madvise_sampled_allocations_; static std::atomic event_trace_memory_limit_; static std::atomic release_drained_slab_metadata_; }; diff --git a/tcmalloc/testing/BUILD b/tcmalloc/testing/BUILD index bef267f6a..0cdacba6d 100644 --- a/tcmalloc/testing/BUILD +++ b/tcmalloc/testing/BUILD @@ -52,6 +52,7 @@ cc_library( deps = [ "//tcmalloc:malloc_extension", "//tcmalloc/internal:logging", + "//tcmalloc/internal:parameter_accessors", "//tcmalloc/internal:percpu", "@com_github_google_benchmark//:benchmark", "@com_google_absl//absl/base:core_headers", @@ -1032,13 +1033,17 @@ create_tcmalloc_testsuite( copts = TCMALLOC_DEFAULT_COPTS, deps = [ ":test_allocator_harness", + ":testutil", ":thread_manager", "//tcmalloc:malloc_extension", "//tcmalloc:malloc_hook", "//tcmalloc/internal:config", "//tcmalloc/internal:logging", + "//tcmalloc/internal:memory_tag", + "//tcmalloc/internal:page_size", "//tcmalloc/internal:profile_builder", "//tcmalloc/internal:profile_cc_proto", + "//tcmalloc/internal:residency", "//tcmalloc/internal:sampled_allocation", "@com_google_absl//absl/base", "@com_google_absl//absl/base:core_headers", diff --git a/tcmalloc/testing/get_stats_test.cc b/tcmalloc/testing/get_stats_test.cc index 58ad6d0e0..6563e8203 100644 --- a/tcmalloc/testing/get_stats_test.cc +++ b/tcmalloc/testing/get_stats_test.cc @@ -42,6 +42,7 @@ namespace tcmalloc { namespace { +using tcmalloc_internal::MadviseSampledAllocations; using tcmalloc_internal::Parameters; using ::testing::AnyOf; using ::testing::ContainsRegex; @@ -188,6 +189,7 @@ TEST_F(GetStatsTest, Parameters) { const absl::Duration old_skip_subrelease_long = Parameters::filler_skip_subrelease_long_interval(); Parameters::set_filler_skip_subrelease_long_interval(absl::Seconds(3)); + ScopedMadviseSampledAllocations s(MadviseSampledAllocations::kDisabled); auto using_hpaa = [](absl::string_view sv) { return absl::StrContains(sv, "HugePageAwareAllocator"); @@ -260,11 +262,15 @@ TEST_F(GetStatsTest, Parameters) { EXPECT_THAT(buf, HasSubstr(R"(PARAMETER madvise_cold_regions_nohugepage 0)")); } + EXPECT_THAT( + buf, HasSubstr(R"(PARAMETER tcmalloc_madvise_sampled_allocations 0)")); if (using_hpaa(buf)) { EXPECT_THAT(buf, HasSubstr(R"(using_hpaa_subrelease: false)")); } EXPECT_THAT(pbtxt, HasSubstr(R"(guarded_sample_parameter: -1)")); + EXPECT_THAT(pbtxt, + HasSubstr(R"(tcmalloc_madvise_sampled_allocations: false)")); #ifdef TCMALLOC_DEPRECATED_PERTHREAD EXPECT_THAT(pbtxt, HasSubstr(R"(tcmalloc_per_cpu_caches: false)")); #endif // TCMALLOC_DEPRECATED_PERTHREAD @@ -309,6 +315,8 @@ TEST_F(GetStatsTest, Parameters) { Parameters::set_filler_skip_subrelease_long_interval( absl::Milliseconds(180375)); Parameters::set_min_hot_access_hint(hot_cold_t{3}); + Parameters::set_madvise_sampled_allocations( + MadviseSampledAllocations::kEnabled); buf = MallocExtension::GetStats(); pbtxt = GetStatsInPbTxt(); @@ -337,6 +345,8 @@ TEST_F(GetStatsTest, Parameters) { buf, HasSubstr( R"(PARAMETER tcmalloc_skip_subrelease_long_interval 3m0.375s)")); + EXPECT_THAT( + buf, HasSubstr(R"(PARAMETER tcmalloc_madvise_sampled_allocations 1)")); if (using_hpaa(buf)) { EXPECT_THAT(pbtxt, HasSubstr(R"(using_hpaa_subrelease: true)")); @@ -358,6 +368,8 @@ TEST_F(GetStatsTest, Parameters) { HasSubstr( R"(tcmalloc_skip_subrelease_long_interval_ns: 180375000000)")); EXPECT_THAT(pbtxt, HasSubstr(R"(min_hot_access_hint: 3)")); + EXPECT_THAT(pbtxt, + HasSubstr(R"(tcmalloc_madvise_sampled_allocations: true)")); } Parameters::set_hpaa_subrelease(old_hpaa_subrelease); diff --git a/tcmalloc/testing/heap_profiling_test.cc b/tcmalloc/testing/heap_profiling_test.cc index ebfd84226..8b2f2c8e0 100644 --- a/tcmalloc/testing/heap_profiling_test.cc +++ b/tcmalloc/testing/heap_profiling_test.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -34,16 +35,20 @@ #include "absl/log/check.h" #include "absl/status/statusor.h" #include "absl/strings/str_format.h" -#include "absl/time/clock.h" -#include "absl/time/time.h" +#include "tcmalloc/common.h" #include "tcmalloc/internal/config.h" #include "tcmalloc/internal/logging.h" +#include "tcmalloc/internal/memory_tag.h" +#include "tcmalloc/internal/page_size.h" #include "tcmalloc/internal/profile_builder.h" +#include "tcmalloc/internal/residency.h" #include "tcmalloc/internal/sampled_allocation.h" #include "tcmalloc/malloc_extension.h" #include "tcmalloc/malloc_hook.h" +#include "tcmalloc/parameters.h" #include "tcmalloc/static_vars.h" #include "tcmalloc/testing/test_allocator_harness.h" +#include "tcmalloc/testing/testutil.h" #include "tcmalloc/testing/thread_manager.h" namespace tcmalloc { @@ -249,6 +254,155 @@ TEST(HeapProfilingTest, CheckResidency) { } } +TEST(HeapProfilingTest, MadviseSampledAllocations) { + if (tcmalloc_internal::kSanitizerPresent) { + GTEST_SKIP() << "Sanitizers intercept allocations"; + } + + const ScopedProfileSamplingInterval sample_interval(1); + + const size_t kPageSize = tcmalloc_internal::GetPageSize(); + constexpr int kNumAllocations = 50; + + enum class AllocationHeap { + kSampled, + kCold, + kNormal, + }; + + using tcmalloc_internal::MadviseSampledAllocations; + struct TestCase { + absl::string_view name; + MadviseSampledAllocations madvise_sampled; + AllocationHeap heap; + bool expect_madvised; + bool guarded; + }; + + const TestCase kTestCases[] = { + {"disabled_sampled", MadviseSampledAllocations::kDisabled, + AllocationHeap::kSampled, + /*expect_madvised=*/false, /*guarded=*/false}, + {"enabled_sampled", MadviseSampledAllocations::kEnabled, + AllocationHeap::kSampled, + /*expect_madvised=*/true, /*guarded=*/false}, + {"enabled_guarded", MadviseSampledAllocations::kEnabled, + AllocationHeap::kSampled, + /*expect_madvised=*/true, /*guarded=*/true}, + {"enabled_cold", MadviseSampledAllocations::kEnabled, + AllocationHeap::kCold, + /*expect_madvised=*/true, /*guarded=*/false}, + {"enabled_normal", MadviseSampledAllocations::kEnabled, + AllocationHeap::kNormal, + /*expect_madvised=*/false, /*guarded=*/false}, + }; + + tcmalloc_internal::ResidencyPageMap residency; + + for (const auto& test_case : kTestCases) { + SCOPED_TRACE(test_case.name); + if (test_case.heap == AllocationHeap::kCold && + (!tcmalloc_internal::ColdFeatureActive() || + tcmalloc_internal::Parameters::heap_partitioning_mode() == + tcmalloc_internal::HeapPartitioningMode::kFull)) { + continue; + } + + ScopedMadviseSampledAllocations s(test_case.madvise_sampled); + + const int num_allocations = test_case.guarded ? 1 : kNumAllocations; + const ScopedGuardedSamplingInterval guarded_interval( + test_case.guarded ? 1 : -1); + + const size_t alloc_size = (test_case.heap == AllocationHeap::kNormal || + test_case.heap == AllocationHeap::kCold) + ? tcmalloc_internal::kMaxSize + 2 * kPageSize + : (test_case.guarded ? 32 : 2 * kPageSize); + + auto allocate = [&]() -> void* { + if (test_case.heap == AllocationHeap::kCold) { + return ::operator new(alloc_size, tcmalloc::hot_cold_t{0}); + } + return ::operator new(alloc_size); + }; + + void* allocs[kNumAllocations]; + for (int i = 0; i < num_allocations; ++i) { + allocs[i] = allocate(); + switch (test_case.heap) { + case AllocationHeap::kSampled: + EXPECT_TRUE(tcmalloc_internal::IsSampledMemory(allocs[i])); + break; + case AllocationHeap::kCold: + EXPECT_EQ(tcmalloc_internal::GetMemoryTag(allocs[i]), + tcmalloc_internal::MemoryTag::kCold); + break; + case AllocationHeap::kNormal: + EXPECT_TRUE(tcmalloc_internal::IsNormalMemory(allocs[i])); + break; + } + memset(allocs[i], 0xAB, alloc_size); + } + for (int i = 0; i < num_allocations; ++i) { + sized_delete(allocs[i], alloc_size); + } + + // Reallocate and touch only the first page. + const size_t touch_size = std::min(alloc_size, kPageSize); + for (int i = 0; i < num_allocations; ++i) { + allocs[i] = allocate(); + memset(allocs[i], 0xCD, touch_size); + } + + size_t total_resident = 0; + for (int i = 0; i < num_allocations; ++i) { + auto info = residency.Get(allocs[i], alloc_size); + ASSERT_TRUE(info.has_value()); + total_resident += info->bytes_resident; + } + + if (test_case.expect_madvised) { + EXPECT_LE(total_resident, num_allocations * touch_size); + EXPECT_GE(total_resident, (num_allocations - 1) * touch_size); + } else { + EXPECT_GT(total_resident, num_allocations * touch_size); + } + + auto converted_or = tcmalloc_internal::MakeProfileProto( + MallocExtension::SnapshotCurrent(ProfileType::kHeap)); + ASSERT_TRUE(converted_or.ok()); + const auto& converted = **converted_or; + + std::optional resident_space_id; + for (int i = 0, n = converted.string_table().size(); i < n; ++i) { + if (converted.string_table(i) == "resident_space") { + resident_space_id = i; + break; + } + } + ASSERT_TRUE(resident_space_id.has_value()); + + std::optional resident_value_index; + for (int i = 0; i < converted.sample_type_size(); ++i) { + if (converted.sample_type(i).type() == resident_space_id) { + resident_value_index = i; + break; + } + } + ASSERT_TRUE(resident_value_index.has_value()); + + size_t profile_resident = 0; + for (const auto& sample : converted.sample()) { + profile_resident += sample.value(*resident_value_index); + } + EXPECT_GE(profile_resident, total_resident * 9 / 10); + + for (int i = 0; i < num_allocations; ++i) { + sized_delete(allocs[i], alloc_size); + } + } +} + // Make sure users can allocate when iterating over the heap samples. For now // `MallocExtension::SnapshotCurrent()` uses `StackTraceTable` to make a copy of // the sampled allocations from `tc_globals.sampled_allocation_recorder()` and diff --git a/tcmalloc/testing/testutil.h b/tcmalloc/testing/testutil.h index 860b0a729..9de307090 100644 --- a/tcmalloc/testing/testutil.h +++ b/tcmalloc/testing/testutil.h @@ -32,6 +32,7 @@ #include "absl/time/clock.h" #include "absl/time/time.h" #include "tcmalloc/internal/logging.h" +#include "tcmalloc/internal/parameter_accessors.h" #include "tcmalloc/internal/percpu.h" #include "tcmalloc/malloc_extension.h" @@ -216,6 +217,22 @@ class ScopedAlwaysSample { ScopedProfileSamplingInterval profile_sampling_interval_; }; +class ScopedMadviseSampledAllocations { + public: + explicit ScopedMadviseSampledAllocations( + tcmalloc_internal::MadviseSampledAllocations new_madvise) + : old_madvise_(TCMalloc_Internal_GetMadviseSampledAllocations()) { + TCMalloc_Internal_SetMadviseSampledAllocations(new_madvise); + } + + ~ScopedMadviseSampledAllocations() { + TCMalloc_Internal_SetMadviseSampledAllocations(old_madvise_); + } + + private: + tcmalloc_internal::MadviseSampledAllocations old_madvise_; +}; + inline void UnregisterRseq() { #if TCMALLOC_INTERNAL_PERCPU_USE_RSEQ syscall(__NR_rseq, &tcmalloc_internal::subtle::percpu::__rseq_abi,