[PATCH] D92621: [SampleFDO] Store fixed length MD5 in NameTable instead of using ULEB128 if MD5 is used.

Mon Dec 7 20:54:14 PST 2020

wenlei accepted this revision.
wenlei added a comment.

In D92621#2437658 <https://reviews.llvm.org/D92621#2437658>, @wmi wrote:

> In D92621#2435924 <https://reviews.llvm.org/D92621#2435924>, @wenlei wrote:
>
>>> We find that after this change, the elapsed time to build a large application distributively is reduced by 5% and the accumulative cpu time used for building is also reduced by 5%.
>>
>> This is great, thanks for the patch! Just to clarify, did you mean 5% of end to end ThinLTO+AutoFDO build? I'm curious what is the total profile reading time your saw in terms of %? and how different that % is between md5 profile vs non-md5 profile? Currently we mostly use non-md5 profile, assuming md5 profile is more about saving profile size.
>
> Yes, it is 5% saving of end to end ThinLTO + AutoFDO build time.
>
> Each module compiling has different % of profile reading time. The percentage is negligible for large source module but can be significant for small source module. I tried it on a very small file.
>
>   The build time using md5 profile was 0.55 second. 
>   The build time using non-md5 profile was 0.75 second. 
>   The build time using fixlenmd5 profile was 0.15 second. 
>   The build time not using any profile was 0.1 second. 
>
> Because for a large project, many source files are small or medium sizes so the building time for small files matters especially for build cpu resource. It also matters for end-to-end build time but usually not as significant. For the experiment I did (a 20M binary profile), the end-to-end build time saving was about the same as the aggregate build cpu resource saving (both were 5%), but from our experience using larger profile (like 300M profile), it will affect aggregate build cpu resource more than end-to-end build time.

Thanks for sharing those numbers, and it's good evidence that the name string in profile can be non-trivial for build time. I think CSSPGO may need to mitigate the overhead introduced by long names - we will get to that.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92621/new/

https://reviews.llvm.org/D92621