* [PATCH v2] perf annotate: allow 's' on source code lines @ 2021-06-24 22:34 Riccardo Mancini 2021-06-25 5:37 ` Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: Riccardo Mancini @ 2021-06-24 22:34 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, 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. Changes in v2: - handle case of no asm line found in annotate_browser__find_next_asm_line by returning NULL and handling error in caller. Signed-off-by: Riccardo Mancini <rickyman7@gmail.com> --- tools/perf/ui/browsers/annotate.c | 32 ++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index ad0a70f0edaf..f5509a958e38 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 NULL; +} + static bool annotate_browser__toggle_source(struct annotate_browser *browser) { struct annotation *notes = browser__annotation(&browser->b); @@ -363,9 +386,12 @@ 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) { + browser->b.seek(&browser->b, -offset, SEEK_CUR); + return false; + } } if (al->idx_asm < offset) -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf annotate: allow 's' on source code lines 2021-06-24 22:34 [PATCH v2] perf annotate: allow 's' on source code lines Riccardo Mancini @ 2021-06-25 5:37 ` Ian Rogers 2021-06-25 15:53 ` Riccardo Mancini 0 siblings, 1 reply; 5+ messages in thread From: Ian Rogers @ 2021-06-25 5:37 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, linux-perf-users, linux-kernel On Thu, Jun 24, 2021 at 3:37 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. > > Changes in v2: > - handle case of no asm line found in > annotate_browser__find_next_asm_line by returning NULL and > handling error in caller. > > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com> Acked-by: Ian Rogers <irogers@google.com> > --- > tools/perf/ui/browsers/annotate.c | 32 ++++++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c > index ad0a70f0edaf..f5509a958e38 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 NULL; > +} > + > static bool annotate_browser__toggle_source(struct annotate_browser *browser) > { > struct annotation *notes = browser__annotation(&browser->b); > @@ -363,9 +386,12 @@ 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 */ comment nit, perhaps prefer "closest" rather than "next" due to searching backward. Thanks, Ian > + al = annotate_browser__find_next_asm_line(browser, al); > + if (!al) { > + browser->b.seek(&browser->b, -offset, SEEK_CUR); > + return false; > + } > } > > if (al->idx_asm < offset) > -- > 2.31.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf annotate: allow 's' on source code lines 2021-06-25 5:37 ` Ian Rogers @ 2021-06-25 15:53 ` Riccardo Mancini 2021-06-29 15:00 ` Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: Riccardo Mancini @ 2021-06-25 15:53 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, linux-perf-users, linux-kernel Hi Ian, On Thu, 2021-06-24 at 22:37 -0700, Ian Rogers wrote: > On Thu, Jun 24, 2021 at 3:37 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. > > > > Changes in v2: > > - handle case of no asm line found in > > annotate_browser__find_next_asm_line by returning NULL and > > handling error in caller. > > > > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com> > > Acked-by: Ian Rogers <irogers@google.com> > > > --- > > tools/perf/ui/browsers/annotate.c | 32 ++++++++++++++++++++++++++++--- > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > diff --git a/tools/perf/ui/browsers/annotate.c > > b/tools/perf/ui/browsers/annotate.c > > index ad0a70f0edaf..f5509a958e38 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 NULL; > > +} > > + > > static bool annotate_browser__toggle_source(struct annotate_browser > > *browser) > > { > > struct annotation *notes = browser__annotation(&browser->b); > > @@ -363,9 +386,12 @@ 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 */ > > comment nit, perhaps prefer "closest" rather than "next" due to > searching backward. The backward search is just a fallback in case the forward one finds no asm line, which I believe is unlikely. Maybe it's also impossible, but I don't really know how those lines are generated, so I put a fallback in place. Furthermore, "closest" would imply that a previous asm line could be chosen over a subsequent one if closer, even if the latter is present. Thanks, Riccardo > > Thanks, > Ian > > > + al = annotate_browser__find_next_asm_line(browser, > > al); > > + if (!al) { > > + browser->b.seek(&browser->b, -offset, > > SEEK_CUR); > > + return false; > > + } > > } > > > > if (al->idx_asm < offset) > > -- > > 2.31.1 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf annotate: allow 's' on source code lines 2021-06-25 15:53 ` Riccardo Mancini @ 2021-06-29 15:00 ` Ian Rogers 2021-07-01 18:03 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 5+ messages in thread From: Ian Rogers @ 2021-06-29 15:00 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, linux-perf-users, linux-kernel On Fri, Jun 25, 2021 at 8:53 AM Riccardo Mancini <rickyman7@gmail.com> wrote: > > Hi Ian, > > On Thu, 2021-06-24 at 22:37 -0700, Ian Rogers wrote: > > On Thu, Jun 24, 2021 at 3:37 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. > > > > > > Changes in v2: > > > - handle case of no asm line found in > > > annotate_browser__find_next_asm_line by returning NULL and > > > handling error in caller. > > > > > > Signed-off-by: Riccardo Mancini <rickyman7@gmail.com> > > > > Acked-by: Ian Rogers <irogers@google.com> > > > > > --- > > > tools/perf/ui/browsers/annotate.c | 32 ++++++++++++++++++++++++++++--- > > > 1 file changed, 29 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/perf/ui/browsers/annotate.c > > > b/tools/perf/ui/browsers/annotate.c > > > index ad0a70f0edaf..f5509a958e38 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 NULL; > > > +} > > > + > > > static bool annotate_browser__toggle_source(struct annotate_browser > > > *browser) > > > { > > > struct annotation *notes = browser__annotation(&browser->b); > > > @@ -363,9 +386,12 @@ 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 */ > > > > comment nit, perhaps prefer "closest" rather than "next" due to > > searching backward. > > The backward search is just a fallback in case the forward one finds no asm > line, which I believe is unlikely. Maybe it's also impossible, but I don't > really know how those lines are generated, so I put a fallback in place. > Furthermore, "closest" would imply that a previous asm line could be chosen over > a subsequent one if closer, even if the latter is present. > > Thanks, > Riccardo Agreed, thanks for thinking about this. Acked-by: Ian Rogers <irogers@google.com> > > > > Thanks, > > Ian > > > > > + al = annotate_browser__find_next_asm_line(browser, > > > al); > > > + if (!al) { > > > + browser->b.seek(&browser->b, -offset, > > > SEEK_CUR); > > > + return false; > > > + } > > > } > > > > > > if (al->idx_asm < offset) > > > -- > > > 2.31.1 > > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] perf annotate: allow 's' on source code lines 2021-06-29 15:00 ` Ian Rogers @ 2021-07-01 18:03 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 5+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-07-01 18:03 UTC (permalink / raw) To: Ian Rogers Cc: Riccardo Mancini, Namhyung Kim, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Jiri Olsa, Martin Liska, linux-perf-users, linux-kernel Em Tue, Jun 29, 2021 at 08:00:31AM -0700, Ian Rogers escreveu: > On Fri, Jun 25, 2021 at 8:53 AM Riccardo Mancini <rickyman7@gmail.com> wrote: > > On Thu, 2021-06-24 at 22:37 -0700, Ian Rogers wrote: > > > On Thu, Jun 24, 2021 at 3:37 PM Riccardo Mancini <rickyman7@gmail.com> wrote: > > > comment nit, perhaps prefer "closest" rather than "next" due to > > > searching backward. > > > > The backward search is just a fallback in case the forward one finds no asm > > line, which I believe is unlikely. Maybe it's also impossible, but I don't > > really know how those lines are generated, so I put a fallback in place. > > Furthermore, "closest" would imply that a previous asm line could be chosen over > > a subsequent one if closer, even if the latter is present. > > > > Thanks, > > Riccardo > > Agreed, thanks for thinking about this. > > Acked-by: Ian Rogers <irogers@google.com> Thanks, applied. - Arnaldo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-01 18:03 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-24 22:34 [PATCH v2] perf annotate: allow 's' on source code lines Riccardo Mancini 2021-06-25 5:37 ` Ian Rogers 2021-06-25 15:53 ` Riccardo Mancini 2021-06-29 15:00 ` Ian Rogers 2021-07-01 18:03 ` Arnaldo Carvalho de Melo
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).