[1/5] perf annotate: Split struct cycles_info
Commit Message
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@kernel.org>
---
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(-)
Comments
On Thu, Nov 2, 2023 at 3:26 PM Namhyung Kim <namhyung@kernel.org> 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@kernel.org>
> ---
> 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
>
Hi Ian,
On Thu, Nov 2, 2023 at 3:53 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Nov 2, 2023 at 3:26 PM Namhyung Kim <namhyung@kernel.org> 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@kernel.org>
> > ---
> > 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?
annotation__count_and_fill() also needs it.
Thanks,
Namhyung
>
> 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;
@@ -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;
}
@@ -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;
+ 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,
@@ -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;