Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Jun 05, 2025 at 06:12:38PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Jun 05, 2025 at 05:46:22PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> > > Support data type display with a key press so that users can toggle the
> > > output dynamically on TUI.
> > 
> > 'd', 'T', control+d, control+t are all available, why 'y'? :-)
> > 
> > Apart from these minor nits,
> 
> There is one issue I noticed now that Ingo mentioned earlier, if you
> press 'y' to get Data type display we better add some visual cue that
> this is toggled, like was done for source code view in:
> 
> commit c6043d35c0f3eb5bcbeb6309e10c4ae33d203841
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Tue Apr 8 19:03:43 2025 -0300
> 
>     perf ui browser annotate: Show in the title the source code view toggle
>     
>     Ingo reported that having a visual cue if the source code view is
>     enabled will help in noticing a bug when no source is presented.
>     
>     Change the title scnprintf routine for the annotation browser to do
>     that.
>     
>     Suggested-by: Ingo Molnar <mingo@kernel.org>

Yep, I'll do that as well.

> 
> Also I did it with a simple perf.data file, no data stuff in it,
> probably it is best to show that if what is required for it to work is
> present in the perf.data file.
> 
> Or to tell what is needed for that hotkey/feature to work, which I think
> is even better and more informative.

Sure, I'll add a message that the dso has no DWARF info.

Thanks,
Namhyung

> 
> - Arnaldo
>  
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > index 686fa54e545e275c..cd1d452035a265d3 100644
> > > --- a/tools/perf/ui/browsers/annotate.c
> > > +++ b/tools/perf/ui/browsers/annotate.c
> > > @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > >  		"b             Toggle percent base [period/hits]\n"
> > >  		"B             Branch counter abbr list (Optional)\n"
> > >  		"?             Search string backwards\n"
> > > -		"f             Toggle showing offsets to full address\n");
> > > +		"f             Toggle showing offsets to full address\n"
> > > +		"y             Toggle data type display\n");
> > >  			continue;
> > >  		case 'r':
> > >  			script_browse(NULL, NULL);
> > > @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > >  		case 'f':
> > >  			annotation__toggle_full_addr(notes, ms);
> > >  			continue;
> > > +		case 'y':
> > > +			annotate_opts.code_with_type ^= 1;
> > > +			if (browser->dbg == NULL)
> > > +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> > > +			continue;
> > >  		case K_LEFT:
> > >  		case '<':
> > >  		case '>':
> > > @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > >  
> > >  	ret = annotate_browser__run(&browser, evsel, hbt);
> > >  
> > > -	if (annotate_opts.code_with_type)
> > > -		debuginfo__delete(browser.dbg);
> > > +	debuginfo__delete(browser.dbg);
> > >  	if(not_annotated)
> > >  		annotated_source__purge(notes->src);
> > >  
> > > -- 
> > > 2.49.0
> > >