[v2,0/2] perf tools: annotation browser from c2c tui

  These patches add ability to start annotation browser from c2c report
tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].

[1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/

v1->v2: Addressed comments from Namhyung Kim
- No longer saving evsel for each hist entry, using evlist__first
  instead.
- Factored out preparations to call annotation browser to do_annotate()
  function
- Other small fixes and adjustments.

Artem Savkov (2):
  perf util: move symbol__new_unresolved() to util/symbol.c
  perf tools: allow running annotation browser from c2c-report

 tools/perf/builtin-c2c.c       | 73 +++++++++++++++++++++++++++++++---
 tools/perf/ui/browsers/hists.c | 22 ----------
 tools/perf/util/symbol.c       | 22 ++++++++++
 tools/perf/util/symbol.h       |  1 +
 4 files changed, 91 insertions(+), 27 deletions(-)
  

On Thu, Jun 08, 2023 at 10:44:05AM +0200, Artem Savkov wrote:
> These patches add ability to start annotation browser from c2c report
> tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> 
> [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/

So what are the plans for doing IP to datatype::member resolution and
deleting this C2C abomination?

/me still regretting ever letting c2c happen..
  

Hi Peter,

On Thu, Jun 8, 2023 at 6:25 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Jun 08, 2023 at 10:44:05AM +0200, Artem Savkov wrote:
> > These patches add ability to start annotation browser from c2c report
> > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> >
> > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
>
> So what are the plans for doing IP to datatype::member resolution and
> deleting this C2C abomination?

I'm working on it and hoping to post an RFC soonish.. but no plan to kill
c2c entirely :)

Thanks,
Namhyung

>
> /me still regretting ever letting c2c happen..
  

Hello,

On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
>
> These patches add ability to start annotation browser from c2c report
> tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].

I was thinking about how it works and realized that it didn't collect
samples by symbol.  Then I'm not sure if the result is meaningful.
I think it'd show a random symbol that touched the cache line
first.  The same cache line can be accessed from other locations
but it cannot know where they are.

Also different instructions in a function (symbol) would access a
different cache line.  The annotate output just shows any memory
access.  So it might be good to check the instruction at the point
but others should not be considered related.

Hmm.. I suspect even the same instruction will hit the different
cache lines at different times.  Then probably the annotation
won't work well in terms of correlating cache lines.

Thanks,
Namhyung

>
> [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
>
> v1->v2: Addressed comments from Namhyung Kim
> - No longer saving evsel for each hist entry, using evlist__first
>   instead.
> - Factored out preparations to call annotation browser to do_annotate()
>   function
> - Other small fixes and adjustments.
>
> Artem Savkov (2):
>   perf util: move symbol__new_unresolved() to util/symbol.c
>   perf tools: allow running annotation browser from c2c-report
>
>  tools/perf/builtin-c2c.c       | 73 +++++++++++++++++++++++++++++++---
>  tools/perf/ui/browsers/hists.c | 22 ----------
>  tools/perf/util/symbol.c       | 22 ++++++++++
>  tools/perf/util/symbol.h       |  1 +
>  4 files changed, 91 insertions(+), 27 deletions(-)
>
> --
> 2.40.1
>
  

On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> Hello,
> 
> On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
> >
> > These patches add ability to start annotation browser from c2c report
> > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> 
> I was thinking about how it works and realized that it didn't collect
> samples by symbol.  Then I'm not sure if the result is meaningful.
> I think it'd show a random symbol that touched the cache line
> first.  The same cache line can be accessed from other locations
> but it cannot know where they are.
> 
> Also different instructions in a function (symbol) would access a
> different cache line.  The annotate output just shows any memory
> access.  So it might be good to check the instruction at the point
> but others should not be considered related.
> 
> Hmm.. I suspect even the same instruction will hit the different
> cache lines at different times.  Then probably the annotation
> won't work well in terms of correlating cache lines.

The annotation hotkey is only added to the cacheline detailed view where
we do have symbol instruction information. The idea is to give the user
ability to quickly jump to source code/disassembly directly from c2c
TUI.

The hit percentages in annotation view don't make much sense in this
case though, so maybe it is better to use dummy evsel so that none are
shown.

> Thanks,
> Namhyung
> 
> >
> > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
> >
> > v1->v2: Addressed comments from Namhyung Kim
> > - No longer saving evsel for each hist entry, using evlist__first
> >   instead.
> > - Factored out preparations to call annotation browser to do_annotate()
> >   function
> > - Other small fixes and adjustments.
> >
> > Artem Savkov (2):
> >   perf util: move symbol__new_unresolved() to util/symbol.c
> >   perf tools: allow running annotation browser from c2c-report
> >
> >  tools/perf/builtin-c2c.c       | 73 +++++++++++++++++++++++++++++++---
> >  tools/perf/ui/browsers/hists.c | 22 ----------
> >  tools/perf/util/symbol.c       | 22 ++++++++++
> >  tools/perf/util/symbol.h       |  1 +
> >  4 files changed, 91 insertions(+), 27 deletions(-)
> >
> > --
> > 2.40.1
> >
>
  

Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > Hello,
> > 
> > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
> > >
> > > These patches add ability to start annotation browser from c2c report
> > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> > 
> > I was thinking about how it works and realized that it didn't collect
> > samples by symbol.  Then I'm not sure if the result is meaningful.
> > I think it'd show a random symbol that touched the cache line
> > first.  The same cache line can be accessed from other locations
> > but it cannot know where they are.
> > 
> > Also different instructions in a function (symbol) would access a
> > different cache line.  The annotate output just shows any memory
> > access.  So it might be good to check the instruction at the point
> > but others should not be considered related.
> > 
> > Hmm.. I suspect even the same instruction will hit the different
> > cache lines at different times.  Then probably the annotation
> > won't work well in terms of correlating cache lines.
> 
> The annotation hotkey is only added to the cacheline detailed view where
> we do have symbol instruction information. The idea is to give the user
> ability to quickly jump to source code/disassembly directly from c2c
> TUI.
> 
> The hit percentages in annotation view don't make much sense in this
> case though, so maybe it is better to use dummy evsel so that none are
> shown.

Yes, the point is just to reuse the source browser, if there is no
annotation data applicable, don't use any.

- Arnaldo
 
> > Thanks,
> > Namhyung
> > 
> > >
> > > [1]: http://vger.kernel.org/~acme/prez/linux-plumbers-2022/
> > >
> > > v1->v2: Addressed comments from Namhyung Kim
> > > - No longer saving evsel for each hist entry, using evlist__first
> > >   instead.
> > > - Factored out preparations to call annotation browser to do_annotate()
> > >   function
> > > - Other small fixes and adjustments.
> > >
> > > Artem Savkov (2):
> > >   perf util: move symbol__new_unresolved() to util/symbol.c
> > >   perf tools: allow running annotation browser from c2c-report
> > >
> > >  tools/perf/builtin-c2c.c       | 73 +++++++++++++++++++++++++++++++---
> > >  tools/perf/ui/browsers/hists.c | 22 ----------
> > >  tools/perf/util/symbol.c       | 22 ++++++++++
> > >  tools/perf/util/symbol.h       |  1 +
> > >  4 files changed, 91 insertions(+), 27 deletions(-)
> > >
> > > --
> > > 2.40.1
> > >
> > 
> 
> -- 
>  Artem
>
  

Em Wed, Jun 14, 2023 at 03:38:35PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> > On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
> > > > These patches add ability to start annotation browser from c2c report
> > > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].

> > > I was thinking about how it works and realized that it didn't collect
> > > samples by symbol.  Then I'm not sure if the result is meaningful.
> > > I think it'd show a random symbol that touched the cache line
> > > first.  The same cache line can be accessed from other locations
> > > but it cannot know where they are.

> > > Also different instructions in a function (symbol) would access a
> > > different cache line.  The annotate output just shows any memory
> > > access.  So it might be good to check the instruction at the point
> > > but others should not be considered related.

> > > Hmm.. I suspect even the same instruction will hit the different
> > > cache lines at different times.  Then probably the annotation
> > > won't work well in terms of correlating cache lines.

> > The annotation hotkey is only added to the cacheline detailed view where
> > we do have symbol instruction information. The idea is to give the user
> > ability to quickly jump to source code/disassembly directly from c2c
> > TUI.

> > The hit percentages in annotation view don't make much sense in this
> > case though, so maybe it is better to use dummy evsel so that none are
> > shown.

> Yes, the point is just to reuse the source browser, if there is no
> annotation data applicable, don't use any.

I tried to work on having the annotation browser, both --stdio2 and the
TUI to work with evsels without annotation data, just so that we could
simply navigate source code or disassembly functions using the more
compact form we have in perf than the original one from objdump.

- Arnaldo
  

On Wed, Jun 14, 2023 at 03:38:35PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 09, 2023 at 10:37:31AM +0200, Artem Savkov escreveu:
> > On Thu, Jun 08, 2023 at 02:09:06PM -0700, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > On Thu, Jun 8, 2023 at 1:44 AM Artem Savkov <asavkov@redhat.com> wrote:
> > > >
> > > > These patches add ability to start annotation browser from c2c report
> > > > tui. The idea comes from Arnaldo's "Profiling Data Structures" talk [1].
> > > 
> > > I was thinking about how it works and realized that it didn't collect
> > > samples by symbol.  Then I'm not sure if the result is meaningful.
> > > I think it'd show a random symbol that touched the cache line
> > > first.  The same cache line can be accessed from other locations
> > > but it cannot know where they are.
> > > 
> > > Also different instructions in a function (symbol) would access a
> > > different cache line.  The annotate output just shows any memory
> > > access.  So it might be good to check the instruction at the point
> > > but others should not be considered related.
> > > 
> > > Hmm.. I suspect even the same instruction will hit the different
> > > cache lines at different times.  Then probably the annotation
> > > won't work well in terms of correlating cache lines.
> > 
> > The annotation hotkey is only added to the cacheline detailed view where
> > we do have symbol instruction information. The idea is to give the user
> > ability to quickly jump to source code/disassembly directly from c2c
> > TUI.
> > 
> > The hit percentages in annotation view don't make much sense in this
> > case though, so maybe it is better to use dummy evsel so that none are
> > shown.
> 
> Yes, the point is just to reuse the source browser, if there is no
> annotation data applicable, don't use any.
> 

Ok, should be as easy as the following diff. I'll include it in v3 if
there si more feedback.

---

diff --git b/tools/perf/builtin-c2c.c a/tools/perf/builtin-c2c.c
index dce8604837aec..c856ce7a50740 100644
--- b/tools/perf/builtin-c2c.c
+++ a/tools/perf/builtin-c2c.c
@@ -2701,7 +2701,11 @@ static int perf_c2c__browse_cacheline(struct hist_entry *he)

                switch (key) {
                case 'a':
-                       do_annotate(browser, evlist__first(c2c.evlist));
+                       /*
+                        * We don't need percentage info so use 'dummy:HG'
+                        * evsel which is last in evlist.
+                        */
+                       do_annotate(browser, evlist__last(c2c.evlist));
                        break;
                case 's':
                        c2c.symbol_full = !c2c.symbol_full;