This is an archive of the discontinued LLVM Phabricator instance.

[llvm-profgen] Compute and show profile density
ClosedPublic

Authored by wlei on Nov 12 2021, 9:57 AM.

Details

Summary

AutoFDO performance is sensitive to profile density, i.e., the amount of samples in the profile relative to the program size, because profiles with insufficient samples could be inaccurate due to statistical noise and thus hurt AutoFDO performance. A previous investigation showed that AutoFDO performed better on MySQL with increased amount of samples. Therefore, we implement a profile-density computation feature to give hints about profile density to users and the compiler.

We define the density of a profile Prof as follows:

  • For each function A in the profile, density(A) = total_samples(A) / sizeof(A).
  • density(Prof) = min(density(A)) for all functions A that are warm (defined below).

A function is considered warm if its total-samples is within top N percent of the profile. For implementation, we reuse the ProfileSummaryBuilder::getHotCountThreshold(..) as threshold which can be set by percent(--profile-summary-cutoff-hot) or by value(--profile-summary-hot-count).

We also introduce --hot-function-density-threshold to set hot function density threshold and will give suggestion if profile density is below it which implies we should increase samples.

This also applies for CS profile with all profiles merged into base.

Diff Detail

Event Timeline

wlei created this revision.Nov 12 2021, 9:57 AM
wlei requested review of this revision.Nov 12 2021, 9:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2021, 9:57 AM
wlei retitled this revision from [llvm-profgen] density analysis to [llvm-profgen] Compute and show profile density.Nov 12 2021, 10:31 AM
wlei edited the summary of this revision. (Show Details)
wlei added reviewers: hoy, wenlei.
wenlei added inline comments.Nov 23 2021, 5:50 PM
llvm/test/tools/llvm-profgen/inline-cs-pseudoprobe.test
23–24 ↗(On Diff #386875)

I suggest we use a dedicated test case instead of adding feature test into various existing test cases. That way it helps keep testing isolated separated.

llvm/tools/llvm-profgen/ProfileGenerator.cpp
143–149

Maybe these two messages should be warning?

735

Perhaps we can let calculateAndShowDensity take a merged profile map, then calculateAndShowDensity doesn't have to be a virtual function and can have full implementation in ProfileGeneratorBase. We can prepare merged profiles here and pass into calculateAndShowDensity.

llvm/tools/llvm-profgen/ProfileGenerator.h
88

Can we omit raw_fd_ostream &OS as a parameter and use outs() directly inside?

With that, can we also remove the include for raw_os_ostream.h?

llvm/tools/llvm-profgen/ProfiledBinary.h
80

nit: we've been using uint64_t for size. would be good to be consistent, even though they're the same thing here.

wlei updated this revision to Diff 390259.Nov 28 2021, 11:36 PM
wlei edited the summary of this revision. (Show Details)

Addressing Wenlei's feedback

llvm/test/tools/llvm-profgen/inline-cs-pseudoprobe.test
23–24 ↗(On Diff #386875)

Sounds good!

llvm/tools/llvm-profgen/ProfileGenerator.cpp
143–149

Good point!

735

Changed.

llvm/tools/llvm-profgen/ProfileGenerator.h
88

Sounds good!

llvm/tools/llvm-profgen/ProfiledBinary.h
80

Good to know this!

hoy accepted this revision.Nov 29 2021, 11:05 AM

lgtm, thx.

This revision is now accepted and ready to land.Nov 29 2021, 11:05 AM
wenlei accepted this revision.Nov 29 2021, 7:42 PM

lgtm, thanks.

This revision was automatically updated to reflect the committed changes.