Re: [PATCH v5] perf tools c2c: Add annotation support to perf c2c report

[Li, Tianyou]

Hi Ravi,


Very appreciated for your testing. Could I add you to the 'Tested-by' 
for my next revision?


I am working on the annotate changes. Hope I can get your attention 
again in the next revision. Thanks.


Regards,

Tianyou


On 10/6/2025 6:54 PM, Ravi Bangoria wrote:
> Hi Tianyou,
>
> I tested this on an AMD machine and it's working fine. Few minor issues
> I would suggest to address.
>
> > > Perf c2c report currently specified the code address and source:line
> > > information in the cacheline browser, while it is lack of annotation
> > > support like perf report to directly show the disassembly code for
> > > the particular symbol shared that same cacheline. This patches add
> > > a key 'a' binding to the cacheline browser which reuse the annotation
> > > browser to show the disassembly view for easier analysis of cacheline
> > > contentions. By default, the 'TAB' key navigate to the code address
> > > where the contentions detected.
> 1. Annotate browser title always shows "Samples: 0", which is misleading:
>
>     Samples: 0  of event 'ibs_op//', 100000 Hz, Event count (approx.): 0
>              ^                                                         ^
>               `--- this                                                 `--- and this
>
> 2. Comment from v3:
>
>     > In the annotation browser, we did not need to show the overhead of
>     > particular event type, or switch among events. The only selection
>     > was set to the ip addr where the hist entry indicated in the
>     > cacheline browser. The hist_entry was correctly initialized with
>     > .ms field by addr_location when the mem_info can be successfully
>     > resolved through addr_location.
>
>     It's ideal if we can show sample count/overhead along with the
>     instruction (because that's what users are used to seeing with
>     perf-report->annotate output). I haven't checked the code but is
>     it possible to highlight the instruction by default (Annotate+TAB
>     OR Set the font color to red etc.)? Waiting for user to press the
>     TAB seems counterintuitive esp when instructions are not tagged
>     with overhead.
>
>          |1340: > callq   0x10f0 <_init+0xf0>
>          |1345: v jmp     0x1376 <thread+0x12d>
>          |1347:   cmpl    $0x1, -0xb0(%rbp)
>          |134e: v jne     0x1364 <thread+0x11b>
>          |1350:   movq    0x2d29(%rip), %rax  # 0x4080 <data>   <=====  Highlight it by default
>          |1357:   addq    $0x1, %rax
>          |135b:   movq    %rax, 0x2d1e(%rip)  # 0x4080 <data>
>          |1362: v jmp     0x1376 <thread+0x12d>
>
> These are not blockers if Namhyung / Arnaldo are okay with current design.
>
> Thanks,
> Ravi