[PATCH] D90125: [CSSPGO] Infrastructure for context-sensitive Sample PGO and Inlining

[Wei Mi via llvm-commits]

On Thu, Nov 19, 2020 at 2:59 PM Wenlei He via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> wenlei added inline comments.
>
>
> ================
> Comment at: llvm/include/llvm/ProfileData/SampleProf.h:504-506
> +      // For CSFDO, in order to conserve profile size, we no longer write out
> +      // locations profile for those not hit during training, so we need to
> +      // treat them as zero instead of error here.
> ----------------
> wenlei wrote:
> > wmi wrote:
> > > davidxl wrote:
> > > > CSFDO ==> CSSPGO
> > > It means CSSPGO will treat all the new lines as cold, even if some of them may be inferred from other parts of the profile. How much extra size is needed if zero is emitted?
> > Knowing that CS profile will be much bigger, we started with trimming zero counts trying to save size as much as we can. But I don't actually have the data at hand. Let me see if I can get some data on this.
> >
> > New lines will be less of a problem for pseudo-probe if they don't change CFG.
> >
> I took a look at current profile generation tool. It requires some extra work to fill in zeros for not sample lines for CSSPGO. I collected AutoFDO for mysql w/ and w/o zeros filled, here's the size difference. CSSPGO is likely to see similar or bigger relative size difference (profile for context can be more sparse).
>
> w/ zero filled for unsampled lines: 3.9M
> w/o zero filled for unsampled lines: 1.4M
>

Thanks for the data. That matches my expectation. It will be
interesting to know the performance impact for both CSSPGO and current
SPGO. For example, we can have some stale profile with and without
zero filled for unsampled lines. Coupled with needed compiler change,
we can compare whether the zeros are necessary for performance,
especially under the source drift circumstance. If not, it can bring
some substantial profile size saving and build time reduction for
current SPGO.

>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D90125/new/
>
> https://reviews.llvm.org/D90125
>