Suppress all-zero compressibility columns in heap profiles.

tcmalloc/internal/profile_builder.cc

@@ -145,10 +145,19 @@ using SampleMergedMap =
     absl::flat_hash_map<tcmalloc::Profile::Sample, SampleMergedData,
                         SampleHashWithSubFields, SampleEqWithSubFields>;
 
-SampleMergedMap MergeProfileSamplesAndMaybeGetResidencyInfo(
+struct SampleMergeResult {
+  SampleMergedMap samples;
+  // Whether CompressionAnalyzer produced a result for at least one sample.
+  // Collection being enabled does not imply this: analysis is skipped for any
+  // sample lacking residency information, a span address, or a requested size.
+  bool measured_compressibility = false;
+};
+
+SampleMergeResult MergeProfileSamplesAndMaybeGetResidencyInfo(
     const tcmalloc::Profile& profile, PageFlagsBase* pageflags,
-    Residency* residency, bool exporting_compressibility) {
-  SampleMergedMap map;
+    Residency* residency, bool collecting_compressibility) {
+  SampleMergeResult result;
+  SampleMergedMap& map = result.samples;
   CompressionAnalyzer compression_analyzer;
 
   profile.Iterate([&](const tcmalloc::Profile::Sample& entry) {
@@ -206,18 +215,19 @@ SampleMergedMap MergeProfileSamplesAndMaybeGetResidencyInfo(
       }
     }
 
-    if (exporting_compressibility && residency_info.has_value() &&
+    if (collecting_compressibility && residency_info.has_value() &&
         entry.span_start_address != nullptr && entry.requested_size > 0) {
       absl::Span<const char> sample_mem(
           reinterpret_cast<const char*>(entry.span_start_address), size);
       absl::StatusOr<CompressionAnalyzer::Results> res =
           compression_analyzer.Analyze(sample_mem, *residency_info);
       if (res.ok()) {
+        result.measured_compressibility = true;
         data.zero_size += entry.count * res->zero_bytes;
       }
     }
   });
-  return map;
+  return result;
 }
 
 }  // namespace
@@ -926,7 +936,16 @@ absl::StatusOr<std::unique_ptr<perftools::profiles::Profile>> MakeProfileProto(
     sample_type->set_unit(bytes_id);
   }
 
-  bool exporting_compressibility = false;
+  bool collecting_compressibility = false;
+
+  // Only advertise space_compressed/zero_space when at least one sample was
+  // actually analyzed.  Emitting the columns with a zero in every sample is
+  // indistinguishable downstream from a genuinely incompressible heap, so a
+  // profile that never ran the analyzer would otherwise be reported as 100%
+  // compressible rather than as having no data.
+  const auto [samples, exporting_compressibility] =
+      MergeProfileSamplesAndMaybeGetResidencyInfo(profile, pageflags, residency,
+                                                  collecting_compressibility);
   if (exporting_compressibility) {
     perftools::profiles::ValueType* sample_type = nullptr;
 
@@ -950,8 +969,6 @@ absl::StatusOr<std::unique_ptr<perftools::profiles::Profile>> MakeProfileProto(
 
   converted.set_default_sample_type(default_sample_type_id);
 
-  SampleMergedMap samples = MergeProfileSamplesAndMaybeGetResidencyInfo(
-      profile, pageflags, residency, exporting_compressibility);
   for (const auto& [entry, data] : samples) {
     perftools::profiles::Profile& profile = builder.profile();
     perftools::profiles::Sample& sample = *profile.add_sample();

tcmalloc/internal/profile_builder_test.cc

@@ -112,7 +112,7 @@ class StubPageFlags final : public PageFlagsBase {
  private:
   absl::flat_hash_map<uintptr_t, int> stale_bytes_;
   absl::flat_hash_map<uintptr_t, int> locked_bytes_;
-  uint64_t stale_scan_period_;
+  uint64_t stale_scan_period_ = 0;
 };
 
 class StubResidency final : public Residency {
@@ -1449,7 +1449,7 @@ TEST(ProfileConverterTest, CompressedSizeDoesNotExceedAnalyzedSize) {
   Profile profile = ProfileAccessor::MakeProfile(std::move(fake_profile));
 
   auto converted_or = MakeProfileProto(profile, &pageflags, &residency);
-  ASSERT_TRUE(converted_or.ok());
+  CHECK_OK(converted_or.status());
   const auto& converted = **converted_or;
 
   // Find the space_compressed sample type index.
@@ -1499,6 +1499,56 @@ TEST(ProfileConverterTest, CompressedSizeDoesNotExceedAnalyzedSize) {
   EXPECT_LE(sizes[1], kAllocatedSize);
 }
 
+// A heap profile whose samples were never analyzed must not advertise the
+// compressibility columns at all.  Emitting a zero in every sample is
+// indistinguishable downstream from a fully compressible heap.
+TEST(ProfileConverterTest, NoCompressibilityColumnsWithoutMeasurements) {
+  std::vector<char> buf(128);
+
+  Profile::Sample sample = {};
+  sample.sum = buf.size();
+  sample.count = 1;
+  sample.requested_size = buf.size();
+  sample.requested_alignment = std::nullopt;
+  sample.requested_size_returning = false;
+  sample.allocated_size = buf.size();
+  sample.span_start_address = buf.data();
+  sample.depth = 1;
+  sample.stack[0] = reinterpret_cast<void*>(&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;
+
+  StubPageFlags pageflags;
+  // Residency has nothing for this address, so compressibility is never
+  // computed even though collection is enabled.
+  StubResidency residency;
+
+  auto fake_profile = std::make_unique<FakeProfile>();
+  fake_profile->SetType(ProfileType::kHeap);
+  fake_profile->SetDuration(absl::Milliseconds(100));
+  fake_profile->SetSamples({sample});
+  Profile profile = ProfileAccessor::MakeProfile(std::move(fake_profile));
+
+  auto converted_or = MakeProfileProto(profile, &pageflags, &residency);
+  CHECK_OK(converted_or.status());
+  const auto& converted = **converted_or;
+
+  std::vector<std::string> types;
+  for (const auto& s : converted.sample_type()) {
+    types.push_back(converted.string_table(s.type()));
+  }
+  EXPECT_THAT(types, Not(Contains("space_compressed")));
+  EXPECT_THAT(types, Not(Contains("zero_space")));
+
+  // Every sample must still agree with the advertised sample types.
+  for (const auto& s : converted.sample()) {
+    EXPECT_EQ(s.value_size(), types.size());
+  }
+}
+
 }  // namespace
 }  // namespace tcmalloc_internal
 }  // namespace tcmalloc