[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZrEnU6_MMEYY4GMU@x1>
Date: Mon, 5 Aug 2024 16:26:11 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 5/5] perf annotate: Add --skip-empty option
On Sat, Aug 03, 2024 at 02:13:32PM -0700, Namhyung Kim wrote:
> Like in perf report, we want to hide empty events in the perf annotate
> output. This is consistent when the option is set in perf report.
>
> For example, the following command would use 3 events including dummy.
The option --skip-empty is useful, but I wonder if for "dummy" it
shouldn't be the default, i.e. a per-event "skip" or "hide" flag that we
would set for the "dummy" event in addition to this --skip-empty command
line option?
- Arnaldo
> $ perf mem record -a -- perf test -w noploop
>
> $ perf evlist
> cpu/mem-loads,ldlat=30/P
> cpu/mem-stores/P
> dummy:u
>
> Just using perf annotate with --group will show the all 3 events.
>
> $ perf annotate --group --stdio | head
> Percent | Source code & Disassembly of ...
> --------------------------------------------------------------
> : 0 0xe060 <_dl_relocate_object>:
> 0.00 0.00 0.00 : e060: pushq %rbp
> 0.00 0.00 0.00 : e061: movq %rsp, %rbp
> 0.00 0.00 0.00 : e064: pushq %r15
> 0.00 0.00 0.00 : e066: movq %rdi, %r15
> 0.00 0.00 0.00 : e069: pushq %r14
> 0.00 0.00 0.00 : e06b: pushq %r13
> 0.00 0.00 0.00 : e06d: movl %edx, %r13d
>
> Now with --skip-empty, it'll hide the last dummy event.
>
> $ perf annotate --group --stdio --skip-empty | head
> Percent | Source code & Disassembly of ...
> ------------------------------------------------------
> : 0 0xe060 <_dl_relocate_object>:
> 0.00 0.00 : e060: pushq %rbp
> 0.00 0.00 : e061: movq %rsp, %rbp
> 0.00 0.00 : e064: pushq %r15
> 0.00 0.00 : e066: movq %rdi, %r15
> 0.00 0.00 : e069: pushq %r14
> 0.00 0.00 : e06b: pushq %r13
> 0.00 0.00 : e06d: movl %edx, %r13d
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/Documentation/perf-annotate.txt | 3 +++
> tools/perf/builtin-annotate.c | 2 ++
> tools/perf/util/annotate.c | 22 +++++++++++++++++-----
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
> index b95524bea021..156c5f37b051 100644
> --- a/tools/perf/Documentation/perf-annotate.txt
> +++ b/tools/perf/Documentation/perf-annotate.txt
> @@ -165,6 +165,9 @@ include::itrace.txt[]
> --type-stat::
> Show stats for the data type annotation.
>
> +--skip-empty::
> + Do not display empty (or dummy) events.
> +
>
> SEE ALSO
> --------
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index cf60392b1c19..efcadb7620b8 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -795,6 +795,8 @@ int cmd_annotate(int argc, const char **argv)
> "Show stats for the data type annotation"),
> OPT_BOOLEAN(0, "insn-stat", &annotate.insn_stat,
> "Show instruction stats for the data type annotation"),
> + OPT_BOOLEAN(0, "skip-empty", &symbol_conf.skip_empty,
> + "Do not display empty (or dummy) events in the output"),
> OPT_END()
> };
> int ret;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 917897fe44a2..eafe8d65052e 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -848,6 +848,10 @@ static void annotation__calc_percent(struct annotation *notes,
>
> BUG_ON(i >= al->data_nr);
>
> + if (symbol_conf.skip_empty &&
> + evsel__hists(evsel)->stats.nr_samples == 0)
> + continue;
> +
> data = &al->data[i++];
>
> calc_percent(notes, evsel, data, al->offset, end);
> @@ -901,7 +905,7 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
> .options = &annotate_opts,
> };
> struct arch *arch = NULL;
> - int err;
> + int err, nr;
>
> err = evsel__get_arch(evsel, &arch);
> if (err < 0)
> @@ -922,10 +926,18 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
> return -1;
> }
>
> - if (evsel__is_group_event(evsel))
> - notes->src->nr_events = evsel->core.nr_members;
> - else
> - notes->src->nr_events = 1;
> + nr = 0;
> + if (evsel__is_group_event(evsel)) {
> + struct evsel *pos;
> +
> + for_each_group_evsel(pos, evsel) {
> + if (symbol_conf.skip_empty &&
> + evsel__hists(pos)->stats.nr_samples == 0)
> + continue;
> + nr++;
> + }
> + }
> + notes->src->nr_events = nr ? nr : 1;
>
> if (annotate_opts.full_addr)
> notes->src->start = map__objdump_2mem(ms->map, ms->sym->start);
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
Powered by blists - more mailing lists