Re: [PATCH 4/5] perf annotate: Move some fields to annotated_source

[Ian Rogers]

On Thu, Nov 2, 2023 at 3:27 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> Some fields in the struct annotation are only used with annotated_source
> so better to be moved there in order to reduce memory consumption for
> other symbols.
>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/perf/ui/browsers/annotate.c | 12 ++++++------
>  tools/perf/util/annotate.c        | 17 +++++++++--------
>  tools/perf/util/annotate.h        | 14 +++++++-------
>  3 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index d2470f87344d..1b42db70c998 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -384,7 +384,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>                 if (al->idx_asm < offset)
>                         offset = al->idx;
>
> -               browser->b.nr_entries = notes->nr_entries;
> +               browser->b.nr_entries = notes->src->nr_entries;
>                 notes->options->hide_src_code = false;
>                 browser->b.seek(&browser->b, -offset, SEEK_CUR);
>                 browser->b.top_idx = al->idx - offset;
> @@ -402,7 +402,7 @@ static bool annotate_browser__toggle_source(struct annotate_browser *browser)
>                 if (al->idx_asm < offset)
>                         offset = al->idx_asm;
>
> -               browser->b.nr_entries = notes->nr_asm_entries;
> +               browser->b.nr_entries = notes->src->nr_asm_entries;
>                 notes->options->hide_src_code = true;
>                 browser->b.seek(&browser->b, -offset, SEEK_CUR);
>                 browser->b.top_idx = al->idx_asm - offset;
> @@ -435,7 +435,7 @@ static void ui_browser__init_asm_mode(struct ui_browser *browser)
>  {
>         struct annotation *notes = browser__annotation(browser);
>         ui_browser__reset_index(browser);
> -       browser->nr_entries = notes->nr_asm_entries;
> +       browser->nr_entries = notes->src->nr_asm_entries;
>  }
>
>  static int sym_title(struct symbol *sym, struct map *map, char *title,
> @@ -860,7 +860,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>                                            browser->b.height,
>                                            browser->b.index,
>                                            browser->b.top_idx,
> -                                          notes->nr_asm_entries);
> +                                          notes->src->nr_asm_entries);
>                 }
>                         continue;
>                 case K_ENTER:
> @@ -991,8 +991,8 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
>
>         ui_helpline__push("Press ESC to exit");
>
> -       browser.b.width = notes->max_line_len;
> -       browser.b.nr_entries = notes->nr_entries;
> +       browser.b.width = notes->src->max_line_len;
> +       browser.b.nr_entries = notes->src->nr_entries;
>         browser.b.entries = &notes->src->source,
>         browser.b.width += 18; /* Percentage */
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 92a9adf9d5eb..ee7b8e1848a8 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -2808,19 +2808,20 @@ void annotation__mark_jump_targets(struct annotation *notes, struct symbol *sym)
>  void annotation__set_offsets(struct annotation *notes, s64 size)
>  {
>         struct annotation_line *al;
> +       struct annotated_source *src = notes->src;
>
> -       notes->max_line_len = 0;
> -       notes->nr_entries = 0;
> -       notes->nr_asm_entries = 0;
> +       src->max_line_len = 0;
> +       src->nr_entries = 0;
> +       src->nr_asm_entries = 0;
>
> -       list_for_each_entry(al, &notes->src->source, node) {
> +       list_for_each_entry(al, &src->source, node) {
>                 size_t line_len = strlen(al->line);
>
> -               if (notes->max_line_len < line_len)
> -                       notes->max_line_len = line_len;
> -               al->idx = notes->nr_entries++;
> +               if (src->max_line_len < line_len)
> +                       src->max_line_len = line_len;
> +               al->idx = src->nr_entries++;
>                 if (al->offset != -1) {
> -                       al->idx_asm = notes->nr_asm_entries++;
> +                       al->idx_asm = src->nr_asm_entries++;
>                         /*
>                          * FIXME: short term bandaid to cope with assembly
>                          * routines that comes with labels in the same column
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index d8a221591926..c51ceb857bd6 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -268,10 +268,13 @@ struct cyc_hist {
>   * returns.
>   */
>  struct annotated_source {
> -       struct list_head   source;
> -       int                nr_histograms;
> -       size_t             sizeof_sym_hist;
> -       struct sym_hist    *histograms;
> +       struct list_head        source;
> +       int                     nr_histograms;
> +       size_t                  sizeof_sym_hist;
> +       struct sym_hist         *histograms;
> +       int                     nr_entries;
> +       int                     nr_asm_entries;
> +       u16                     max_line_len;
>  };
>
>  struct annotated_branch {
> @@ -289,9 +292,6 @@ struct LOCKABLE annotation {
>         struct annotation_line  **offsets;
>         int                     nr_events;
>         int                     max_jump_sources;
> -       int                     nr_entries;
> -       int                     nr_asm_entries;
> -       u16                     max_line_len;
>         struct {
>                 u8              addr;
>                 u8              jumps;
> --
> 2.42.0.869.gea05f2083d-goog
>