* Re: [PATCH] perf report: Show zero counters as well in 'perf report --stat'
2018-03-07 15:24 [PATCH] perf report: Show zero counters as well in 'perf report --stat' Ingo Molnar
@ 2018-03-07 15:37 ` Jiri Olsa
2018-03-07 15:43 ` Arnaldo Carvalho de Melo
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2018-03-07 15:37 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Arnaldo Carvalho de Melo, linux-kernel
On Wed, Mar 07, 2018 at 04:24:30PM +0100, Ingo Molnar wrote:
>
> When recently using 'perf report --stat' it was not clear to me from the output
> whether a particular statistics field (LOST_SAMPLES) was not present, or just
> zero:
>
> fomalhaut:~> perf report --stat
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85
> COMM events: 3389
> EXIT events: 1605
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377
> SAMPLE events: 472629
> MMAP2 events: 14753
> FINISHED_ROUND events: 139
> THREAD_MAP events: 1
> CPU_MAP events: 1
> TIME_CONV events: 1
>
> I had to check the output several times to ascertain that I'm not misreading the
> output, that the field didn't change and that I didn't misremember the name. In
> fact I had to look into the perf source to make sure that zero fields are indeed
> not shown.
>
> With the patch applied:
>
> fomalhaut:~> perf report --stat
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85
> LOST events: 0
> COMM events: 3389
> EXIT events: 1605
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377
> READ events: 0
> SAMPLE events: 472629
> MMAP2 events: 14753
> AUX events: 0
> ITRACE_START events: 0
> LOST_SAMPLES events: 0
> SWITCH events: 0
> SWITCH_CPU_WIDE events: 0
> NAMESPACES events: 0
> ATTR events: 0
> EVENT_TYPE events: 0
> TRACING_DATA events: 0
> BUILD_ID events: 0
> FINISHED_ROUND events: 139
> ID_INDEX events: 0
> AUXTRACE_INFO events: 0
> AUXTRACE events: 0
> AUXTRACE_ERROR events: 0
> THREAD_MAP events: 1
> CPU_MAP events: 1
> STAT_CONFIG events: 0
> STAT events: 0
> STAT_ROUND events: 0
> EVENT_UPDATE events: 0
> TIME_CONV events: 1
> FEATURE events: 0
ok, it also might be better to read since now the events
appear in the same place, not scattered based on what was
data was collected
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] perf report: Show zero counters as well in 'perf report --stat'
2018-03-07 15:24 [PATCH] perf report: Show zero counters as well in 'perf report --stat' Ingo Molnar
2018-03-07 15:37 ` Jiri Olsa
@ 2018-03-07 15:43 ` Arnaldo Carvalho de Melo
2018-03-09 8:38 ` Ingo Molnar
2018-03-09 15:19 ` Arnaldo Carvalho de Melo
2018-03-20 6:27 ` [tip:perf/core] " tip-bot for Ingo Molnar
3 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-07 15:43 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
Em Wed, Mar 07, 2018 at 04:24:30PM +0100, Ingo Molnar escreveu:
>
> When recently using 'perf report --stat' it was not clear to me from the output
> whether a particular statistics field (LOST_SAMPLES) was not present, or just
> zero:
>
> fomalhaut:~> perf report --stat
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85
> COMM events: 3389
> EXIT events: 1605
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377
> SAMPLE events: 472629
> MMAP2 events: 14753
> FINISHED_ROUND events: 139
> THREAD_MAP events: 1
> CPU_MAP events: 1
> TIME_CONV events: 1
>
> I had to check the output several times to ascertain that I'm not misreading the
> output, that the field didn't change and that I didn't misremember the name. In
> fact I had to look into the perf source to make sure that zero fields are indeed
> not shown.
>
> With the patch applied:
>
> fomalhaut:~> perf report --stat
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85
> LOST events: 0
> COMM events: 3389
> EXIT events: 1605
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377
> READ events: 0
> SAMPLE events: 472629
> MMAP2 events: 14753
> AUX events: 0
> ITRACE_START events: 0
> LOST_SAMPLES events: 0
> SWITCH events: 0
> SWITCH_CPU_WIDE events: 0
> NAMESPACES events: 0
> ATTR events: 0
> EVENT_TYPE events: 0
> TRACING_DATA events: 0
> BUILD_ID events: 0
> FINISHED_ROUND events: 139
> ID_INDEX events: 0
> AUXTRACE_INFO events: 0
> AUXTRACE events: 0
> AUXTRACE_ERROR events: 0
> THREAD_MAP events: 1
> CPU_MAP events: 1
> STAT_CONFIG events: 0
> STAT events: 0
> STAT_ROUND events: 0
> EVENT_UPDATE events: 0
> TIME_CONV events: 1
> FEATURE events: 0
>
> It's pretty clear at a glance that LOST_SAMPLES is present but zero.
Your wording confused me a bit, that "is present" part, because there
are some PERF_RECORD_ events that will only be "present" if we actually
explicitely ask them to be, by setting flags in perf_event_attr, like:
mmap : 1, /* include mmap data */
comm : 1, /* include comm data */
task : 1, /* trace fork/exit */
mmap_data : 1, /* non-exec mmap data */
sample_id_all : 1, /* sample_type all events */
mmap2 : 1, /* include mmap with inode data */
comm_exec : 1, /* flag comm events that are due to an exec */
context_switch : 1, /* context switch data */
namespaces : 1, /* include namespaces data */
So, for PERF_RECORD_MMAP2 events, one can say that it is "present but
zero" if we have an event with perf_event_attr.mmap2 = 1 and no
PERF_RECORD_MMAP2 events recorded.
But for PERF_RECORD_LOST, that will always be present, if we lose
records.
So perhaps to make all this clean we can add your patch, and on top of
it another that shows the perf_record_attr flag for the ones that are
not on all the time, something like:
fomalhaut:~> perf report --stat
Aggregated stats:
TOTAL events: 495984
MMAP events: 85 attr.mmap: 1
LOST events: 0
COMM events: 3389 attr.comm: 1, attr.comm_exec: 0
EXIT events: 1605 attr.task: 1
THROTTLE events: 2
UNTHROTTLE events: 2
FORK events: 3377 attr.task: 1
READ events: 0
SAMPLE events: 472629
MMAP2 events: 14753 attr.mmap2: 1
AUX events: 0 have to look at what enables this, etc
ITRACE_START events: 0 ditto
LOST_SAMPLES events: 0
SWITCH events: 0 attr.context_switch: 0
SWITCH_CPU_WIDE events: 0 attr.context_switch: 0
NAMESPACES events: 0 attr.namespaces: 0
ATTR events: 0
EVENT_TYPE events: 0
TRACING_DATA events: 0
BUILD_ID events: 0 this implies disabling in 'perf record' command line
FINISHED_ROUND events: 139
ID_INDEX events: 0
AUXTRACE_INFO events: 0
AUXTRACE events: 0
AUXTRACE_ERROR events: 0
THREAD_MAP events: 1
CPU_MAP events: 1
STAT_CONFIG events: 0
STAT events: 0
STAT_ROUND events: 0
EVENT_UPDATE events: 0
TIME_CONV events: 1 have to check
FEATURE events: 0
> The original output can still be gotten via:
>
> fomalhaut:~> perf report --stat | grep -vw 0
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85
> COMM events: 3389
> EXIT events: 1605
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377
> SAMPLE events: 472629
> MMAP2 events: 14753
> FINISHED_ROUND events: 139
> THREAD_MAP events: 1
> CPU_MAP events: 1
> TIME_CONV events: 1
>
> So I don't think there's any real loss in functionality.
>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> tools/perf/ui/stdio/hist.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> Index: linux/tools/perf/ui/stdio/hist.c
> ===================================================================
> --- linux.orig/tools/perf/ui/stdio/hist.c
> +++ linux/tools/perf/ui/stdio/hist.c
> @@ -840,15 +840,11 @@ size_t events_stats__fprintf(struct even
> for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
> const char *name;
>
> - if (stats->nr_events[i] == 0)
> - continue;
> -
> name = perf_event__name(i);
> if (!strcmp(name, "UNKNOWN"))
> continue;
>
> - ret += fprintf(fp, "%16s events: %10d\n", name,
> - stats->nr_events[i]);
> + ret += fprintf(fp, "%16s events: %10d\n", name, stats->nr_events[i]);
> }
>
> return ret;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] perf report: Show zero counters as well in 'perf report --stat'
2018-03-07 15:43 ` Arnaldo Carvalho de Melo
@ 2018-03-09 8:38 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2018-03-09 8:38 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > STAT events: 0
> > STAT_ROUND events: 0
> > EVENT_UPDATE events: 0
> > TIME_CONV events: 1
> > FEATURE events: 0
> >
> > It's pretty clear at a glance that LOST_SAMPLES is present but zero.
>
> Your wording confused me a bit, that "is present" part, because there
> are some PERF_RECORD_ events that will only be "present" if we actually
> explicitely ask them to be, by setting flags in perf_event_attr, like:
>
> mmap : 1, /* include mmap data */
> comm : 1, /* include comm data */
> task : 1, /* trace fork/exit */
> mmap_data : 1, /* non-exec mmap data */
> sample_id_all : 1, /* sample_type all events */
> mmap2 : 1, /* include mmap with inode data */
> comm_exec : 1, /* flag comm events that are due to an exec */
> context_switch : 1, /* context switch data */
> namespaces : 1, /* include namespaces data */
>
> So, for PERF_RECORD_MMAP2 events, one can say that it is "present but
> zero" if we have an event with perf_event_attr.mmap2 = 1 and no
> PERF_RECORD_MMAP2 events recorded.
>
> But for PERF_RECORD_LOST, that will always be present, if we lose
> records.
Yeah, the triple ambiguity between:
- did the event disappear due to 'we did not lose any records',
- or did it disappear because it's somehow a conditional stat field,
- or did it disappear because I'm blind and/or mis-remembering the field name.
... is what was causing trouble to me personally, to the level that I had to go
and look into the tooling code to make sure I'm interpreting it correctly.
> So perhaps to make all this clean we can add your patch, and on top of
> it another that shows the perf_record_attr flag for the ones that are
> not on all the time, something like:
>
> fomalhaut:~> perf report --stat
>
> Aggregated stats:
> TOTAL events: 495984
> MMAP events: 85 attr.mmap: 1
> LOST events: 0
> COMM events: 3389 attr.comm: 1, attr.comm_exec: 0
> EXIT events: 1605 attr.task: 1
> THROTTLE events: 2
> UNTHROTTLE events: 2
> FORK events: 3377 attr.task: 1
> READ events: 0
> SAMPLE events: 472629
> MMAP2 events: 14753 attr.mmap2: 1
> AUX events: 0 have to look at what enables this, etc
> ITRACE_START events: 0 ditto
> LOST_SAMPLES events: 0
> SWITCH events: 0 attr.context_switch: 0
> SWITCH_CPU_WIDE events: 0 attr.context_switch: 0
> NAMESPACES events: 0 attr.namespaces: 0
> ATTR events: 0
> EVENT_TYPE events: 0
> TRACING_DATA events: 0
> BUILD_ID events: 0 this implies disabling in 'perf record' command line
> FINISHED_ROUND events: 139
> ID_INDEX events: 0
> AUXTRACE_INFO events: 0
> AUXTRACE events: 0
> AUXTRACE_ERROR events: 0
> THREAD_MAP events: 1
> CPU_MAP events: 1
> STAT_CONFIG events: 0
> STAT events: 0
> STAT_ROUND events: 0
> EVENT_UPDATE events: 0
> TIME_CONV events: 1 have to check
> FEATURE events: 0
Yeah, I'm fine with this too if it's easy enough to implement - although I suspect
in most cases the knowledge that a stat field not present must be due to an
environment/setup dependency (and not a runtime/workload dependency) is enough.
It's the uncertainty of 'stat line can disappear because it was zero' that was my
main beef :-)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: Show zero counters as well in 'perf report --stat'
2018-03-07 15:24 [PATCH] perf report: Show zero counters as well in 'perf report --stat' Ingo Molnar
2018-03-07 15:37 ` Jiri Olsa
2018-03-07 15:43 ` Arnaldo Carvalho de Melo
@ 2018-03-09 15:19 ` Arnaldo Carvalho de Melo
2018-03-20 6:27 ` [tip:perf/core] " tip-bot for Ingo Molnar
3 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-09 15:19 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Arnaldo Carvalho de Melo, Jiri Olsa, linux-kernel
Em Wed, Mar 07, 2018 at 04:24:30PM +0100, Ingo Molnar escreveu:
>
> I had to check the output several times to ascertain that I'm not misreading the
> output, that the field didn't change and that I didn't misremember the name. In
> fact I had to look into the perf source to make sure that zero fields are indeed
> not shown.
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] perf report: Show zero counters as well in 'perf report --stat'
2018-03-07 15:24 [PATCH] perf report: Show zero counters as well in 'perf report --stat' Ingo Molnar
` (2 preceding siblings ...)
2018-03-09 15:19 ` Arnaldo Carvalho de Melo
@ 2018-03-20 6:27 ` tip-bot for Ingo Molnar
3 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Ingo Molnar @ 2018-03-20 6:27 UTC (permalink / raw)
To: linux-tip-commits; +Cc: mingo, tglx, acme, jolsa, linux-kernel, hpa
Commit-ID: 39ce7fb31530c6d4648919e03e16c5e9286a5940
Gitweb: https://git.kernel.org/tip/39ce7fb31530c6d4648919e03e16c5e9286a5940
Author: Ingo Molnar <mingo@kernel.org>
AuthorDate: Wed, 7 Mar 2018 16:24:30 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 16 Mar 2018 13:55:36 -0300
perf report: Show zero counters as well in 'perf report --stat'
When recently using 'perf report --stat' it was not clear to me from the
output whether a particular statistics field (LOST_SAMPLES) was not
present, or just zero:
fomalhaut:~> perf report --stat
Aggregated stats:
TOTAL events: 495984
MMAP events: 85
COMM events: 3389
EXIT events: 1605
THROTTLE events: 2
UNTHROTTLE events: 2
FORK events: 3377
SAMPLE events: 472629
MMAP2 events: 14753
FINISHED_ROUND events: 139
THREAD_MAP events: 1
CPU_MAP events: 1
TIME_CONV events: 1
I had to check the output several times to ascertain that I'm not
misreading the output, that the field didn't change and that I didn't
misremember the name. In fact I had to look into the perf source to make
sure that zero fields are indeed not shown.
With the patch applied:
fomalhaut:~> perf report --stat
Aggregated stats:
TOTAL events: 495984
MMAP events: 85
LOST events: 0
COMM events: 3389
EXIT events: 1605
THROTTLE events: 2
UNTHROTTLE events: 2
FORK events: 3377
READ events: 0
SAMPLE events: 472629
MMAP2 events: 14753
AUX events: 0
ITRACE_START events: 0
LOST_SAMPLES events: 0
SWITCH events: 0
SWITCH_CPU_WIDE events: 0
NAMESPACES events: 0
ATTR events: 0
EVENT_TYPE events: 0
TRACING_DATA events: 0
BUILD_ID events: 0
FINISHED_ROUND events: 139
ID_INDEX events: 0
AUXTRACE_INFO events: 0
AUXTRACE events: 0
AUXTRACE_ERROR events: 0
THREAD_MAP events: 1
CPU_MAP events: 1
STAT_CONFIG events: 0
STAT events: 0
STAT_ROUND events: 0
EVENT_UPDATE events: 0
TIME_CONV events: 1
FEATURE events: 0
It's pretty clear at a glance that LOST_SAMPLES is present but zero.
The original output can still be gotten via:
fomalhaut:~> perf report --stat | grep -vw 0
Aggregated stats:
TOTAL events: 495984
MMAP events: 85
COMM events: 3389
EXIT events: 1605
THROTTLE events: 2
UNTHROTTLE events: 2
FORK events: 3377
SAMPLE events: 472629
MMAP2 events: 14753
FINISHED_ROUND events: 139
THREAD_MAP events: 1
CPU_MAP events: 1
TIME_CONV events: 1
So I don't think there's any real loss in functionality.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Link: http://lkml.kernel.org/r/20180307152430.7e5h7e657b7bgd7q@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/stdio/hist.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 25dd1e0ecc58..6832fcb2e6ff 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -840,15 +840,11 @@ size_t events_stats__fprintf(struct events_stats *stats, FILE *fp)
for (i = 0; i < PERF_RECORD_HEADER_MAX; ++i) {
const char *name;
- if (stats->nr_events[i] == 0)
- continue;
-
name = perf_event__name(i);
if (!strcmp(name, "UNKNOWN"))
continue;
- ret += fprintf(fp, "%16s events: %10d\n", name,
- stats->nr_events[i]);
+ ret += fprintf(fp, "%16s events: %10d\n", name, stats->nr_events[i]);
}
return ret;
^ permalink raw reply related [flat|nested] 6+ messages in thread