linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Fischer <florian.fischer@muhq.space>
To: Ian Rogers <irogers@google.com>
Cc: Xing Zhengjun <zhengjun.xing@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Florian Schmaus <flow@cs.fau.de>
Subject: Re: [PATCH v3 2/3] perf stat: add rusage utime and stime events
Date: Sun, 10 Apr 2022 18:41:36 +0200	[thread overview]
Message-ID: <20220410164136.bxqtpbrmfbqxdx4n@pasture> (raw)
In-Reply-To: <CAP-5=fWC5e9PTs9PVttVDdNbCzYQVeqyuf95q181Vkg4NqJxqg@mail.gmail.com>

> > This patch adds two new tool internal events 'rusage_user_time'
> > and 'rusage_system_time' as well as their aliases 'ru_utime' and
> > 'ru_stime', similarly to the already present 'duration_time' event.
> >
> > Both events use the already collected rusage information obtained by wait4
> > and tracked in the global ru_stats.
> >
> > Examples presenting cache-misses and rusage information in both human and
> > machine-readable form:
> >
> > $ ./perf stat -e duration_time,ru_utime,ru_stime,cache-misses -- grep -q -r duration_time .
> >
> >  Performance counter stats for 'grep -q -r duration_time .':
> >
> >         67,422,542 ns   duration_time:u
> >         50,517,000 ns   ru_utime:u
> >         16,839,000 ns   ru_stime:u
> >             30,937      cache-misses:u
> >
> >        0.067422542 seconds time elapsed
> >
> >        0.050517000 seconds user
> >        0.016839000 seconds sys
> >
> > $ ./perf stat -x, -e duration_time,ru_utime,ru_stime,cache-misses -- grep -q -r duration_time .
> > 72134524,ns,duration_time:u,72134524,100.00,,
> > 65225000,ns,ru_utime:u,65225000,100.00,,
> > 6865000,ns,ru_stime:u,6865000,100.00,,
> > 38705,,cache-misses:u,71189328,100.00,,
> 
> This is really nice. For metric code we currently handle duration_time
> in a special way, for example:
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/metricgroup.c?h=perf/core#n745
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/metricgroup.c?h=perf/core#n1131
> We will need to do something similar with these tool events, but I'm
> happy that it can be follow-up work.
> 
> I'm not a huge fan of the names ru_utime and ru_stime, two thoughts
> here we could do duration_time:u and duration_time:k but I don't think
> that really makes sense. My preference would be to just call ru_utime
> user_time and ru_stime system_time.

I considered ru_{u,s}_time only as aliases because those are the field names in
the rusage struct filled by wait4 and are probably known by perf users.
The "official" names are currently rusage_{user,system}_time.
I could change them to only {user,system}_time because those names are more in line
with the already present duration_time and are independent of the rusage
implementation detail.

What do you think of?

---
 tools/perf/util/parse-events.c | 4 ++--
 tools/perf/util/parse-events.l | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c232ab79d434..afcba6671748 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -160,11 +160,11 @@ struct event_symbol event_symbols_tool[PERF_TOOL_LAST] = {
 		.alias  = "",
 	},
 	[PERF_TOOL_RU_UTIME] = {
-		.symbol = "rusage_user_time",
+		.symbol = "user_time",
 		.alias  = "ru_utime",
 	},
 	[PERF_TOOL_RU_STIME] = {
-		.symbol = "rusage_system_time",
+		.symbol = "system_time",
 		.alias  = "ru_stime",
 	},
 };
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 3c7227b8035c..7ee8613b6011 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -353,8 +353,8 @@ alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_AL
 emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
 dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
 duration_time					{ return tool(yyscanner, PERF_TOOL_DURATION_TIME); }
-rusage_user_time|ru_utime	{ return tool(yyscanner, PERF_TOOL_RU_UTIME); }
-rusage_system_time|ru_stime	{ return tool(yyscanner, PERF_TOOL_RU_STIME); }
+user_time|ru_utime	{ return tool(yyscanner, PERF_TOOL_RU_UTIME); }
+system_time|ru_stime	{ return tool(yyscanner, PERF_TOOL_RU_STIME); }
 bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
 cgroup-switches					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); }
 
-- 
2.35.1

Florian Fischer

  reply	other threads:[~2022-04-10 16:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-05 17:40 [RFC] perf stat: add rusage utime and stime events Florian Fischer
2022-04-05 17:40 ` [PATCH] " Florian Fischer
2022-04-05 19:42   ` Arnaldo Carvalho de Melo
2022-04-06  7:28     ` [RFC v2] " Florian Fischer
2022-04-06  7:28       ` [PATCH v2 1/4] perf stat: introduce stats for the user and system rusage times Florian Fischer
2022-04-06  7:28       ` [PATCH v2 2/4] perf stat: add rusage utime and stime events Florian Fischer
2022-04-07  1:39         ` Xing Zhengjun
2022-04-07  8:57           ` [RFC v3] " Florian Fischer
2022-04-07  8:57             ` [PATCH v3 1/3] perf stat: introduce stats for the user and system rusage times Florian Fischer
2022-04-07  8:57             ` [PATCH v3 2/3] perf stat: add rusage utime and stime events Florian Fischer
2022-04-08 15:58               ` Ian Rogers
2022-04-10 16:41                 ` Florian Fischer [this message]
2022-04-11 15:29                   ` Ian Rogers
2022-04-12 22:31                     ` Namhyung Kim
2022-04-07  8:57             ` [PATCH v3 3/3] perf list: print all available tool events Florian Fischer
2022-04-06  7:28       ` [PATCH v2 3/4] " Florian Fischer
2022-04-06  7:28       ` [PATCH v2 4/4] perf util: add 'us' unit to the rusage time events Florian Fischer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220410164136.bxqtpbrmfbqxdx4n@pasture \
    --to=florian.fischer@muhq.space \
    --cc=acme@kernel.org \
    --cc=flow@cs.fau.de \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=zhengjun.xing@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).