Skip to content

Commit 2598aa6

Browse files
authored
[CSSPGO] Reject high checksum mismatched profile (#84097)
Error out the build if the checksum mismatch is extremely high, it's better to drop the profile rather than apply the bad profile. Note that the check is on a module level, the user could make big changes to functions in one single module but those changes might not be performance significant to the whole binary, so we want to be conservative, only expect to catch big perf regression. To do this, we select a set of the "hot" functions for the check. We use two parameter(`hot-func-cutoff-for-staleness-error` and `min-functions-for-staleness-error`) to control the function selection to make sure the selected are hot enough and the num of function is not small. Tuned the parameters on our internal services, it works to catch big perf regression due to the high mismatch .
1 parent 52431fd commit 2598aa6

File tree

2 files changed

+78
-0
lines changed

2 files changed

+78
-0
lines changed

llvm/lib/Transforms/IPO/SampleProfile.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,21 @@ static cl::opt<unsigned> ProfileICPRelativeHotnessSkip(
234234
cl::desc(
235235
"Skip relative hotness check for ICP up to given number of targets."));
236236

237+
static cl::opt<unsigned> HotFuncCutoffForStalenessError(
238+
"hot-func-cutoff-for-staleness-error", cl::Hidden, cl::init(800000),
239+
cl::desc("A function is considered hot for staleness error check if its "
240+
"total sample count is above the specified percentile"));
241+
242+
static cl::opt<unsigned> MinfuncsForStalenessError(
243+
"min-functions-for-staleness-error", cl::Hidden, cl::init(50),
244+
cl::desc("Skip the check if the number of hot functions is smaller than "
245+
"the specified number."));
246+
247+
static cl::opt<unsigned> PrecentMismatchForStalenessError(
248+
"precent-mismatch-for-staleness-error", cl::Hidden, cl::init(80),
249+
cl::desc("Reject the profile if the mismatch percent is higher than the "
250+
"given number."));
251+
237252
static cl::opt<bool> CallsitePrioritizedInline(
238253
"sample-profile-prioritized-inline", cl::Hidden,
239254

@@ -630,6 +645,8 @@ class SampleProfileLoader final : public SampleProfileLoaderBaseImpl<Function> {
630645
std::vector<Function *> buildFunctionOrder(Module &M, LazyCallGraph &CG);
631646
std::unique_ptr<ProfiledCallGraph> buildProfiledCallGraph(Module &M);
632647
void generateMDProfMetadata(Function &F);
648+
bool rejectHighStalenessProfile(Module &M, ProfileSummaryInfo *PSI,
649+
const SampleProfileMap &Profiles);
633650

634651
/// Map from function name to Function *. Used to find the function from
635652
/// the function name. If the function name contains suffix, additional
@@ -2187,6 +2204,55 @@ bool SampleProfileLoader::doInitialization(Module &M,
21872204
return true;
21882205
}
21892206

2207+
// Note that this is a module-level check. Even if one module is errored out,
2208+
// the entire build will be errored out. However, the user could make big
2209+
// changes to functions in single module but those changes might not be
2210+
// performance significant to the whole binary. Therefore, to avoid those false
2211+
// positives, we select a reasonable big set of hot functions that are supposed
2212+
// to be globally performance significant, only compute and check the mismatch
2213+
// within those functions. The function selection is based on two criteria:
2214+
// 1) The function is hot enough, which is tuned by a hotness-based
2215+
// flag(HotFuncCutoffForStalenessError). 2) The num of function is large enough
2216+
// which is tuned by the MinfuncsForStalenessError flag.
2217+
bool SampleProfileLoader::rejectHighStalenessProfile(
2218+
Module &M, ProfileSummaryInfo *PSI, const SampleProfileMap &Profiles) {
2219+
assert(FunctionSamples::ProfileIsProbeBased &&
2220+
"Only support for probe-based profile");
2221+
uint64_t TotalHotFunc = 0;
2222+
uint64_t NumMismatchedFunc = 0;
2223+
for (const auto &I : Profiles) {
2224+
const auto &FS = I.second;
2225+
const auto *FuncDesc = ProbeManager->getDesc(FS.getGUID());
2226+
if (!FuncDesc)
2227+
continue;
2228+
2229+
// Use a hotness-based threshold to control the function selection.
2230+
if (!PSI->isHotCountNthPercentile(HotFuncCutoffForStalenessError,
2231+
FS.getTotalSamples()))
2232+
continue;
2233+
2234+
TotalHotFunc++;
2235+
if (ProbeManager->profileIsHashMismatched(*FuncDesc, FS))
2236+
NumMismatchedFunc++;
2237+
}
2238+
// Make sure that the num of selected function is not too small to distinguish
2239+
// from the user's benign changes.
2240+
if (TotalHotFunc < MinfuncsForStalenessError)
2241+
return false;
2242+
2243+
// Finally check the mismatch percentage against the threshold.
2244+
if (NumMismatchedFunc * 100 >=
2245+
TotalHotFunc * PrecentMismatchForStalenessError) {
2246+
auto &Ctx = M.getContext();
2247+
const char *Msg =
2248+
"The input profile significantly mismatches current source code. "
2249+
"Please recollect profile to avoid performance regression.";
2250+
Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg));
2251+
return true;
2252+
}
2253+
return false;
2254+
}
2255+
21902256
void SampleProfileMatcher::findIRAnchors(
21912257
const Function &F, std::map<LineLocation, StringRef> &IRAnchors) {
21922258
// For inlined code, recover the original callsite and callee by finding the
@@ -2718,6 +2784,11 @@ bool SampleProfileLoader::runOnModule(Module &M, ModuleAnalysisManager *AM,
27182784
ProfileSummary::PSK_Sample);
27192785
PSI->refresh();
27202786
}
2787+
2788+
if (FunctionSamples::ProfileIsProbeBased &&
2789+
rejectHighStalenessProfile(M, PSI, Reader->getProfiles()))
2790+
return false;
2791+
27212792
// Compute the total number of samples collected in this profile.
27222793
for (const auto &I : Reader->getProfiles())
27232794
TotalCollectedSamples += I.second.getTotalSamples();
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
; REQUIRES: x86_64-linux
2+
; RUN: not opt < %S/pseudo-probe-profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -min-functions-for-staleness-error=1 -precent-mismatch-for-staleness-error=1 -S 2>&1 | FileCheck %s
3+
; RUN: opt < %S/pseudo-probe-profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -min-functions-for-staleness-error=3 -precent-mismatch-for-staleness-error=70 -S 2>&1
4+
; RUN: opt < %S/pseudo-probe-profile-mismatch.ll -passes=sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile-mismatch.prof -min-functions-for-staleness-error=4 -precent-mismatch-for-staleness-error=1 -S 2>&1
5+
6+
7+
; CHECK: error: {{.*}}: The input profile significantly mismatches current source code. Please recollect profile to avoid performance regression.

0 commit comments

Comments
 (0)