<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 4, 2015 at 3:57 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">><br>
> I think it is reasonable to simply replace the key we currently use with<br>
> MD5(key) for getting a size reduction.  In practice for my use cases, I have<br>
> not observed any of the issues you mentioned under "Large size of overhead<br>
> can limit the usability of PGO greatly", but I can understand that some of<br>
> these issues could become problems in Google's use case. I would personally<br>
> prefer to keep the existing behavior as the default (see below), and have<br>
> MD5(key) as an option.<br>
<br>
</span>The problem is that this requires profile format changes.</blockquote><div><br></div><div>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.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> It will be<br>
very messy to support multiple formats in instr-codegen and<br>
instr-runtime.  For compatibility concerns, the reader is taught to<br>
support previous format, but the changes there are isolated (also<br>
expected to be removed in the future).<br>
<span class=""><br>
><br>
> My primary concern is that if the function name are not kept at all stages,<br>
> then it becomes difficult to analyze the profile data in a standalone way.<br>
> Many times, I have used `llvm-profdata show -all-functions foo.profdata` on<br>
> the resulting profile data and then imported that data into Mathematica for<br>
> analysis.<br>
<br>
</span>This is certainly a very valid use case.<br>
<span class=""><br>
>My understanding of your proposal is that `llvm-profdata show<br>
> -all-functions foo.profdata` will not show the actual function names but<br>
> instead MD5 hashes,<br>
<br>
</span>Yes.<br>
<br>
To support your use case, there are two solutions:<br>
<br>
1) user can add -fcoverage-mapping option in the build<br>
2) introduce a new option -fprofile-instr-names that force the<br>
emission of the name sections in the .o file. This is similar to 1),<br>
but no covmap section is needed.<br>
<br>
llvm-profdata tool  will be taught to read the name section and attach<br>
function names to the profile records.<br></blockquote><div><br></div><div>Needing to pass the executable to llvm-profdata would cause deployment issues for my customers in practice.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Note that with 1) or 2), the user can still benefit from the reduced<br>
profile size.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
thanks,<br>
<br>
David<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<br>
>which will make it more difficult for me to do this kind<br>
> of analysis (would require using nm on the original binary, hashing<br>
> everything, etc.).<br>
><br>
> btw, feel free to attach the patch even if it in a rough state. It can still<br>
> help to clarify the proposal and be a good talking point. Fine-grained patch<br>
> review for caring about the rough parts will happen on llvm-commits; the<br>
> rough parts will not distract the discussion here on llvm-dev.<br>
><br>
> -- Sean Silva<br>
><br>
>><br>
>><br>
>> thanks,<br>
>><br>
>> David<br>
>> _______________________________________________<br>
>> LLVM Developers mailing list<br>
>> <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>