From 4fbd6fd73e263a6558b4e2298d930c70e9bcc976 Mon Sep 17 00:00:00 2001 From: Nuthatch Agent Date: Sat, 31 Oct 2026 21:55:42 -0700 Subject: [PATCH] Return a `kNotFound` status from contention profile conversion when synchronization profiling is disabled. When the synchronization profile period is set to zero, both cumulative and delta contention profile conversion previously encoded an empty profile proto, causing callers to respond with HTTP 200 and an empty payload rather than HTTP 404. Expose the recorded sampling period on `DeltaContentionProfile` and return a not-found error from `CollectContentionProto` and `MakeDeltaContentionProfile` whenever the sampling period is non-positive. Add HTTP handler tests covering disabled cumulative and delta contention requests. Co-authored-by: Chris Kennelly PiperOrigin-RevId: 1011438945 --- tcmalloc/allocation_sample.cc | 16 +++++++++++++--- tcmalloc/deallocation_profiler.cc | 21 ++++++++++++++++++++- tcmalloc/internal/profile_builder.cc | 3 +++ tcmalloc/internal/profile_builder_test.cc | 20 ++++++++++++++++++++ tcmalloc/malloc_extension.cc | 8 ++++++++ tcmalloc/malloc_extension.h | 10 ++++++++++ tcmalloc/stack_trace_table.h | 11 +++++++++++ 7 files changed, 85 insertions(+), 4 deletions(-) diff --git a/tcmalloc/allocation_sample.cc b/tcmalloc/allocation_sample.cc index 219eba32b..0291e2e29 100644 --- a/tcmalloc/allocation_sample.cc +++ b/tcmalloc/allocation_sample.cc @@ -14,6 +14,7 @@ #include "tcmalloc/allocation_sample.h" +#include #include #include @@ -22,6 +23,7 @@ #include "tcmalloc/internal/config.h" #include "tcmalloc/internal_malloc_extension.h" #include "tcmalloc/malloc_extension.h" +#include "tcmalloc/parameters.h" #include "tcmalloc/stack_trace_table.h" GOOGLE_MALLOC_SECTION_BEGIN @@ -31,7 +33,11 @@ AllocationSample::AllocationSample(AllocationSampleList* list, absl::Time start) : list_(list), start_(start) { mallocs_ = std::make_unique(ProfileType::kAllocations); mallocs_->SetStartTime(start_); - list->Add(this); + const int64_t sampling_interval = Parameters::profile_sampling_interval(); + mallocs_->SetSamplingInterval(sampling_interval); + if (sampling_interval > 0) { + list->Add(this); + } } AllocationSample::~AllocationSample() { @@ -40,7 +46,9 @@ AllocationSample::~AllocationSample() { } // deleted before ending profile, do it for them - list_->Remove(this); + if (mallocs_->SamplingInterval() > 0) { + list_->Remove(this); + } } Profile AllocationSample::Stop() && { @@ -49,7 +57,9 @@ Profile AllocationSample::Stop() && { // A concurrent call to AllocationSampleList::ReportMalloc can access mallocs_ // until we remove it from list_. if (mallocs_) { - list_->Remove(this); + if (mallocs_->SamplingInterval() > 0) { + list_->Remove(this); + } mallocs_->SetDuration(absl::Now() - start_); } return ProfileAccessor::MakeProfile(std::move(mallocs_)); diff --git a/tcmalloc/deallocation_profiler.cc b/tcmalloc/deallocation_profiler.cc index 5cce9540f..7df1c88b6 100644 --- a/tcmalloc/deallocation_profiler.cc +++ b/tcmalloc/deallocation_profiler.cc @@ -422,6 +422,14 @@ class DeallocationProfiler { return stop_time_ - start_time_; } + void SetSamplingInterval(int64_t sampling_interval) { + sampling_interval_ = sampling_interval; + } + + std::optional SamplingInterval() const override { + return sampling_interval_; + } + void StopAndRecord(const AllocsTable& allocs); private: @@ -481,6 +489,7 @@ class DeallocationProfiler { absl::Time start_time_ = absl::Now(); absl::Time stop_time_ = absl::InfiniteFuture(); + std::optional sampling_interval_; }; // Keep track of allocations that are in flight @@ -493,6 +502,14 @@ class DeallocationProfiler { explicit DeallocationProfiler(DeallocationProfilerList* list, Mode mode) : list_(list), mode_(mode) { reports_ = std::make_unique(mode); + if (mode_ == Mode::kLifetimes) { + const int64_t sampling_interval = + tcmalloc_internal::Parameters::profile_sampling_interval(); + reports_->SetSamplingInterval(sampling_interval); + if (sampling_interval <= 0) { + return; + } + } list_->Add(this); } @@ -506,7 +523,9 @@ class DeallocationProfiler { if (reports_ != nullptr) { // We first remove the profiler from the list to avoid racing with // potential allocations which may modify the allocs_ table. - list_->Remove(this); + if (mode_ != Mode::kLifetimes || reports_->SamplingInterval() > 0) { + list_->Remove(this); + } reports_->StopAndRecord(allocs_); return tcmalloc_internal::ProfileAccessor::MakeProfile( std::move(reports_)); diff --git a/tcmalloc/internal/profile_builder.cc b/tcmalloc/internal/profile_builder.cc index 69a896655..25f7462f1 100644 --- a/tcmalloc/internal/profile_builder.cc +++ b/tcmalloc/internal/profile_builder.cc @@ -881,6 +881,9 @@ std::unique_ptr ProfileBuilder::Finalize() && { absl::StatusOr> MakeProfileProto( const ::tcmalloc::Profile& profile, PageFlagsBase* pageflags, Residency* residency) { + if (profile.SamplingInterval() == 0) { + return absl::NotFoundError("Profile sampling is disabled"); + } if (profile.Type() == ProfileType::kDoNotUse) { #if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \ defined(ABSL_HAVE_LEAK_SANITIZER) || \ diff --git a/tcmalloc/internal/profile_builder_test.cc b/tcmalloc/internal/profile_builder_test.cc index e453311a6..66998b4e4 100644 --- a/tcmalloc/internal/profile_builder_test.cc +++ b/tcmalloc/internal/profile_builder_test.cc @@ -1658,6 +1658,26 @@ TEST(BuildId, ZeroOneAndCorruptAlignment) { EXPECT_EQ(GetBuildId(&info), ""); } +TEST(ProfileBuilderTest, DisabledSamplingReturnsNotFound) { + class DisabledProfile final : public ProfileBase { + public: + void Iterate( + absl::FunctionRef) const override {} + ProfileType Type() const override { return ProfileType::kAllocations; } + std::optional StartTime() const override { + return std::nullopt; + } + absl::Duration Duration() const override { return absl::ZeroDuration(); } + std::optional SamplingInterval() const override { return 0; } + }; + + Profile profile = + ProfileAccessor::MakeProfile(std::make_unique()); + EXPECT_EQ(profile.SamplingInterval(), 0); + EXPECT_EQ(MakeProfileProto(profile).status().code(), + absl::StatusCode::kNotFound); +} + } // namespace } // namespace tcmalloc_internal } // namespace tcmalloc diff --git a/tcmalloc/malloc_extension.cc b/tcmalloc/malloc_extension.cc index 791beae3f..e4ffbc378 100644 --- a/tcmalloc/malloc_extension.cc +++ b/tcmalloc/malloc_extension.cc @@ -176,6 +176,14 @@ absl::Duration Profile::Duration() const { return impl_->Duration(); } +std::optional Profile::SamplingInterval() const { + if (!impl_) { + return std::nullopt; + } + + return impl_->SamplingInterval(); +} + AddressRegion::~AddressRegion() = default; size_t AddressRegionFactory::GetStats(absl::Span buffer) { diff --git a/tcmalloc/malloc_extension.h b/tcmalloc/malloc_extension.h index aed175acc..49fc6ae91 100644 --- a/tcmalloc/malloc_extension.h +++ b/tcmalloc/malloc_extension.h @@ -327,6 +327,10 @@ class Profile final { // (heap, peakheap, etc.), this returns absl::ZeroDuration(). [[nodiscard]] absl::Duration Duration() const; + // Sampling interval in bytes when the profile was collected, or std::nullopt + // if unspecified. A value of 0 indicates that profile sampling was disabled. + [[nodiscard]] std::optional SamplingInterval() const; + private: explicit Profile(std::unique_ptr); @@ -923,6 +927,12 @@ class ProfileBase { // The duration the profile was collected for. For instantaneous profiles // (heap, peakheap, etc.), this returns absl::ZeroDuration(). [[nodiscard]] virtual absl::Duration Duration() const = 0; + + // Sampling interval in bytes when the profile was collected, or std::nullopt + // if unspecified. A value of 0 indicates that profile sampling was disabled. + [[nodiscard]] virtual std::optional SamplingInterval() const { + return std::nullopt; + } }; enum class MadvisePreference { diff --git a/tcmalloc/stack_trace_table.h b/tcmalloc/stack_trace_table.h index 552ee03dc..dfd361970 100644 --- a/tcmalloc/stack_trace_table.h +++ b/tcmalloc/stack_trace_table.h @@ -17,6 +17,9 @@ #ifndef TCMALLOC_STACK_TRACE_TABLE_H_ #define TCMALLOC_STACK_TRACE_TABLE_H_ +#include +#include + #include "absl/base/thread_annotations.h" #include "absl/time/time.h" #include "tcmalloc/common.h" @@ -48,6 +51,13 @@ class StackTraceTable final : public ProfileBase { void SetDuration(absl::Duration duration) { duration_ = duration; } [[nodiscard]] absl::Duration Duration() const override { return duration_; } + void SetSamplingInterval(int64_t sampling_interval) { + sampling_interval_ = sampling_interval; + } + [[nodiscard]] std::optional SamplingInterval() const override { + return sampling_interval_; + } + // Adds stack trace "t" of the sample to table with the given weight of the // sample. `sample_weight` is a floating point value used to calculate the // the expected number of objects allocated (might be fractional considering @@ -68,6 +78,7 @@ class StackTraceTable final : public ProfileBase { ProfileType type_; absl::Duration duration_ = absl::ZeroDuration(); absl::Time start_time_; + std::optional sampling_interval_; int depth_total_; LinkedSample* all_; };