From 8258d6ec583be9b688cfe85134a3f7a2052f8d43 Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Mon, 25 Aug 2025 03:29:18 -0700 Subject: [PATCH 1/7] [RFC] Add profile summary information to GCOV This patch adds supports for writing out profile summaries and histogram-based percentile information for samples to the GCOV profile, in a similar manner to how this information is written out for LLVM profiles. It also bumps the GCOV version to 3. --- gcov.cc | 7 +++++-- gcov.h | 1 + profile_reader.cc | 18 ++++++++++++++++++ profile_reader.h | 1 + profile_writer.cc | 27 +++++++++++++++++++++++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/gcov.cc b/gcov.cc index 0351048..d9f1aa1 100644 --- a/gcov.cc +++ b/gcov.cc @@ -29,6 +29,7 @@ ABSL_FLAG(uint64_t, gcov_version, 0x3430372a, "Gcov version number."); +const uint32 GCOV_TAG_AFDO_SUMMARY = 0xa8000000; const uint32 GCOV_TAG_AFDO_FILE_NAMES = 0xaa000000; const uint32 GCOV_TAG_AFDO_FUNCTION = 0xac000000; const uint32 GCOV_TAG_MODULE_GROUPING = 0xae000000; @@ -149,7 +150,8 @@ void gcov_write_string(const char *string) { if (string) { length = strlen(string); - if (absl::GetFlag(FLAGS_gcov_version) >= 2) { + if (absl::GetFlag(FLAGS_gcov_version) == 2 || + absl::GetFlag(FLAGS_gcov_version) == 3) { // Length includes the terminating 0 and is saved in bytes. alloc = length + 1; char *byte_buffer = gcov_write_bytes(4 + alloc); @@ -229,7 +231,8 @@ const char * gcov_read_string(void) { return 0; } - if (absl::GetFlag(FLAGS_gcov_version) >= 2) { + if (absl::GetFlag(FLAGS_gcov_version) == 2 || + absl::GetFlag(FLAGS_gcov_version) == 3) { return gcov_read_bytes (length); } else { diff --git a/gcov.h b/gcov.h index 562d603..1c7947b 100644 --- a/gcov.h +++ b/gcov.h @@ -20,6 +20,7 @@ #include "base/common.h" #include "third_party/abseil/absl/flags/declare.h" +extern const uint32 GCOV_TAG_AFDO_SUMMARY; extern const uint32 GCOV_TAG_AFDO_FILE_NAMES; extern const uint32 GCOV_TAG_AFDO_FUNCTION; extern const uint32 GCOV_TAG_MODULE_GROUPING; diff --git a/profile_reader.cc b/profile_reader.cc index 099fe77..63faf83 100644 --- a/profile_reader.cc +++ b/profile_reader.cc @@ -95,6 +95,23 @@ void AutoFDOProfileReader::ReadSymbolProfile(const SourceStack &stack, } } +void AutoFDOProfileReader::ReadSummary() { + if (absl::GetFlag(FLAGS_gcov_version) >= 3) { + CHECK_EQ(gcov_read_unsigned(), GCOV_TAG_AFDO_SUMMARY); + gcov_read_counter(); // Total count + gcov_read_counter(); // Max count + gcov_read_counter(); // Max function count + gcov_read_counter(); // Num counts + gcov_read_counter(); // Num functions + unsigned num = gcov_read_counter(); + for (unsigned i = 0; i < num; i++) { + gcov_read_unsigned(); // Cutoff + gcov_read_counter(); // Min count + gcov_read_counter(); // Num cutoffs + } + } +} + void AutoFDOProfileReader::ReadNameTable() { CHECK_EQ(gcov_read_unsigned(), GCOV_TAG_AFDO_FILE_NAMES); gcov_read_unsigned(); @@ -132,6 +149,7 @@ bool AutoFDOProfileReader::ReadFromFile(const std::string &output_file) { absl::SetFlag(&FLAGS_gcov_version, gcov_read_unsigned()); gcov_read_unsigned(); + ReadSummary(); ReadNameTable(); ReadFunctionProfile(); ReadModuleGroup(); diff --git a/profile_reader.h b/profile_reader.h index 3812cf6..9d332c3 100644 --- a/profile_reader.h +++ b/profile_reader.h @@ -54,6 +54,7 @@ class AutoFDOProfileReader : public ProfileReader { // where symbol_map was built purely from profile thus alias symbol info // is not available. In that case, we should always update the symbol. void ReadSymbolProfile(const SourceStack &stack, bool update); + void ReadSummary(); void ReadNameTable(); SymbolMap *symbol_map_; diff --git a/profile_writer.cc b/profile_writer.cc index 958165e..59c2807 100644 --- a/profile_writer.cc +++ b/profile_writer.cc @@ -26,6 +26,10 @@ #include "third_party/abseil/absl/flags/flag.h" #include "third_party/abseil/absl/strings/str_format.h" +#include "llvm_profile_writer.h" +#include "llvm/ProfileData/SampleProf.h" +#include "llvm/ProfileData/ProfileCommon.h" + // sizeof(gcov_unsigned_t) #define SIZEOF_UNSIGNED 4 @@ -156,6 +160,29 @@ void AutoFDOProfileWriter::WriteFunctionProfile() { FileIndexMap file_map; StringTableUpdater::Update(*symbol_map_, &string_index_map, &file_map); + LLVMProfileBuilder builder(string_index_map); + const llvm::SampleProfileMap &profiles = + builder.ConvertProfiles(*symbol_map_); + llvm::SampleProfileSummaryBuilder summary_builder(llvm::ProfileSummaryBuilder::DefaultCutoffs); + std::unique_ptr summary = summary_builder.computeSummaryForProfiles(profiles); + + // Write out the GCOV_TAG_AFDO_SUMMARY section. + if (absl::GetFlag(FLAGS_gcov_version) >= 3) { + assert(summary != nullptr); + gcov_write_unsigned(GCOV_TAG_AFDO_SUMMARY); + gcov_write_counter(summary->getTotalCount()); + gcov_write_counter(summary->getMaxCount()); + gcov_write_counter(summary->getMaxFunctionCount()); + gcov_write_counter(summary->getNumCounts()); + gcov_write_counter(summary->getNumFunctions()); + gcov_write_counter(summary->getDetailedSummary().size()); + for (const auto &detailed_summary : summary->getDetailedSummary()) { + gcov_write_unsigned(detailed_summary.Cutoff); + gcov_write_counter(detailed_summary.MinCount); + gcov_write_counter(detailed_summary.NumCounts); + } + } + for (auto &name_index : string_index_map) { name_index.second = current_name_index++; length_4bytes += (name_index.first.size() From 96c7bf850b5cf0fea84f557ae7a5f4c5735cc9cc Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Mon, 27 Oct 2025 00:19:50 -0700 Subject: [PATCH 2/7] Simplify version comparison --- gcov.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/gcov.cc b/gcov.cc index d9f1aa1..f16437b 100644 --- a/gcov.cc +++ b/gcov.cc @@ -150,8 +150,7 @@ void gcov_write_string(const char *string) { if (string) { length = strlen(string); - if (absl::GetFlag(FLAGS_gcov_version) == 2 || - absl::GetFlag(FLAGS_gcov_version) == 3) { + if (absl::GetFlag(FLAGS_gcov_version) >= 2) { // Length includes the terminating 0 and is saved in bytes. alloc = length + 1; char *byte_buffer = gcov_write_bytes(4 + alloc); @@ -231,8 +230,7 @@ const char * gcov_read_string(void) { return 0; } - if (absl::GetFlag(FLAGS_gcov_version) == 2 || - absl::GetFlag(FLAGS_gcov_version) == 3) { + if (absl::GetFlag(FLAGS_gcov_version) >= 2) { return gcov_read_bytes (length); } else { From d123f7014b9e4fa3ca12ba9b0a522f4be93a7825 Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Wed, 12 Nov 2025 00:55:17 -0800 Subject: [PATCH 3/7] Implement summary calculation as standalone visitor --- profile_writer.cc | 90 +++++++++++++++++++++++++++++++++++++---------- profile_writer.h | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 19 deletions(-) diff --git a/profile_writer.cc b/profile_writer.cc index 59c2807..8971c3a 100644 --- a/profile_writer.cc +++ b/profile_writer.cc @@ -26,10 +26,6 @@ #include "third_party/abseil/absl/flags/flag.h" #include "third_party/abseil/absl/strings/str_format.h" -#include "llvm_profile_writer.h" -#include "llvm/ProfileData/SampleProf.h" -#include "llvm/ProfileData/ProfileCommon.h" - // sizeof(gcov_unsigned_t) #define SIZEOF_UNSIGNED 4 @@ -160,26 +156,24 @@ void AutoFDOProfileWriter::WriteFunctionProfile() { FileIndexMap file_map; StringTableUpdater::Update(*symbol_map_, &string_index_map, &file_map); - LLVMProfileBuilder builder(string_index_map); - const llvm::SampleProfileMap &profiles = - builder.ConvertProfiles(*symbol_map_); - llvm::SampleProfileSummaryBuilder summary_builder(llvm::ProfileSummaryBuilder::DefaultCutoffs); - std::unique_ptr summary = summary_builder.computeSummaryForProfiles(profiles); + ProfileSummaryInformation info = ProfileSummaryComputer::Compute( + *symbol_map_, {std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)}); // Write out the GCOV_TAG_AFDO_SUMMARY section. if (absl::GetFlag(FLAGS_gcov_version) >= 3) { assert(summary != nullptr); gcov_write_unsigned(GCOV_TAG_AFDO_SUMMARY); - gcov_write_counter(summary->getTotalCount()); - gcov_write_counter(summary->getMaxCount()); - gcov_write_counter(summary->getMaxFunctionCount()); - gcov_write_counter(summary->getNumCounts()); - gcov_write_counter(summary->getNumFunctions()); - gcov_write_counter(summary->getDetailedSummary().size()); - for (const auto &detailed_summary : summary->getDetailedSummary()) { - gcov_write_unsigned(detailed_summary.Cutoff); - gcov_write_counter(detailed_summary.MinCount); - gcov_write_counter(detailed_summary.NumCounts); + gcov_write_counter(info.total_count_); + gcov_write_counter(info.max_count_); + gcov_write_counter(info.max_function_count_); + gcov_write_counter(info.num_counts_); + gcov_write_counter(info.num_functions_); + gcov_write_counter(info.detailed_summaries_.size()); + for (const auto &detailed_summary : info.detailed_summaries_) { + gcov_write_unsigned(detailed_summary.cutoff_); + gcov_write_counter(detailed_summary.min_count_); + gcov_write_counter(detailed_summary.num_counts_); } } @@ -270,6 +264,64 @@ bool AutoFDOProfileWriter::WriteToFile(const std::string &output_filename) { return true; } +ProfileSummaryComputer::ProfileSummaryComputer() + : cutoffs_{std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)} {} + +ProfileSummaryComputer::ProfileSummaryComputer(std::vector cutoffs) + : cutoffs_{std::move(cutoffs)} {} + +void ProfileSummaryComputer::VisitTopSymbol(const std::string &name, + const Symbol *node) { + info_.num_functions_++; + info_.max_function_count_ = + std::max(info_.max_function_count_, node->head_count); +} + +void ProfileSummaryComputer::Visit(const Symbol *node) { + // There is a slight difference against the values computed by + // SampleProfileSummaryBuilder/LLVMProfileBuilder as it represents + // lineno:discriminator pairs as 16:32 bits. This causes line numbers >= + // UINT16_MAX to be counted incorrectly (see GetLineNumberFromOffset in + // source_info.h) as they collide with line numbers < UINT16_MAX. This issue + // is completely avoided here by just not using the offset info at all. + for (auto &pos_count : node->pos_counts) { + uint64_t count = pos_count.second.count; + info_.total_count_ += count; + info_.max_count_ = std::max(info_.max_count_, count); + info_.num_counts_++; + count_frequencies_[count]++; + } +} + +void ProfileSummaryComputer::ComputeDetailedSummary() { + auto iter = count_frequencies_.begin(); + auto end = count_frequencies_.end(); + + uint32_t counts_seen = 0; + uint64_t curr_sum = 0; + uint64_t count = 0; + + for (const uint32_t cutoff : cutoffs_) { + assert(cutoff <= 999'999); + constexpr int scale = 1'000'000; + using uint128_t = unsigned __int128; + uint128_t desired_count = info_.total_count_; + desired_count = desired_count * uint128_t(cutoff); + desired_count = desired_count / uint128_t(scale); + assert(desired_count <= info_.total_count_); + while (curr_sum < desired_count && iter != end) { + count = iter->first; + uint32_t freq = iter->second; + curr_sum += (count * freq); + counts_seen += freq; + iter++; + } + assert(curr_sum >= desired_count); + info_.detailed_summaries_.push_back({cutoff, count, counts_seen}); + } +} + // Debugging support. ProfileDumper emits a detailed dump of the contents // of the input profile. class ProfileDumper : public SymbolTraverser { diff --git a/profile_writer.h b/profile_writer.h index 2d843e0..1fe1451 100644 --- a/profile_writer.h +++ b/profile_writer.h @@ -242,6 +242,68 @@ class StringTableUpdater: public SymbolTraverser { FileIndexMap *file_map_; }; +class ProfileSummaryInformation { +public: + static constexpr std::array default_cutoffs = { + 10000, 100000, 200000, 300000, 400000, 500000, 600000, 700000, + 800000, 900000, 950000, 990000, 999000, 999900, 999990, 999999}; + + // The detailed summary is a histogram-based calculation of the minimum + // execution count required to belong to a certain set of percentile of + // counts. + struct DetailedSummary { + // The percentile that this represents (multiplied by 1,000,000). + uint32_t cutoff_{}; + // The minimum execution count required to belong to this percentile. + uint64_t min_count_{}; + // The number of samples which belong to this percentile. + uint64_t num_counts_{}; + }; + + // The sum of execution counts of all samples. + uint64_t total_count_{}; + // The maximum individual count. + uint64_t max_count_{}; + // The maximum head count across all functions. + uint64_t max_function_count_{}; + // The number of lines that have samples. + uint64_t num_counts_{}; + // The number of functions that have samples. + uint64_t num_functions_{}; + // The percentile threshold information. + std::vector detailed_summaries_{}; +}; + +class ProfileSummaryComputer : public SymbolTraverser { +public: + // This type is neither copyable nor movable. + ProfileSummaryComputer(const ProfileSummaryComputer &) = delete; + ProfileSummaryComputer &operator=(const ProfileSummaryComputer &) = delete; + + ProfileSummaryComputer(); + ProfileSummaryComputer(std::vector cutoffs); + + static ProfileSummaryInformation Compute(const SymbolMap &symbol_map, + std::vector cutoffs) { + ProfileSummaryComputer computer(std::move(cutoffs)); + computer.Start(symbol_map); + computer.ComputeDetailedSummary(); + return std::move(computer.info_); + } + +protected: + void VisitTopSymbol(const std::string &name, const Symbol *node) override; + void Visit(const Symbol *node) override; + +private: + ProfileSummaryInformation info_{}; + // Sorted map of frequencies - used to compute the histogram. + std::map> count_frequencies_; + std::vector cutoffs_; + + void ComputeDetailedSummary(); +}; + } // namespace devtools_crosstool_autofdo #endif // AUTOFDO_PROFILE_WRITER_H_ From da84bf2021b4070650cf9f81d1a51235364a7e49 Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Tue, 6 Jan 2026 21:16:49 -0800 Subject: [PATCH 4/7] Address review comments --- profile_reader.cc | 2 +- profile_writer.cc | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/profile_reader.cc b/profile_reader.cc index 63faf83..d88b04d 100644 --- a/profile_reader.cc +++ b/profile_reader.cc @@ -107,7 +107,7 @@ void AutoFDOProfileReader::ReadSummary() { for (unsigned i = 0; i < num; i++) { gcov_read_unsigned(); // Cutoff gcov_read_counter(); // Min count - gcov_read_counter(); // Num cutoffs + gcov_read_counter(); // Num counts >= min count } } } diff --git a/profile_writer.cc b/profile_writer.cc index 8971c3a..622e481 100644 --- a/profile_writer.cc +++ b/profile_writer.cc @@ -156,13 +156,12 @@ void AutoFDOProfileWriter::WriteFunctionProfile() { FileIndexMap file_map; StringTableUpdater::Update(*symbol_map_, &string_index_map, &file_map); - ProfileSummaryInformation info = ProfileSummaryComputer::Compute( - *symbol_map_, {std::begin(ProfileSummaryInformation::default_cutoffs), - std::end(ProfileSummaryInformation::default_cutoffs)}); - // Write out the GCOV_TAG_AFDO_SUMMARY section. if (absl::GetFlag(FLAGS_gcov_version) >= 3) { assert(summary != nullptr); + ProfileSummaryInformation info = ProfileSummaryComputer::Compute( + *symbol_map_, {std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)}); gcov_write_unsigned(GCOV_TAG_AFDO_SUMMARY); gcov_write_counter(info.total_count_); gcov_write_counter(info.max_count_); From 99310717bd828f7c11180bde0866db07ef59ca15 Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Sun, 18 Jan 2026 22:28:07 -0800 Subject: [PATCH 5/7] Add testcase for summary calculation --- CMakeLists.txt | 13 +++++ summary_calculation_test.cc | 102 ++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 summary_calculation_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 6eb26e4..bcb7aae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -167,6 +167,19 @@ function (build_gcov) quipper_perf) add_test(NAME addr2line_test COMMAND addr2line_test) + add_executable(summary_calculation_test + summary_calculation_test.cc + gcov.cc + profile_writer.cc + symbol_map.cc) + target_link_libraries(summary_calculation_test + gtest + gtest_main + absl::flags + addr2line_lib + glog) + add_test(NAME summary_calculation_test COMMAND summary_calculation_test) + add_custom_command(PRE_BUILD OUTPUT prepare_cmds COMMAND ln -s -f ${CMAKE_HOME_DIRECTORY}/testdata) diff --git a/summary_calculation_test.cc b/summary_calculation_test.cc new file mode 100644 index 0000000..24c4b91 --- /dev/null +++ b/summary_calculation_test.cc @@ -0,0 +1,102 @@ +#include +#include + +#include "profile_writer.h" +#include "symbol_map.h" +#include "gtest/gtest.h" + +using namespace devtools_crosstool_autofdo; + +TEST(SummaryCalculationTest, SummaryCalculator) { + std::string binary = ::testing::SrcDir() + "/testdata/test.binary"; + + // Taken from SymbolMapTest::TestEntryCount + SymbolMap symbol_map(binary); + symbol_map.AddSymbol("foo"); + symbol_map.AddSymbolEntryCount("foo", 200); + + // foo->bar->baz->qux ==> callsites + SourceStack foo_stack1 = { + {"qux", "", "", 0, 10, 0}, + {"baz", "", "", 0, 20, 0}, + {"bar", "", "", 0, 25, 0}, + {"foo", "", "", 0, 50, 0}, + }; + symbol_map.AddSourceCount("foo", foo_stack1, 300, 2); + + // foo->bar->baz ==> callsites + SourceStack foo_stack2 = { + {"baz", "", "", 0, 30, 0}, + {"bar", "", "", 0, 25, 0}, + {"foo", "", "", 0, 50, 0}, + }; + symbol_map.AddSourceCount("foo", foo_stack2, 0, 2); + + // foo only ==> pos_counts + SourceStack foo_stack3 = { + {"foo", "", "", 0, 55, 0}, + }; + symbol_map.AddSourceCount("foo", foo_stack3, 450, 2); + + symbol_map.AddSymbol("boo"); + symbol_map.AddSymbolEntryCount("boo", 300); + + // boo->dar->daz->dux ==> callsites + SourceStack boo_stack1 = { + {"dux", "", "", 0, 10, 0}, + {"daz", "", "", 0, 20, 0}, + {"dar", "", "", 0, 25, 0}, + {"boo", "", "", 0, 50, 0}, + }; + symbol_map.AddSourceCount("boo", boo_stack1, 100, 2); + + // boo->dar->daz ==> callsites + SourceStack boo_stack2 = { + {"daz", "", "", 0, 30, 0}, + {"dar", "", "", 0, 25, 0}, + {"boo", "", "", 0, 50, 0}, + }; + symbol_map.AddSourceCount("boo", boo_stack2, 0, 2); + + // boo only ==> pos_counts + SourceStack boo_stack3 = { + {"boo", "", "", 0, 55, 0}, + }; + symbol_map.AddSourceCount("boo", boo_stack3, 150, 2); + + ProfileSummaryInformation info = ProfileSummaryComputer::Compute( + symbol_map, {std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)}); + + EXPECT_EQ(info.total_count_, 1000); + EXPECT_EQ(info.max_count_, 450); + EXPECT_EQ(info.max_function_count_, 300); + EXPECT_EQ(info.num_counts_, 6); + EXPECT_EQ(info.num_functions_, 2); + EXPECT_EQ(info.detailed_summaries_.size(), 16); + + std::array, 16> expected_percentiles = { + std::tuple{10000, 450, 1}, + {100000, 450, 1}, + {200000, 450, 1}, + {300000, 450, 1}, + {400000, 450, 1}, + {500000, 300, 2}, + {600000, 300, 2}, + {700000, 300, 2}, + {800000, 150, 3}, + {900000, 150, 3}, + {950000, 100, 4}, + {990000, 100, 4}, + {999000, 100, 4}, + {999900, 100, 4}, + {999990, 100, 4}, + {999999, 100, 4}, + }; + + for (int i = 0; i < 16; i++) { + EXPECT_EQ(info.detailed_summaries_[i].cutoff_, std::get<0>(expected_percentiles[i])); + EXPECT_EQ(info.detailed_summaries_[i].min_count_, std::get<1>(expected_percentiles[i])); + EXPECT_EQ(info.detailed_summaries_[i].num_counts_, std::get<2>(expected_percentiles[i])); + } +} From d25dcf41a907cb4d81dd23cad56a538a53d105de Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Wed, 21 Jan 2026 22:04:22 -0800 Subject: [PATCH 6/7] Address review comments --- CMakeLists.txt | 1 + profile_reader.cc | 30 ++++++--- profile_reader.h | 12 +++- profile_writer.cc | 7 ++ profile_writer.h | 7 ++ summary_calculation_test.cc | 126 ++++++++++++++++++++++++++++-------- 6 files changed, 145 insertions(+), 38 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bcb7aae..b137172 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -170,6 +170,7 @@ function (build_gcov) add_executable(summary_calculation_test summary_calculation_test.cc gcov.cc + profile_reader.cc profile_writer.cc symbol_map.cc) target_link_libraries(summary_calculation_test diff --git a/profile_reader.cc b/profile_reader.cc index d88b04d..745e95d 100644 --- a/profile_reader.cc +++ b/profile_reader.cc @@ -7,10 +7,16 @@ #include "base/logging.h" #include "addr2line.h" #include "gcov.h" +#include "profile_writer.h" #include "symbol_map.h" #include "third_party/abseil/absl/flags/flag.h" namespace devtools_crosstool_autofdo { +AutoFDOProfileReader::~AutoFDOProfileReader() { + if (summary_info_) { + delete summary_info_; + } +} void AutoFDOProfileReader::ReadModuleGroup() { CHECK_EQ(gcov_read_unsigned(), GCOV_TAG_MODULE_GROUPING); @@ -97,18 +103,21 @@ void AutoFDOProfileReader::ReadSymbolProfile(const SourceStack &stack, void AutoFDOProfileReader::ReadSummary() { if (absl::GetFlag(FLAGS_gcov_version) >= 3) { + ProfileSummaryInformation info; CHECK_EQ(gcov_read_unsigned(), GCOV_TAG_AFDO_SUMMARY); - gcov_read_counter(); // Total count - gcov_read_counter(); // Max count - gcov_read_counter(); // Max function count - gcov_read_counter(); // Num counts - gcov_read_counter(); // Num functions + info.total_count_ = gcov_read_counter(); // Total count + info.max_count_ = gcov_read_counter(); // Max count + info.max_function_count_ = gcov_read_counter(); // Max function count + info.num_counts_ = gcov_read_counter(); // Num counts + info.num_functions_ = gcov_read_counter(); // Num functions unsigned num = gcov_read_counter(); + info.detailed_summaries_.resize(num); for (unsigned i = 0; i < num; i++) { - gcov_read_unsigned(); // Cutoff - gcov_read_counter(); // Min count - gcov_read_counter(); // Num counts >= min count + info.detailed_summaries_[i].cutoff_ = gcov_read_unsigned(); // Cutoff + info.detailed_summaries_[i].min_count_ = gcov_read_counter(); // Min count + info.detailed_summaries_[i].num_counts_ = gcov_read_counter(); // Num counts >= min count } + summary_info_ = new ProfileSummaryInformation(info); } } @@ -159,4 +168,9 @@ bool AutoFDOProfileReader::ReadFromFile(const std::string &output_file) { return true; } + +ProfileSummaryInformation *AutoFDOProfileReader::GetSummaryInformation() const { + return summary_info_; +} + } // namespace devtools_crosstool_autofdo diff --git a/profile_reader.h b/profile_reader.h index 9d332c3..425e900 100644 --- a/profile_reader.h +++ b/profile_reader.h @@ -12,19 +12,25 @@ namespace devtools_crosstool_autofdo { +class ProfileSummaryInformation; class SymbolMap; class AutoFDOProfileReader : public ProfileReader { public: // None of the args are owned by this class. explicit AutoFDOProfileReader(SymbolMap *symbol_map, bool force_update) - : symbol_map_(symbol_map), force_update_(force_update) {} + : symbol_map_(symbol_map), force_update_(force_update), summary_info_(nullptr) {} explicit AutoFDOProfileReader() - : symbol_map_(nullptr), force_update_(false) {} + : symbol_map_(nullptr), force_update_(false), summary_info_(nullptr) {} + + ~AutoFDOProfileReader(); bool ReadFromFile(const std::string &output_file) override; + // Get the summary from the profile, if it was read in. + ProfileSummaryInformation *GetSummaryInformation() const; + private: void ReadWorkingSet(); // Reads the module grouping info into the gcda file. @@ -54,11 +60,13 @@ class AutoFDOProfileReader : public ProfileReader { // where symbol_map was built purely from profile thus alias symbol info // is not available. In that case, we should always update the symbol. void ReadSymbolProfile(const SourceStack &stack, bool update); + // Read in the summary information. This requires at least GCOV version 3. void ReadSummary(); void ReadNameTable(); SymbolMap *symbol_map_; bool force_update_; + ProfileSummaryInformation *summary_info_; std::vector> names_; std::vector file_names_; }; diff --git a/profile_writer.cc b/profile_writer.cc index 622e481..c803c7c 100644 --- a/profile_writer.cc +++ b/profile_writer.cc @@ -308,6 +308,8 @@ void ProfileSummaryComputer::ComputeDetailedSummary() { uint128_t desired_count = info_.total_count_; desired_count = desired_count * uint128_t(cutoff); desired_count = desired_count / uint128_t(scale); + // This should never fail as cutoff is always <= scale, so + // (info_.total_count_ * (cutoff / scale)) is always <= info_.total_count_. assert(desired_count <= info_.total_count_); while (curr_sum < desired_count && iter != end) { count = iter->first; @@ -316,6 +318,11 @@ void ProfileSummaryComputer::ComputeDetailedSummary() { counts_seen += freq; iter++; } + // curr_sum is the cumulative sum of frequencies, of which + // info_.total_count_ is the maximum value (as computed in + // ProfileSummaryComputer::Visit). Thus, this assertion will only fail if + // desired_count > info_.total_count_ and the maximum value that curr_sum + // can sum to is lesser than it. assert(curr_sum >= desired_count); info_.detailed_summaries_.push_back({cutoff, count, counts_seen}); } diff --git a/profile_writer.h b/profile_writer.h index 1fe1451..a204da6 100644 --- a/profile_writer.h +++ b/profile_writer.h @@ -301,6 +301,13 @@ class ProfileSummaryComputer : public SymbolTraverser { std::map> count_frequencies_; std::vector cutoffs_; + // Compute the percentile information. This is done using a histogram based on + // (execution count, number of such counts) pairs, where a percentile + // represents the minimum execution count required to belong to that + // percentile. + // + // This is adapted from the LLVM implementation in + // ProfileSummaryBuilder::computeDetailedSummary (ProfileSummaryBuilder.cpp). void ComputeDetailedSummary(); }; diff --git a/summary_calculation_test.cc b/summary_calculation_test.cc index 24c4b91..87ad8f3 100644 --- a/summary_calculation_test.cc +++ b/summary_calculation_test.cc @@ -1,17 +1,17 @@ #include #include +#include "profile_reader.h" #include "profile_writer.h" #include "symbol_map.h" #include "gtest/gtest.h" using namespace devtools_crosstool_autofdo; -TEST(SummaryCalculationTest, SummaryCalculator) { - std::string binary = ::testing::SrcDir() + "/testdata/test.binary"; +namespace { +void InitializeSymbolMap(SymbolMap &symbol_map) { // Taken from SymbolMapTest::TestEntryCount - SymbolMap symbol_map(binary); symbol_map.AddSymbol("foo"); symbol_map.AddSymbolEntryCount("foo", 200); @@ -63,11 +63,30 @@ TEST(SummaryCalculationTest, SummaryCalculator) { {"boo", "", "", 0, 55, 0}, }; symbol_map.AddSourceCount("boo", boo_stack3, 150, 2); +} - ProfileSummaryInformation info = ProfileSummaryComputer::Compute( - symbol_map, {std::begin(ProfileSummaryInformation::default_cutoffs), - std::end(ProfileSummaryInformation::default_cutoffs)}); +// clang-format off +std::array, 16> ExpectedPercentiles = { + std::tuple{10000, 450, 1}, + {100000, 450, 1}, + {200000, 450, 1}, + {300000, 450, 1}, + {400000, 450, 1}, + {500000, 300, 2}, + {600000, 300, 2}, + {700000, 300, 2}, + {800000, 150, 3}, + {900000, 150, 3}, + {950000, 100, 4}, + {990000, 100, 4}, + {999000, 100, 4}, + {999900, 100, 4}, + {999990, 100, 4}, + {999999, 100, 4}, +}; +// clang-format on +void VerifySummaryInformation(ProfileSummaryInformation &info) { EXPECT_EQ(info.total_count_, 1000); EXPECT_EQ(info.max_count_, 450); EXPECT_EQ(info.max_function_count_, 300); @@ -75,28 +94,79 @@ TEST(SummaryCalculationTest, SummaryCalculator) { EXPECT_EQ(info.num_functions_, 2); EXPECT_EQ(info.detailed_summaries_.size(), 16); - std::array, 16> expected_percentiles = { - std::tuple{10000, 450, 1}, - {100000, 450, 1}, - {200000, 450, 1}, - {300000, 450, 1}, - {400000, 450, 1}, - {500000, 300, 2}, - {600000, 300, 2}, - {700000, 300, 2}, - {800000, 150, 3}, - {900000, 150, 3}, - {950000, 100, 4}, - {990000, 100, 4}, - {999000, 100, 4}, - {999900, 100, 4}, - {999990, 100, 4}, - {999999, 100, 4}, - }; - for (int i = 0; i < 16; i++) { - EXPECT_EQ(info.detailed_summaries_[i].cutoff_, std::get<0>(expected_percentiles[i])); - EXPECT_EQ(info.detailed_summaries_[i].min_count_, std::get<1>(expected_percentiles[i])); - EXPECT_EQ(info.detailed_summaries_[i].num_counts_, std::get<2>(expected_percentiles[i])); + EXPECT_EQ(info.detailed_summaries_[i].cutoff_, + std::get<0>(ExpectedPercentiles[i])); + EXPECT_EQ(info.detailed_summaries_[i].min_count_, + std::get<1>(ExpectedPercentiles[i])); + EXPECT_EQ(info.detailed_summaries_[i].num_counts_, + std::get<2>(ExpectedPercentiles[i])); } } + +TEST(ProfileSummaryCalculator, SummaryCalculationTest) { + std::string binary = ::testing::SrcDir() + "/testdata/test.binary"; + + SymbolMap symbol_map(binary); + InitializeSymbolMap(symbol_map); + + ProfileSummaryInformation info = ProfileSummaryComputer::Compute( + symbol_map, {std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)}); + + // Verify that the summary was calculated correctly. + VerifySummaryInformation(info); +} + +TEST(ProfileSummaryCalculator, SummaryReadWriteTest) { + std::string binary = ::testing::SrcDir() + "/testdata/test.binary"; + + SymbolMap symbol_map_1(binary); + InitializeSymbolMap(symbol_map_1); + + char template_name[] = "summary_read_test.XXXXXX"; + const char *name = mktemp(template_name); + ASSERT_NE(name, nullptr); + + // Write out the summary information. + AutoFDOProfileWriter writer(&symbol_map_1, 3); + writer.WriteToFile(name); + + // Read the summary information back in. + SymbolMap symbol_map_2; + AutoFDOProfileReader reader(&symbol_map_2, false); + reader.ReadFromFile(name); + + // Re-calculate the summary that was written out. + ProfileSummaryInformation info_1 = ProfileSummaryComputer::Compute( + symbol_map_1, {std::begin(ProfileSummaryInformation::default_cutoffs), + std::end(ProfileSummaryInformation::default_cutoffs)}); + // Verify that this was calculated correctly. + VerifySummaryInformation(info_1); + + // Get the summary that was read back in. + ProfileSummaryInformation *info_2 = reader.GetSummaryInformation(); + ASSERT_NE(info_2, nullptr); + + // Make sure the summary that was written out is the same as that which is + // read back in. + EXPECT_EQ(info_1.total_count_, info_2->total_count_); + EXPECT_EQ(info_1.max_count_, info_2->max_count_); + EXPECT_EQ(info_1.max_function_count_, info_2->max_function_count_); + EXPECT_EQ(info_1.num_counts_, info_2->num_counts_); + EXPECT_EQ(info_1.num_functions_, info_2->num_functions_); + EXPECT_EQ(info_1.detailed_summaries_.size(), + info_2->detailed_summaries_.size()); + for (int i = 0; i < info_1.detailed_summaries_.size(); i++) { + EXPECT_EQ(info_1.detailed_summaries_[i].cutoff_, + info_2->detailed_summaries_[i].cutoff_); + EXPECT_EQ(info_1.detailed_summaries_[i].min_count_, + info_2->detailed_summaries_[i].min_count_); + EXPECT_EQ(info_1.detailed_summaries_[i].num_counts_, + info_2->detailed_summaries_[i].num_counts_); + } + + remove(name); +} + +} // namespace From b9e980d84a1bfef4c9425253af4e90840826eafe Mon Sep 17 00:00:00 2001 From: Dhruv Chawla Date: Thu, 22 Jan 2026 21:11:24 -0800 Subject: [PATCH 7/7] Address review comments 2 --- profile_reader.h | 2 +- profile_writer.cc | 18 ++++++++++++++++++ profile_writer.h | 7 +++++-- summary_calculation_test.cc | 19 +++---------------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/profile_reader.h b/profile_reader.h index 425e900..0ea42e5 100644 --- a/profile_reader.h +++ b/profile_reader.h @@ -12,7 +12,7 @@ namespace devtools_crosstool_autofdo { -class ProfileSummaryInformation; +struct ProfileSummaryInformation; class SymbolMap; class AutoFDOProfileReader : public ProfileReader { diff --git a/profile_writer.cc b/profile_writer.cc index c803c7c..c45d0a8 100644 --- a/profile_writer.cc +++ b/profile_writer.cc @@ -263,6 +263,24 @@ bool AutoFDOProfileWriter::WriteToFile(const std::string &output_filename) { return true; } +bool ProfileSummaryInformation::operator==( + const ProfileSummaryInformation &other) const { + return total_count_ == other.total_count_ + && max_count_ == other.max_count_ + && max_function_count_ == other.max_function_count_ + && num_counts_ == other.num_counts_ + && num_functions_ == other.num_functions_ + && std::equal(detailed_summaries_.begin(), detailed_summaries_.end(), + other.detailed_summaries_.begin()); +} + +bool ProfileSummaryInformation::DetailedSummary::operator==( + const ProfileSummaryInformation::DetailedSummary &other) const { + return cutoff_ == other.cutoff_ + && min_count_ == other.min_count_ + && num_counts_ == other.num_counts_; +} + ProfileSummaryComputer::ProfileSummaryComputer() : cutoffs_{std::begin(ProfileSummaryInformation::default_cutoffs), std::end(ProfileSummaryInformation::default_cutoffs)} {} diff --git a/profile_writer.h b/profile_writer.h index a204da6..04431d7 100644 --- a/profile_writer.h +++ b/profile_writer.h @@ -242,8 +242,7 @@ class StringTableUpdater: public SymbolTraverser { FileIndexMap *file_map_; }; -class ProfileSummaryInformation { -public: +struct ProfileSummaryInformation { static constexpr std::array default_cutoffs = { 10000, 100000, 200000, 300000, 400000, 500000, 600000, 700000, 800000, 900000, 950000, 990000, 999000, 999900, 999990, 999999}; @@ -258,6 +257,8 @@ class ProfileSummaryInformation { uint64_t min_count_{}; // The number of samples which belong to this percentile. uint64_t num_counts_{}; + + bool operator==(const DetailedSummary &other) const; }; // The sum of execution counts of all samples. @@ -272,6 +273,8 @@ class ProfileSummaryInformation { uint64_t num_functions_{}; // The percentile threshold information. std::vector detailed_summaries_{}; + + bool operator==(const ProfileSummaryInformation &other) const; }; class ProfileSummaryComputer : public SymbolTraverser { diff --git a/summary_calculation_test.cc b/summary_calculation_test.cc index 87ad8f3..7ee780c 100644 --- a/summary_calculation_test.cc +++ b/summary_calculation_test.cc @@ -4,11 +4,13 @@ #include "profile_reader.h" #include "profile_writer.h" #include "symbol_map.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" using namespace devtools_crosstool_autofdo; namespace { +using ::testing::Eq; void InitializeSymbolMap(SymbolMap &symbol_map) { // Taken from SymbolMapTest::TestEntryCount @@ -150,22 +152,7 @@ TEST(ProfileSummaryCalculator, SummaryReadWriteTest) { // Make sure the summary that was written out is the same as that which is // read back in. - EXPECT_EQ(info_1.total_count_, info_2->total_count_); - EXPECT_EQ(info_1.max_count_, info_2->max_count_); - EXPECT_EQ(info_1.max_function_count_, info_2->max_function_count_); - EXPECT_EQ(info_1.num_counts_, info_2->num_counts_); - EXPECT_EQ(info_1.num_functions_, info_2->num_functions_); - EXPECT_EQ(info_1.detailed_summaries_.size(), - info_2->detailed_summaries_.size()); - for (int i = 0; i < info_1.detailed_summaries_.size(); i++) { - EXPECT_EQ(info_1.detailed_summaries_[i].cutoff_, - info_2->detailed_summaries_[i].cutoff_); - EXPECT_EQ(info_1.detailed_summaries_[i].min_count_, - info_2->detailed_summaries_[i].min_count_); - EXPECT_EQ(info_1.detailed_summaries_[i].num_counts_, - info_2->detailed_summaries_[i].num_counts_); - } - + EXPECT_THAT(info_1, Eq(*info_2)); remove(name); }