Re: [PATCH 1/5] perf annotate: Split struct cycles_info
From: Ian Rogers
Date: Thu Nov 02 2023 - 18:53:17 EST
On Thu, Nov 2, 2023 at 3:26 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> The cycles info is used only when branch stack is provided. Split them
> into a separate struct and lazy allocate them to save some memory.
>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> tools/perf/ui/browsers/annotate.c | 2 +-
> tools/perf/util/annotate.c | 34 ++++++++++++++++++-------------
> tools/perf/util/annotate.h | 14 ++++++++-----
> 3 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index ccdb2cd11fbf..d2470f87344d 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -337,7 +337,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> max_percent = percent;
> }
>
> - if (max_percent < 0.01 && pos->al.ipc == 0) {
> + if (max_percent < 0.01 && (!pos->al.cycles || pos->al.cycles->ipc == 0)) {
> RB_CLEAR_NODE(&pos->al.rb_node);
> continue;
> }
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 82956adf9963..3e7f75827270 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1100,8 +1100,8 @@ static void annotation__count_and_fill(struct annotation *notes, u64 start, u64
> for (offset = start; offset <= end; offset++) {
> struct annotation_line *al = notes->offsets[offset];
>
> - if (al && al->ipc == 0.0) {
> - al->ipc = ipc;
> + if (al && al->cycles && al->cycles->ipc == 0.0) {
> + al->cycles->ipc = ipc;
> cover_insn++;
> }
> }
> @@ -1134,13 +1134,18 @@ void annotation__compute_ipc(struct annotation *notes, size_t size)
> if (ch && ch->cycles) {
> struct annotation_line *al;
>
> + al = notes->offsets[offset];
> + if (al && al->cycles == NULL) {
> + al->cycles = zalloc(sizeof(*al->cycles));
> + if (al->cycles == NULL)
> + continue;
> + }
> if (ch->have_start)
> annotation__count_and_fill(notes, ch->start, offset, ch);
> - al = notes->offsets[offset];
> if (al && ch->num_aggr) {
> - al->cycles = ch->cycles_aggr / ch->num_aggr;
> - al->cycles_max = ch->cycles_max;
> - al->cycles_min = ch->cycles_min;
Thanks for doing this! Would it make sense to do the zalloc here to be
lazier about allocation?
Ian
> + al->cycles->avg = ch->cycles_aggr / ch->num_aggr;
> + al->cycles->max = ch->cycles_max;
> + al->cycles->min = ch->cycles_min;
> }
> notes->have_cycles = true;
> }
> @@ -1225,6 +1230,7 @@ static void annotation_line__exit(struct annotation_line *al)
> {
> zfree_srcline(&al->path);
> zfree(&al->line);
> + zfree(&al->cycles);
> }
>
> static size_t disasm_line_size(int nr)
> @@ -3083,8 +3089,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> int printed;
>
> if (first_line && (al->offset == -1 || percent_max == 0.0)) {
> - if (notes->have_cycles) {
> - if (al->ipc == 0.0 && al->cycles == 0)
> + if (notes->have_cycles && al->cycles) {
> + if (al->cycles->ipc == 0.0 && al->cycles->avg == 0)
> show_title = true;
> } else
> show_title = true;
> @@ -3121,17 +3127,17 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> }
>
> if (notes->have_cycles) {
> - if (al->ipc)
> - obj__printf(obj, "%*.2f ", ANNOTATION__IPC_WIDTH - 1, al->ipc);
> + if (al->cycles && al->cycles->ipc)
> + obj__printf(obj, "%*.2f ", ANNOTATION__IPC_WIDTH - 1, al->cycles->ipc);
> else if (!show_title)
> obj__printf(obj, "%*s", ANNOTATION__IPC_WIDTH, " ");
> else
> obj__printf(obj, "%*s ", ANNOTATION__IPC_WIDTH - 1, "IPC");
>
> if (!notes->options->show_minmax_cycle) {
> - if (al->cycles)
> + if (al->cycles && al->cycles->avg)
> obj__printf(obj, "%*" PRIu64 " ",
> - ANNOTATION__CYCLES_WIDTH - 1, al->cycles);
> + ANNOTATION__CYCLES_WIDTH - 1, al->cycles->avg);
> else if (!show_title)
> obj__printf(obj, "%*s",
> ANNOTATION__CYCLES_WIDTH, " ");
> @@ -3145,8 +3151,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
>
> scnprintf(str, sizeof(str),
> "%" PRIu64 "(%" PRIu64 "/%" PRIu64 ")",
> - al->cycles, al->cycles_min,
> - al->cycles_max);
> + al->cycles->avg, al->cycles->min,
> + al->cycles->max);
>
> obj__printf(obj, "%*s ",
> ANNOTATION__MINMAX_CYCLES_WIDTH - 1,
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 962780559176..16d27952fd5c 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -130,6 +130,13 @@ struct annotation_data {
> struct sym_hist_entry he;
> };
>
> +struct cycles_info {
> + float ipc;
> + u64 avg;
> + u64 max;
> + u64 min;
> +};
> +
> struct annotation_line {
> struct list_head node;
> struct rb_node rb_node;
> @@ -137,12 +144,9 @@ struct annotation_line {
> char *line;
> int line_nr;
> char *fileloc;
> - int jump_sources;
> - float ipc;
> - u64 cycles;
> - u64 cycles_max;
> - u64 cycles_min;
> char *path;
> + struct cycles_info *cycles;
> + int jump_sources;
> u32 idx;
> int idx_asm;
> int data_nr;
> --
> 2.42.0.869.gea05f2083d-goog
>