[llvm-dev] RFC: Reducing Instr PGO size overhead

[Sean Silva via llvm-dev]

On Fri, Sep 4, 2015 at 3:57 PM, Xinliang David Li <davidxl at google.com>
wrote:

> >
> > I think it is reasonable to simply replace the key we currently use with
> > MD5(key) for getting a size reduction.  In practice for my use cases, I
> have
> > not observed any of the issues you mentioned under "Large size of
> overhead
> > can limit the usability of PGO greatly", but I can understand that some
> of
> > these issues could become problems in Google's use case. I would
> personally
> > prefer to keep the existing behavior as the default (see below), and have
> > MD5(key) as an option.
>
> The problem is that this requires profile format changes.


Why? AFAIK the "function name" is just an arbitrary string. Using s or
MD5(s) shouldn't matter. Of course, the user will need to pass consistent
flags to clang.



> It will be
> very messy to support multiple formats in instr-codegen and
> instr-runtime.  For compatibility concerns, the reader is taught to
> support previous format, but the changes there are isolated (also
> expected to be removed in the future).
>
> >
> > My primary concern is that if the function name are not kept at all
> stages,
> > then it becomes difficult to analyze the profile data in a standalone
> way.
> > Many times, I have used `llvm-profdata show -all-functions foo.profdata`
> on
> > the resulting profile data and then imported that data into Mathematica
> for
> > analysis.
>
> This is certainly a very valid use case.
>
> >My understanding of your proposal is that `llvm-profdata show
> > -all-functions foo.profdata` will not show the actual function names but
> > instead MD5 hashes,
>
> Yes.
>
> To support your use case, there are two solutions:
>
> 1) user can add -fcoverage-mapping option in the build
> 2) introduce a new option -fprofile-instr-names that force the
> emission of the name sections in the .o file. This is similar to 1),
> but no covmap section is needed.
>
> llvm-profdata tool  will be taught to read the name section and attach
> function names to the profile records.
>

Needing to pass the executable to llvm-profdata would cause deployment
issues for my customers in practice.


>
> Note that with 1) or 2), the user can still benefit from the reduced
> profile size.
>

Let me reiterate that the size of the profile is not a problem I have
observed in practice (nor have I heard of this being a problem in practice
until this thread). Therefore I'm skeptical of any changes to our default
behavior or any new requirements that are not opt-in.

-- Sean Silva


>
> thanks,
>
> David
>
>
>
>
> >which will make it more difficult for me to do this kind
> > of analysis (would require using nm on the original binary, hashing
> > everything, etc.).
> >
> > btw, feel free to attach the patch even if it in a rough state. It can
> still
> > help to clarify the proposal and be a good talking point. Fine-grained
> patch
> > review for caring about the rough parts will happen on llvm-commits; the
> > rough parts will not distract the discussion here on llvm-dev.
> >
> > -- Sean Silva
> >
> >>
> >>
> >> thanks,
> >>
> >> David
> >> _______________________________________________
> >> LLVM Developers mailing list
> >> llvm-dev at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150904/a5f30100/attachment.html>