[llvm-profgen][NFC] Detect pre-aggregated format#191593
Conversation
Created using spr 1.3.4
|
@llvm/pr-subscribers-pgo Author: Amir Ayupov (aaupov) ChangesDistinguish the following profgen input formats:
Pre-aggregated input lacks mmap information, so to enable using single input file for optimizing the main binary and shared libraries, the addresses will be augmented with buildid information in a follow-up (#190863). Full diff: https://github.com/llvm/llvm-project/pull/191593.diff 2 Files Affected:
diff --git a/llvm/tools/llvm-profgen/PerfReader.cpp b/llvm/tools/llvm-profgen/PerfReader.cpp
index 1dc59321fd91f..bbfde1256f2cc 100644
--- a/llvm/tools/llvm-profgen/PerfReader.cpp
+++ b/llvm/tools/llvm-profgen/PerfReader.cpp
@@ -365,9 +365,11 @@ PerfReaderBase::create(ProfiledBinary *Binary, PerfInputFile &PerfInput,
PerfInput.Content =
PerfScriptReader::checkPerfScriptType(PerfInput.InputFile);
- if (PerfInput.Content == PerfContent::LBRStack) {
- PerfReader.reset(
- new HybridPerfReader(Binary, PerfInput.InputFile, PIDFilter));
+ if (PerfInput.Content == PerfContent::LBRStack ||
+ PerfInput.Content == PerfContent::AggLBRStack) {
+ auto *Reader = new HybridPerfReader(Binary, PerfInput.InputFile, PIDFilter);
+ Reader->setIsPreAggregated(PerfInput.Content == PerfContent::AggLBRStack);
+ PerfReader.reset(Reader);
} else if (PerfInput.Content == PerfContent::LBR) {
PerfReader.reset(new LBRPerfReader(Binary, PerfInput.InputFile, PIDFilter));
} else {
@@ -1191,8 +1193,9 @@ PerfContent PerfScriptReader::checkPerfScriptType(StringRef FileName) {
TraceStream TraceIt(FileName);
uint64_t FrameAddr = 0;
while (!TraceIt.isAtEoF()) {
- // Skip the aggregated count
- if (!TraceIt.getCurrentLine().getAsInteger(10, FrameAddr))
+ // Skip the aggregated count and detect pre-aggregated input.
+ bool HasAggCount = !TraceIt.getCurrentLine().getAsInteger(10, FrameAddr);
+ if (HasAggCount)
TraceIt.advance();
// Detect sample with call stack
@@ -1205,7 +1208,7 @@ PerfContent PerfScriptReader::checkPerfScriptType(StringRef FileName) {
if (!TraceIt.isAtEoF()) {
if (isLBRSample(TraceIt.getCurrentLine())) {
if (Count > 0)
- return PerfContent::LBRStack;
+ return HasAggCount ? PerfContent::AggLBRStack : PerfContent::LBRStack;
else
return PerfContent::LBR;
}
diff --git a/llvm/tools/llvm-profgen/PerfReader.h b/llvm/tools/llvm-profgen/PerfReader.h
index 2a4c7594d3a93..83c4fb0447c5c 100644
--- a/llvm/tools/llvm-profgen/PerfReader.h
+++ b/llvm/tools/llvm-profgen/PerfReader.h
@@ -69,8 +69,9 @@ enum PerfFormat {
// The type of perfscript content.
enum PerfContent {
UnknownContent = 0,
- LBR = 1, // Only LBR sample.
- LBRStack = 2, // Hybrid sample including call stack and LBR stack.
+ LBR = 1, // Only LBR sample.
+ LBRStack = 2, // Hybrid sample including call stack and LBR stack.
+ AggLBRStack = 3, // Pre-aggregated hybrid sample.
};
struct PerfInputFile {
@@ -631,6 +632,8 @@ class PerfScriptReader : public PerfReaderBase {
// receiving signals.
static SmallVector<CleanupInstaller, 2> TempFileCleanups;
+ void setIsPreAggregated(bool V) { IsPreAggregated = V; }
+
protected:
// Check whether a given line is LBR sample
static bool isLBRSample(StringRef Line);
@@ -676,6 +679,8 @@ class PerfScriptReader : public PerfReaderBase {
std::set<uint64_t> InvalidReturnAddresses;
// PID for the process of interest
std::optional<int32_t> PIDFilter;
+ // Whether the input is pre-aggregated
+ bool IsPreAggregated = false;
};
/*
|
HighW4y2H3ll
left a comment
There was a problem hiding this comment.
What is the pre-aggregated input? Is this the same as putting 1 as the aggregated count for each sample? Could you explain why we need the new format? And does this new format only applicable to LBRStack samples?
Pre-aggregated means there's a count line preceding call/branch stacks. It's worth distinguishing it as a separate format to allow buildid prefix only for it – because non-aggregated input comes directly from perf script and includes mmap events which serves as a main mechanism to distinguish addresses from the binary/DSOs. |
|
Why do we need to restrict buildID annotation only for LBRStack samples? It looks like the current implementation is compatible with the pre-aggregated input and if there's no aggregated count the default will be 1: llvm-project/llvm/tools/llvm-profgen/PerfReader.cpp Lines 1048 to 1054 in b9998b1 |
We can add buildid prefix support in two ways:
I chose option 1 to keep perf script parsing strict. |
Distinguish the following profgen input formats:
Pre-aggregated input lacks mmap information, so to enable using single input file for optimizing the main binary and shared libraries, the addresses will be augmented with buildid information in a follow-up (#190863).