[PATCH] D147812: [InstrProf] Use BalancedPartitioning to order temporal profiling trace data

Thu Apr 13 16:30:28 PDT 2023

ellis added a comment.

In D147812#4259507 <https://reviews.llvm.org/D147812#4259507>, @davidxl wrote:

> In D147812#4259461 <https://reviews.llvm.org/D147812#4259461>, @ellis wrote:
>
>> In D147812#4259190 <https://reviews.llvm.org/D147812#4259190>, @davidxl wrote:
>>
>>> A few high level questions:
>>>
>>> Different types of traces don't have the same frequency, so it might be useful to support weighting. The frequency certain trace pattern appear in the profile data does not necessarily match to their frequency in real world usage. To support this, some kind of symbolic id may be needed to annotate the trace data.
>>
>> I see what you are saying. We could have a set of raw profiles collected under type "A" conditions and another set under type "B" conditions. Maybe type "A" is common while type "B" is more rare so we'd like to weight "A"s traces more than "B"s.
>>
>> This could be implemented with an extra "weight" field in the trace data for each trace. It hasn't been long since I landed https://reviews.llvm.org/D147287. Do you think it makes sense to land a patch to add this field without updating the version and supporting two trace formats?
>
> It should be fine -- since there is no ordering tool/support (like this patch does) that enable folks to use the feature yet.
>
>> The other option is to have the `llvm-profdata merge` command duplicate traces with extra weight. Of course this reduces the data density so we couldn't store as many profiles.
>>
>>> The cost metric is entropy like. Have you tried other metrics such as gini impurity?
>>
>> @spupyrev @jmestre Do you have any thoughts on this?

I've added https://reviews.llvm.org/rG4bddef4117403a305727d145a9abf6bda700f8ff but I'm not using the weights yet. I haven't decided the best way to use them yet, and I haven't decided if I'll do that here or in a followup diff.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147812