diff --git a/tcmalloc/internal/profile_builder.cc b/tcmalloc/internal/profile_builder.cc index d1be2da6a..a52f0fc9b 100644 --- a/tcmalloc/internal/profile_builder.cc +++ b/tcmalloc/internal/profile_builder.cc @@ -156,9 +156,14 @@ SampleMergedMap MergeProfileSamplesAndMaybeGetResidencyInfo( data.count += entry.count; data.sum += entry.sum; std::optional residency_info; + // When the caller did not use a size-returning allocation, only + // requested_size bytes are meaningful. Use the same size for both + // residency and compressibility so that EstimateCompressedSize does not + // extrapolate to a total_backed derived from the larger allocated_size. + const size_t size = entry.requested_size_returning ? entry.allocated_size + : entry.requested_size; if (residency) { - residency_info = - residency->Get(entry.span_start_address, entry.allocated_size); + residency_info = residency->Get(entry.span_start_address, size); // As long as `residency_info` provides data in some samples, the merged // data will have their sums. // NOTE: The data here is comparable to `tcmalloc::Profile::Sample::sum`, @@ -203,8 +208,6 @@ SampleMergedMap MergeProfileSamplesAndMaybeGetResidencyInfo( if (exporting_compressibility && residency_info.has_value() && entry.span_start_address != nullptr && entry.requested_size > 0) { - size_t size = entry.requested_size_returning ? entry.allocated_size - : entry.requested_size; absl::Span sample_mem( reinterpret_cast(entry.span_start_address), size); absl::StatusOr res = diff --git a/tcmalloc/internal/profile_builder_test.cc b/tcmalloc/internal/profile_builder_test.cc index 1b28940fe..d6bc64aad 100644 --- a/tcmalloc/internal/profile_builder_test.cc +++ b/tcmalloc/internal/profile_builder_test.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -114,6 +115,39 @@ class StubPageFlags final : public PageFlagsBase { uint64_t stale_scan_period_; }; +class StubResidency final : public Residency { + public: + StubResidency() = default; + ~StubResidency() override = default; + + std::optional Get(const void* addr, size_t size) override { + uintptr_t uaddr = reinterpret_cast(addr); + auto it = infos_.find(uaddr); + if (it != infos_.end()) { + Info result = it->second; + // Cap to queried range, matching real ResidencyPageMap behavior. + result.bytes_resident = std::min(result.bytes_resident, size); + result.bytes_swapped = std::min(result.bytes_swapped, size); + return result; + } + return std::nullopt; + } + + size_t GetHardwarePagesInHugePage() const override { return 512; } + + SinglePageBitmaps GetUnbackedAndSwappedBitmaps(const void* addr) override { + return {.status = absl::StatusCode::kUnimplemented}; + } + + void SetInfo(const void* addr, Info info) { + uintptr_t uaddr = reinterpret_cast(addr); + infos_[uaddr] = info; + } + + private: + absl::flat_hash_map infos_; +}; + // Returns the fully resolved path of this program. std::string RealPath() { char path[PATH_MAX]; @@ -682,7 +716,7 @@ TEST(ProfileConverterTest, HeapProfile) { Pair("objects", 6), }), IsSupersetOf({ - Pair("resident_space", 40), + Pair("resident_space", 20), Pair("swapped_space", 0), Pair("space", 4690), Pair("objects", 10), @@ -1363,6 +1397,108 @@ TEST(BuildId, GnuProperty) { munmap(p, 4096); } +// Verify that when requested_size_returning is false, compressed_size does not +// exceed requested_size. The bug: residency was queried with allocated_size +// but compressibility analyzed only requested_size bytes. +// EstimateCompressedSize then extrapolated the compression ratio to +// total_backed (from residency), which was computed for the larger +// allocated_size. +TEST(ProfileConverterTest, CompressedSizeDoesNotExceedAnalyzedSize) { + constexpr size_t kRequestedSize = 100; + constexpr size_t kAllocatedSize = 128; + + // Incompressible data so compression ratio ≈ 1.0. + std::vector buf(kAllocatedSize); + std::minstd_rand rng(42); + for (auto& b : buf) b = static_cast(rng()); + + Profile::Sample sample = {}; + sample.sum = kRequestedSize; + sample.count = 1; + sample.requested_size = kRequestedSize; + sample.requested_alignment = std::nullopt; + sample.requested_size_returning = false; + sample.allocated_size = kAllocatedSize; + sample.span_start_address = buf.data(); + sample.depth = 1; + sample.stack[0] = reinterpret_cast(&RealPath); + sample.access_hint = hot_cold_t{0}; + sample.access_allocated = Profile::Sample::Access::Hot; + sample.token_id = TokenId{0}; + sample.guarded_status = Profile::Sample::GuardedStatus::NotAttempted; + sample.type = AllocationType::Malloc; + + Profile::Sample returning = sample; + returning.requested_size_returning = true; + + StubPageFlags pageflags; + pageflags.set_stale_scan_period(buf.data(), 60); + + StubResidency residency; + // Residency reports all bytes as resident. + Residency::Info info; + info.bytes_resident = kAllocatedSize; + info.bytes_swapped = 0; + info.page_is_resident.SetBit(0); + residency.SetInfo(buf.data(), info); + + auto fake_profile = std::make_unique(); + fake_profile->SetType(ProfileType::kHeap); + fake_profile->SetDuration(absl::Milliseconds(100)); + fake_profile->SetSamples({sample, returning}); + Profile profile = ProfileAccessor::MakeProfile(std::move(fake_profile)); + + auto converted_or = MakeProfileProto(profile, &pageflags, &residency); + ASSERT_TRUE(converted_or.ok()); + const auto& converted = **converted_or; + + // Find the space_compressed sample type index. + int compressed_idx = -1, size_returning_idx = -1; + for (int i = 0; i < converted.sample_type_size(); ++i) { + if (converted.string_table(converted.sample_type(i).type()) == + "space_compressed") { + compressed_idx = i; + break; + } + } + + for (int i = 0; i < converted.string_table_size(); ++i) { + if (converted.string_table(i) == "size_returning") { + size_returning_idx = i; + } + } + ASSERT_GE(size_returning_idx, 0) << "size_returning sample type not found"; + return; + ASSERT_GE(compressed_idx, 0) << "space_compressed sample type not found"; + + ASSERT_EQ(converted.sample_size(), 2); + auto is_size_returning = [&](const perftools::profiles::Sample& s) -> int { + for (const auto& l : s.label()) { + if (l.key() == size_returning_idx) { + return l.num(); + } + } + return 0; + }; + + std::array sizes = {-1, -1}; + sizes[is_size_returning(converted.sample(0))] = + converted.sample(0).value(compressed_idx); + sizes[is_size_returning(converted.sample(1))] = + converted.sample(1).value(compressed_idx); + + ASSERT_GE(sizes[0], 0); + // The compressed_size should (generally) not exceed the size actually + // analyzed, which for !requested_size_returning is requested_size. + EXPECT_LE(sizes[0], kRequestedSize); + + ASSERT_GE(sizes[1], 0); + // Our random bytes are deterministic, so we can make assertions that may not + // work for all possible inputs. + EXPECT_GE(sizes[1], kRequestedSize); + EXPECT_LE(sizes[1], kAllocatedSize); +} + } // namespace } // namespace tcmalloc_internal } // namespace tcmalloc