<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Nov 12, 2015 at 3:56 PM, Betul Buyukkurt <span dir="ltr"><<a href="mailto:betulb@codeaurora.org" target="_blank">betulb@codeaurora.org</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>
><br>
> On Thu, Nov 12, 2015 at 10:12 AM, Betul Buyukkurt <<a href="mailto:betulb@codeaurora.org">betulb@codeaurora.org</a><br>
</span><div><div class="h5">> <mailto:<a href="mailto:betulb@codeaurora.org">betulb@codeaurora.org</a>> > wrote:<br>
><br>
><br>
>> davidxl added inline comments.<br>
>><br>
>> ================<br>
>> Comment at: lib/profile/InstrProfiling.c:190<br>
>> @@ +189,3 @@<br>
>> +    }<br>
>> +    I->ValueCounters = (void *)PerSiteCountsHead;<br>
>> +    PerSiteCountsHead = (uint8_t *)VDataPtr;<br>
>> ----------------<br>
>> Release memory of ValueCounters ?<br>
><br>
> This is a function that's running only atexit, I thought of adding it in<br>
> but then thought of it as unnecessary too due to this being one of the<br>
> very last actions of the run time during the lifetime of the program.<br>
><br>
><br>
><br>
><br>
> Ok to leave it as it is, but the assumption may not be true in the future.<br>
><br>
> Consider the case when profile dump is not done at exit (e.g. for a server<br>
> program). APIs are provided for the user to periodically dump data for<br>
> selected program phases they care about. One low overhead way of doing<br>
> this<br>
> is let the program continuously do the profiling without the need to check<br>
> the state. APIs are provided so that user can do counter clearing and<br>
> dumping at the start and end of the phase ...<br>
><br>
> Another thing we need to solve in the future is support raw profile data<br>
> on-line merging -- otherwise the raw profile data can get really big with<br>
> the current appending model. Without this, doing clang bootstrap with PGO<br>
> (4<br>
> stage with clang/llvm sources as training data) will be impossible.<br>
><br>
> VP data certainly makes it trickier.<br>
<br>
</div></div>The reason we're updating the ValueCounters pointer in the<br>
__llvm_profile_data is solely for code uniformity in accessing the members<br>
of the __llvm_profile_data. Yet it's not as conforming to the way the<br>
NamePtr or the CounterPtr members of the struct are processed. To support<br>
the cases you mention above, one needs to introduce also a lock to be able<br>
to write out the data and prevent the value profiling from proceeding<br>
during the data write out. I think all of this can be avoided by not<br>
overwriting the ValueCounters pointer.<br></blockquote><div><br></div><div>The tricky part is not the ValueCounters pointer update, but the acutal VP data merging -- because each site may have different number of targets and order.   If in-memory data has the same size of in-disk file, the online merge becomes trivial.   It is easily doable though requires more memory.</div><div><br></div><div>Your compiler-runtime change looks good to me.  Thanks for your great patience! Once the base version is in, the community can also start contribute to it ..</div><div><br></div><div>thanks,</div><div><br></div><div>David</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Betul<br>
<br>
> David<br>
><br>
><br>
><br>
><br>
>><br>
>> <a href="http://reviews.llvm.org/D9009" rel="noreferrer" target="_blank">http://reviews.llvm.org/D9009</a><br>
>><br>
>><br>
>><br>
>><br>
><br>
><br>
><br>
><br>
><br>
><br>
<br>
<br>
</blockquote></div><br></div></div>