No public description PiperOrigin-RevId: 603963015
diff --git a/centipede/BUILD b/centipede/BUILD index 059081b..ddff496 100644 --- a/centipede/BUILD +++ b/centipede/BUILD
@@ -144,6 +144,7 @@ ":control_flow", ":logging", ":pc_info", + ":thread_pool", ":util", "@com_google_absl//absl/container:node_hash_set", "@com_google_absl//absl/log",
diff --git a/centipede/binary_info.cc b/centipede/binary_info.cc index 95a6b08..cc9dc2f 100644 --- a/centipede/binary_info.cc +++ b/centipede/binary_info.cc
@@ -116,7 +116,7 @@ ScopedFile sym_tmp1_path(tmp_dir_path, "symbols_tmp1"); ScopedFile sym_tmp2_path(tmp_dir_path, "symbols_tmp2"); symbols.GetSymbolsFromBinary(pc_table, dso_table, symbolizer_path, - sym_tmp1_path.path(), sym_tmp2_path.path()); + tmp_dir_path); } }
diff --git a/centipede/coverage_test.cc b/centipede/coverage_test.cc index 873156b..ad949e4 100644 --- a/centipede/coverage_test.cc +++ b/centipede/coverage_test.cc
@@ -422,8 +422,7 @@ const DsoTable dso_table = {{GetTargetPath(), pc_table.size()}}; SymbolTable symbols; symbols.GetSymbolsFromBinary(pc_table, dso_table, GetLLVMSymbolizerPath(), - GetTempFilePath(test_info_->name(), 0), - GetTempFilePath(test_info_->name(), 1)); + GetTestTempDir(test_info_->name()).string()); // Empty filter. FunctionFilter empty_filter("", symbols); EXPECT_EQ(empty_filter.count(), 0);
diff --git a/centipede/symbol_table.cc b/centipede/symbol_table.cc index 121fd54..4724445 100644 --- a/centipede/symbol_table.cc +++ b/centipede/symbol_table.cc
@@ -14,6 +14,8 @@ #include "./centipede/symbol_table.h" +#include <algorithm> +#include <atomic> #include <cstdlib> #include <filesystem> // NOLINT #include <fstream> @@ -21,6 +23,7 @@ #include <ostream> #include <string> #include <string_view> +#include <utility> #include <vector> #include "absl/log/check.h" @@ -35,6 +38,7 @@ #include "./centipede/control_flow.h" #include "./centipede/logging.h" #include "./centipede/pc_info.h" +#include "./centipede/thread_pool.h" #include "./centipede/util.h" namespace centipede { @@ -73,16 +77,21 @@ void SymbolTable::GetSymbolsFromOneDso(absl::Span<const PCInfo> pc_infos, std::string_view dso_path, std::string_view symbolizer_path, - std::string_view tmp_path1, - std::string_view tmp_path2) { - auto pcs_path(tmp_path1); - auto symbols_path(tmp_path2); + std::string_view tmp_dir_path) { + static std::atomic_size_t unique_id = 0; + size_t unique_id_value = unique_id.fetch_add(1); + std::string dso_basename = std::filesystem::path{dso_path}.filename(); + ScopedFile pcs_file{tmp_dir_path, + absl::StrCat(dso_basename, ".pcs.", unique_id_value)}; + ScopedFile symbols_file{ + tmp_dir_path, absl::StrCat(dso_basename, ".symbols.", unique_id_value)}; + // Create the input file (one PC per line). std::string pcs_string; for (const auto &pc_info : pc_infos) { absl::StrAppend(&pcs_string, "0x", absl::Hex(pc_info.pc), "\n"); } - WriteToLocalFile(pcs_path, pcs_string); + WriteToLocalFile(pcs_file.path(), pcs_string); // Run the symbolizer. Command cmd(symbolizer_path, { @@ -90,12 +99,12 @@ "-e", std::string(dso_path), "<", - std::string(pcs_path), + std::string(pcs_file.path()), }, - /*env=*/{}, symbols_path); + /*env=*/{}, symbols_file.path()); LOG(INFO) << "Symbolizing " << pc_infos.size() << " PCs from " - << std::filesystem::path(dso_path).filename(); + << dso_basename; int exit_code = cmd.Execute(); if (exit_code != EXIT_SUCCESS) { @@ -103,11 +112,9 @@ return; } // Get and process the symbolizer output. - std::ifstream symbolizer_output(std::string{symbols_path}); + std::ifstream symbolizer_output(std::string{symbols_file.path()}); size_t old_size = size(); ReadFromLLVMSymbolizer(symbolizer_output); - std::filesystem::remove(pcs_path); - std::filesystem::remove(symbols_path); size_t new_size = size(); size_t added_size = new_size - old_size; if (added_size != pc_infos.size()) @@ -117,8 +124,7 @@ void SymbolTable::GetSymbolsFromBinary(const PCTable &pc_table, const DsoTable &dso_table, std::string_view symbolizer_path, - std::string_view tmp_path1, - std::string_view tmp_path2) { + std::string_view tmp_dir_path) { // NOTE: --symbolizer_path=/dev/null is a somewhat expected alternative to // "" that users might pass. if (symbolizer_path.empty() || symbolizer_path == "/dev/null") { @@ -127,18 +133,36 @@ return; } - LOG(INFO) << "Symbolizing " << dso_table.size() << " instrumented DSOs"; + LOG(INFO) << "Symbolizing " << dso_table.size() << " instrumented DSOs."; // Iterate all DSOs, symbolize their respective PCs. + // Symbolizing the PCs can take time, so we + // record them in parallel into separate symbol tables, + // and later merge. + std::vector<SymbolTable> symbol_tables(dso_table.size()); size_t pc_idx_begin = 0; - for (const auto &dso_info : dso_table) { - CHECK_LE(pc_idx_begin + dso_info.num_instrumented_pcs, pc_table.size()) - << VV(pc_idx_begin) << VV(dso_info.num_instrumented_pcs); - const absl::Span<const PCInfo> pc_infos = {pc_table.data() + pc_idx_begin, - dso_info.num_instrumented_pcs}; - GetSymbolsFromOneDso(pc_infos, dso_info.path, symbolizer_path, tmp_path1, - tmp_path2); - pc_idx_begin += dso_info.num_instrumented_pcs; + { + // Symbolization is quite IO-bound so we arbitrarily run 30 at once + // even if we have few CPUs. + const size_t num_threads = std::min(dso_table.size(), 30UL); + centipede::ThreadPool thread_pool(num_threads); + for (size_t dso_id = 0; dso_id < dso_table.size(); ++dso_id) { + const auto &dso_info = dso_table[dso_id]; + auto &symbol_table = symbol_tables[dso_id]; + CHECK_LE(pc_idx_begin + dso_info.num_instrumented_pcs, pc_table.size()) + << VV(pc_idx_begin) << VV(dso_info.num_instrumented_pcs); + const absl::Span<const PCInfo> pc_infos = {pc_table.data() + pc_idx_begin, + dso_info.num_instrumented_pcs}; + thread_pool.Schedule([&dso_info, pc_infos, symbolizer_path, tmp_dir_path, + &symbol_table]() { + symbol_table.GetSymbolsFromOneDso(pc_infos, dso_info.path, + symbolizer_path, tmp_dir_path); + }); + pc_idx_begin += dso_info.num_instrumented_pcs; + } + } + for (const auto &table : symbol_tables) { + AddEntries(table); } CHECK_EQ(pc_idx_begin, pc_table.size()); @@ -194,4 +218,10 @@ return *table_.insert(std::string{str}).first; } +void SymbolTable::AddEntries(const SymbolTable &other) { + for (const auto &entry : other.entries_) { + AddEntryInternal(entry.func, entry.file, entry.line, entry.col); + } +} + } // namespace centipede
diff --git a/centipede/symbol_table.h b/centipede/symbol_table.h index efbf2c2..d1a6433 100644 --- a/centipede/symbol_table.h +++ b/centipede/symbol_table.h
@@ -83,15 +83,13 @@ // Possibly uses files `tmp_path1` and `tmp_path2` for temporary storage. void GetSymbolsFromBinary(const PCTable &pc_table, const DsoTable &dso_table, std::string_view symbolizer_path, - std::string_view tmp_path1, - std::string_view tmp_path2); + std::string_view tmp_dir_path); // Helper for GetSymbolsFromBinary: symbolizes `pc_infos` for `dso_path`. void GetSymbolsFromOneDso(absl::Span<const PCInfo> pc_infos, std::string_view dso_path, std::string_view symbolizer_path, - std::string_view tmp_path1, - std::string_view tmp_path2); + std::string_view tmp_dir_path); // Sets the table to `size` symbols all of which are unknown. void SetAllToUnknown(size_t size); @@ -117,6 +115,9 @@ // Add function name and file location to symbol table. void AddEntry(std::string_view func, std::string_view file_line_col); + // Add all the entries from the other symbol table into this one. + void AddEntries(const SymbolTable &other); + private: void AddEntryInternal(std::string_view func, std::string_view file, int line = -1, int col = -1);