On Fri, Jun 25, 2021 at 8:53 AM Riccardo Mancini [off-list ref] wrote:

Hi Ian,

On Thu, 2021-06-24 at 22:37 -0700, Ian Rogers wrote:

quoted

On Thu, Jun 24, 2021 at 3:37 PM Riccardo Mancini [off-list ref] wrote:

quoted

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 <redacted>

Acked-by: Ian Rogers <irogers@google.com>

quoted

---
 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>

quoted

Thanks,
Ian

quoted

+                       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