linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf annotate: allow 's' on source code lines
@ 2021-06-21 23:00 Riccardo Mancini
  2021-06-22  5:07 ` Ian Rogers
  0 siblings, 1 reply; 3+ messages in thread
From: Riccardo Mancini @ 2021-06-21 23:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Ian Rogers, Riccardo Mancini, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Martin Liska, Jason Yan, linux-perf-users, linux-kernel

In perf annotate, when 's' is pressed on a line containing
source code, it shows the message "Only available for assembly
lines".
This patch gets rid of the error, moving the cursr to the next
available asm line (or the closest previous one if no asm line
is found moving forwards), before hiding source code lines.

Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
---
 tools/perf/ui/browsers/annotate.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ad0a70f0edaf..eb94d20d0d13 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -343,6 +343,29 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 	browser->curr_hot = rb_last(&browser->entries);
 }
 
+static struct annotation_line *annotate_browser__find_next_asm_line(
+					struct annotate_browser *browser,
+					struct annotation_line *al)
+{
+	struct annotation_line *it = al;
+
+	/* find next asm line */
+	list_for_each_entry_continue(it, browser->b.top, node) {
+		if (it->idx_asm >= 0)
+			return it;
+	}
+
+	/* no asm line found forwards, try backwards */
+	it = al;
+	list_for_each_entry_continue_reverse(it, browser->b.top, node) {
+		if (it->idx_asm >= 0)
+			return it;
+	}
+
+	/* There are no asm lines */
+	return al;
+}
+
 static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 {
 	struct annotation *notes = browser__annotation(&browser->b);
@@ -363,9 +386,8 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 		browser->b.index = al->idx;
 	} else {
 		if (al->idx_asm < 0) {
-			ui_helpline__puts("Only available for assembly lines.");
-			browser->b.seek(&browser->b, -offset, SEEK_CUR);
-			return false;
+			/* move cursor to next asm line */
+			al = annotate_browser__find_next_asm_line(browser, al);
 		}
 
 		if (al->idx_asm < offset)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf annotate: allow 's' on source code lines
  2021-06-21 23:00 [PATCH] perf annotate: allow 's' on source code lines Riccardo Mancini
@ 2021-06-22  5:07 ` Ian Rogers
  2021-06-22  7:58   ` Riccardo Mancini
  0 siblings, 1 reply; 3+ messages in thread
From: Ian Rogers @ 2021-06-22  5:07 UTC (permalink / raw)
  To: Riccardo Mancini
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Martin Liska, Jason Yan, linux-perf-users, linux-kernel

On Mon, Jun 21, 2021 at 4:02 PM Riccardo Mancini <rickyman7@gmail.com> wrote:
>
> In perf annotate, when 's' is pressed on a line containing
> source code, it shows the message "Only available for assembly
> lines".
> This patch gets rid of the error, moving the cursr to the next
> available asm line (or the closest previous one if no asm line
> is found moving forwards), before hiding source code lines.
>
> Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> ---
>  tools/perf/ui/browsers/annotate.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index ad0a70f0edaf..eb94d20d0d13 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -343,6 +343,29 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
>         browser->curr_hot = rb_last(&browser->entries);
>  }
>
> +static struct annotation_line *annotate_browser__find_next_asm_line(
> +                                       struct annotate_browser *browser,
> +                                       struct annotation_line *al)
> +{
> +       struct annotation_line *it = al;
> +
> +       /* find next asm line */
> +       list_for_each_entry_continue(it, browser->b.top, node) {
> +               if (it->idx_asm >= 0)
> +                       return it;
> +       }
> +
> +       /* no asm line found forwards, try backwards */
> +       it = al;
> +       list_for_each_entry_continue_reverse(it, browser->b.top, node) {
> +               if (it->idx_asm >= 0)
> +                       return it;
> +       }
> +
> +       /* There are no asm lines */
> +       return al;

Does this error case need handling in the caller?

Thanks,
Ian

> +}
> +
>  static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>  {
>         struct annotation *notes = browser__annotation(&browser->b);
> @@ -363,9 +386,8 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>                 browser->b.index = al->idx;
>         } else {
>                 if (al->idx_asm < 0) {
> -                       ui_helpline__puts("Only available for assembly lines.");
> -                       browser->b.seek(&browser->b, -offset, SEEK_CUR);
> -                       return false;
> +                       /* move cursor to next asm line */
> +                       al = annotate_browser__find_next_asm_line(browser, al);
>                 }
>
>                 if (al->idx_asm < offset)
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf annotate: allow 's' on source code lines
  2021-06-22  5:07 ` Ian Rogers
@ 2021-06-22  7:58   ` Riccardo Mancini
  0 siblings, 0 replies; 3+ messages in thread
From: Riccardo Mancini @ 2021-06-22  7:58 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Arnaldo Carvalho de Melo, Namhyung Kim, Peter Zijlstra,
	Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Martin Liska, Jason Yan, linux-perf-users, linux-kernel

Hi,

thank you for your comment.

On Mon, 2021-06-21 at 22:07 -0700, Ian Rogers wrote:
> On Mon, Jun 21, 2021 at 4:02 PM Riccardo Mancini <rickyman7@gmail.com> wrote:
> > 
> > In perf annotate, when 's' is pressed on a line containing
> > source code, it shows the message "Only available for assembly
> > lines".
> > This patch gets rid of the error, moving the cursr to the next
> > available asm line (or the closest previous one if no asm line
> > is found moving forwards), before hiding source code lines.
> > 
> > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com>
> > ---
> >  tools/perf/ui/browsers/annotate.c | 28 +++++++++++++++++++++++++---
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browsers/annotate.c
> > b/tools/perf/ui/browsers/annotate.c
> > index ad0a70f0edaf..eb94d20d0d13 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -343,6 +343,29 @@ static void annotate_browser__calc_percent(struct
> > annotate_browser *browser,
> >         browser->curr_hot = rb_last(&browser->entries);
> >  }
> > 
> > +static struct annotation_line *annotate_browser__find_next_asm_line(
> > +                                       struct annotate_browser *browser,
> > +                                       struct annotation_line *al)
> > +{
> > +       struct annotation_line *it = al;
> > +
> > +       /* find next asm line */
> > +       list_for_each_entry_continue(it, browser->b.top, node) {
> > +               if (it->idx_asm >= 0)
> > +                       return it;
> > +       }
> > +
> > +       /* no asm line found forwards, try backwards */
> > +       it = al;
> > +       list_for_each_entry_continue_reverse(it, browser->b.top, node) {
> > +               if (it->idx_asm >= 0)
> > +                       return it;
> > +       }
> > +
> > +       /* There are no asm lines */
> > +       return al;
> 
> Does this error case need handling in the caller?

Agreed.
I believe this should never happen, but in case it happens it will cause
problems.
I'll change it to return NULL and handle it in the caller.

Thanks,
Riccardo

> 
> Thanks,
> Ian
> 
> > +}
> > +
> >  static bool annotate_browser__toggle_source(struct annotate_browser
> > *browser)
> >  {
> >         struct annotation *notes = browser__annotation(&browser->b);
> > @@ -363,9 +386,8 @@ static bool annotate_browser__toggle_source(struct
> > annotate_browser *browser)
> >                 browser->b.index = al->idx;
> >         } else {
> >                 if (al->idx_asm < 0) {
> > -                       ui_helpline__puts("Only available for assembly
> > lines.");
> > -                       browser->b.seek(&browser->b, -offset, SEEK_CUR);
> > -                       return false;
> > +                       /* move cursor to next asm line */
> > +                       al = annotate_browser__find_next_asm_line(browser,
> > al);
> >                 }
> > 
> >                 if (al->idx_asm < offset)
> > --
> > 2.31.1
> > 



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-22  7:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 23:00 [PATCH] perf annotate: allow 's' on source code lines Riccardo Mancini
2021-06-22  5:07 ` Ian Rogers
2021-06-22  7:58   ` Riccardo Mancini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).