[llvm-profgen] Improve sample profile density #92144
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,14 +75,18 @@ static cl::opt<int, true> CSProfMaxContextDepth( | |
| "depth limit."), | ||
| cl::location(llvm::sampleprof::CSProfileGenerator::MaxContextDepth)); | ||
|
|
||
| static cl::opt<double> ProfileDensityThreshold( | ||
| "profile-density-threshold", llvm::cl::init(50), | ||
| llvm::cl::desc("If the profile density is below the given threshold, it " | ||
| "will be suggested to increase the sampling rate."), | ||
| llvm::cl::Optional); | ||
| static cl::opt<bool> ShowDensity("show-density", llvm::cl::init(false), | ||
| llvm::cl::desc("show profile density details"), | ||
| llvm::cl::Optional); | ||
| static cl::opt<int> ProfileDensityCutOffHot( | ||
| "profile-density-cutoff-hot", llvm::cl::init(990000), | ||
| llvm::cl::desc("Total samples cutoff for functions used to calculate " | ||
| "profile density.")); | ||
|
|
||
| static cl::opt<bool> UpdateTotalSamples( | ||
| "update-total-samples", llvm::cl::init(false), | ||
|
|
@@ -179,21 +183,22 @@ void ProfileGeneratorBase::write() { | |
|
|
||
| void ProfileGeneratorBase::showDensitySuggestion(double Density) { | ||
| if (Density == 0.0) | ||
| WithColor::warning() << "The output profile is empty or the " | ||
| "--profile-density-cutoff-hot option is " | ||
| "set too low. Please check your command.\n"; | ||
| else if (Density < ProfileDensityThreshold) | ||
| WithColor::warning() | ||
| << "Sample PGO is estimated to optimize better with " | ||
| << format("%.1f", ProfileDensityThreshold / Density) | ||
| << "x more samples. Please consider increasing sampling rate or " | ||
| "profiling for longer duration to get more samples.\n"; | ||
|
|
||
| if (ShowDensity) | ||
| outs() << "Functions with density >= " << format("%.1f", Density) | ||
| << " account for " | ||
| << format("%.2f", | ||
| static_cast<double>(ProfileDensityCutOffHot) / 10000) | ||
| << "% total sample counts.\n"; | ||
| } | ||
|
|
||
| bool ProfileGeneratorBase::filterAmbiguousProfile(FunctionSamples &FS) { | ||
|
|
@@ -238,32 +243,6 @@ void ProfileGeneratorBase::filterAmbiguousProfile(SampleProfileMap &Profiles) { | |
| } | ||
| } | ||
|
|
||
| void ProfileGeneratorBase::findDisjointRanges(RangeSample &DisjointRanges, | ||
| const RangeSample &Ranges) { | ||
|
|
||
|
|
@@ -768,9 +747,95 @@ void ProfileGenerator::populateBoundarySamplesForAllFunctions( | |
| } | ||
| } | ||
|
|
||
| void ProfileGeneratorBase::calculateBodySamplesAndSize( | ||
| const FunctionSamples &FSamples, uint64_t &TotalBodySamples, | ||
|
There was a problem hiding this comment. How different is this There was a problem hiding this comment. It should be no difference for probe-based profile. But very different to line-number based (AutoFDO) profile, in which the entry is the max of the counts in the same line, however, the total_samples is a sum of all the counts in the same line, so the If so, we need to diverge the function size calculation: There was a problem hiding this comment. May still not accurate for AutoFDO to use the There was a problem hiding this comment.
For AutoFDO, based on There was a problem hiding this comment.
Yes, using There was a problem hiding this comment. Even though There was a problem hiding this comment.
It could be 2X~10X bigger, the multiplier is equal to how many physical instruction one source instruction(debug-line) emits, usually one source code line can generate many physical instructions, it could be big. There is an example in our test case: https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-profgen/inline-noprobe.test#L17-L45 See the it's indeed hard to understand why it can cause regression, it's just the observation from data. There was a problem hiding this comment. Ok, can you add a comment explaining why we need to accumulate the samples manually instead of using |
||
| uint64_t &FuncBodySize) { | ||
| // Note that ideally the size should be the number of function instruction. | ||
| // However, for probe-based profile, we don't have the accurate instruction | ||
| // count for each probe, instead, the probe sample is the samples count for | ||
| // the block, which is equivelant to | ||
| // total_instruction_samples/num_of_instruction in one block. Hence, we use | ||
| // the number of probe as a proxy for the function's size. | ||
| FuncBodySize += FSamples.getBodySamples().size(); | ||
|
|
||
| // The accumulated body samples re-calculated here could be different from the | ||
| // TotalSamples(getTotalSamples) field of FunctionSamples for line-number | ||
| // based profile. The reason is that TotalSamples is the sum of all the | ||
| // samples of the machine instruction in one source-code line, however, the | ||
| // entry of Bodysamples is the only max number of them, so the TotalSamples is | ||
| // usually much bigger than the accumulated body samples as one souce-code | ||
| // line can emit many machine instructions. We observed a regression when we | ||
| // switched to use the accumulated body samples(by using | ||
| // -update-total-samples). Hence, it's safer to re-calculate here to avoid | ||
| // such discrepancy. There is no problem for probe-based profile, as the | ||
| // TotalSamples is exactly the same as the accumulated body samples. | ||
| for (const auto &I : FSamples.getBodySamples()) | ||
| TotalBodySamples += I.second.getSamples(); | ||
|
|
||
| for (const auto &CallsiteSamples : FSamples.getCallsiteSamples()) | ||
| for (const auto &Callee : CallsiteSamples.second) { | ||
| // For binary-level density, the inlinees' samples and size should be | ||
| // included in the calculation. | ||
| calculateBodySamplesAndSize(Callee.second, TotalBodySamples, | ||
| FuncBodySize); | ||
| } | ||
| } | ||
|
|
||
| // Calculate Profile-density: | ||
| // Calculate the density for each function and sort them in descending order, | ||
| // keep accumulating their total samples unitl it exceeds the | ||
| // percentage_threshold(cut-off) of total profile samples, the profile-density | ||
| // is the last(minimum) function-density of the processed functions, which means | ||
| // all the functions hot to perf are on good density if the profile-density is | ||
| // good. The percentage_threshold(--profile-density-cutoff-hot) is configurable | ||
| // depending on how much regression the system want to tolerate. | ||
| double | ||
| ProfileGeneratorBase::calculateDensity(const SampleProfileMap &Profiles) { | ||
| double ProfileDensity = 0.0; | ||
|
|
||
| uint64_t TotalProfileSamples = 0; | ||
| // A list of the function profile density and its total samples. | ||
| std::vector<std::pair<double, uint64_t>> FuncDensityList; | ||
| for (const auto &I : Profiles) { | ||
| uint64_t TotalBodySamples = 0; | ||
| uint64_t FuncBodySize = 0; | ||
| calculateBodySamplesAndSize(I.second, TotalBodySamples, FuncBodySize); | ||
|
|
||
| if (FuncBodySize == 0) | ||
| continue; | ||
|
|
||
| double FuncDensity = static_cast<double>(TotalBodySamples) / FuncBodySize; | ||
| TotalProfileSamples += TotalBodySamples; | ||
| FuncDensityList.emplace_back(FuncDensity, TotalBodySamples); | ||
| } | ||
|
|
||
| // Sorted by the density in descending order. | ||
| llvm::stable_sort(FuncDensityList, [&](const std::pair<double, uint64_t> &A, | ||
| const std::pair<double, uint64_t> &B) { | ||
| if (A.first != B.first) | ||
| return A.first > B.first; | ||
| return A.second < B.second; | ||
| }); | ||
|
|
||
| uint64_t AccumulatedSamples = 0; | ||
| uint32_t I = 0; | ||
| assert(ProfileDensityCutOffHot <= 1000000 && | ||
| "The cutoff value is greater than 1000000(100%)"); | ||
| while (AccumulatedSamples < TotalProfileSamples * | ||
| static_cast<float>(ProfileDensityCutOffHot) / | ||
| 1000000 && | ||
| I < FuncDensityList.size()) { | ||
| AccumulatedSamples += FuncDensityList[I].second; | ||
| ProfileDensity = FuncDensityList[I].first; | ||
| I++; | ||
| } | ||
|
|
||
| return ProfileDensity; | ||
| } | ||
|
|
||
| void ProfileGeneratorBase::calculateAndShowDensity( | ||
| const SampleProfileMap &Profiles) { | ||
| double Density = calculateDensity(Profiles); | ||
| showDensitySuggestion(Density); | ||
| } | ||
|
|
||
|
|
@@ -1057,17 +1122,13 @@ void CSProfileGenerator::postProcessProfiles() { | |
| CSProfMaxColdContextDepth, EnableCSPreInliner); | ||
| } | ||
|
|
||
| if (GenCSNestedProfile) { | ||
| ProfileConverter CSConverter(ProfileMap); | ||
| CSConverter.convertCSProfiles(); | ||
| FunctionSamples::ProfileIsCS = false; | ||
| } | ||
| filterAmbiguousProfile(ProfileMap); | ||
| ProfileGeneratorBase::calculateAndShowDensity(ProfileMap); | ||
| } | ||
|
|
||
| void ProfileGeneratorBase::computeSummaryAndThreshold( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was there no test for suggestion? should we add one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the suggestion for CS profile (There was one for the non-CS profile)