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

Namhyung Kim posted 6 patches 8 months, 1 week ago
[PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display
Posted by Namhyung Kim 8 months, 1 week ago
Support data type display with a key press so that users can toggle the
output dynamically on TUI.

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
Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display
Posted by Arnaldo Carvalho de Melo 8 months, 1 week ago
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,

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
>
Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display
Posted by Namhyung Kim 8 months, 1 week ago
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,

Maybe 'T'.  I thought 'y' won't be used by others later but it may be
used by 'y/n' questions.

> 
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks!
Namhyung

>  
> > 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
> >
Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display
Posted by Arnaldo Carvalho de Melo 8 months, 1 week ago
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>

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.

- 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
> >
Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display
Posted by Namhyung Kim 8 months, 1 week ago
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
> > >