From: Ingo Molnar <mingo@elte.hu>
To: Jim Cromie <jim.cromie@gmail.com>
Cc: linux-kernel@vger.kernel.org, a.p.zijlstra@chello.nl,
paulus@samba.org, acme@ghostprotocols.net
Subject: Re: [PATCH 2/2] dont commify big numbers by default, let -B do it
Date: Wed, 25 May 2011 14:44:01 +0200 [thread overview]
Message-ID: <20110525124401.GB29300@elte.hu> (raw)
In-Reply-To: <BANLkTikSMR7AyJi-LEr8iA2x9TxobJr9gA@mail.gmail.com>
* Jim Cromie <jim.cromie@gmail.com> wrote:
> > Also, i think for automation we'd also like to have a 'simple
> > output' mode, would you like to add that?
>
> OK, heres 1st cut at it, adding option --simple, for review /
> feedback
>
> Its based upon the csv-output code mostly, with some vestiges of
> verbose..
Ok, we can do something like this - but please see the comments on
the patch further below:
> > Something a bit like what you can see in 'perf stat -v true':
> > Without the human output later on, and with elapsed time added as
> > well.
>
> for my part, Id like the moral equivalent of time(s)? output too,
> though I suspect thats a separate patch..
yeah. No objection to that either: time is well known and the stddev
output by perf stat --repeat is well liked. Wanna combine the two?
> [jimc@groucho perf]$ time ./perf stat -x' ' -- sh -c 'sleep 3'
> task-clock-msecs6.830999
> context-switches 2
> CPU-migrations 1
> page-faults 477
> cycles 5404163 (scaled from 72.07%)
> instructions 3302699
> branches 804750
> branch-misses 51870
> cache-references 1584532 (scaled from 45.93%)
> cache-misses 36616 (scaled from 31.36%)
>
> real 0m3.019s
> user 0m0.005s
> sys 0m0.012s
>
> are these timings already taken by perf-stat ?
> is it a simple matter of addition and printing ?
> If not, whats involved ?
Do you mean the real/user/sys bits? Elapsed time is already measured,
but you'd have to extract the rusage bits to get to the stime/utime
values.
We could also add those as explicit events.
> Also, task-clock-msec doesnt quite match up with times' user number
> Whats going on here ?
task-clock-msec is generally much more accurate than the 0.005
printed by 'time'.
About the patch:
> - fprintf(logfp, " ( +- %7.3f%% )",
> - 100 * stddev_stats(&ps->res_stats[0]) / avg);
> + if (!csv_output)
> + fprintf(logfp, " ( +- %7.3f%% )",
> + 100 * stddev_stats(&ps->res_stats[0]) / avg);
> + else
> + fprintf(logfp, "%s%.3f%%",
> + csv_sep, 100 * stddev_stats(&ps->res_stats[0]) / avg);
We *really* want something cleaner here: a print_ops vector of
function pointers which would contain a handful of helper functions
called by the higher level code?
That way adding a new format method would not uglify the highlevel
code but could be done by adding a new print_ops 'driver' which would
be selected if the right option is provided.
Also, instead of adding -s/--simple we really want a single
print-format option that knows about all the format 'drivers':
--print simple
--print csv
--print default
--print verbose
or so.
Also, please run scripts/checkpatch.pl over your patches, the above
is i think not matching the coding style we use in the kernel (unless
webmail mangled your patch).
Thanks,
Ingo
next prev parent reply other threads:[~2011-05-25 12:44 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-21 8:56 [PATCH 0/2] perf stat: add -l <N> option to redirect stderr elsewhere Jim Cromie
2011-05-21 8:56 ` [PATCH 1/2] " Jim Cromie
2011-05-21 10:34 ` Ingo Molnar
2011-05-21 20:41 ` Arnaldo Carvalho de Melo
2011-09-01 21:58 ` Jim Cromie
2011-09-02 14:35 ` Arnaldo Carvalho de Melo
2011-09-02 18:02 ` Arnaldo Carvalho de Melo
2011-09-02 18:04 ` Jim Cromie
2011-09-02 18:58 ` Arnaldo Carvalho de Melo
2011-09-07 23:13 ` [patch 0/5] perf stat --log-fd=N Jim Cromie
2011-09-07 23:14 ` [PATCH 1/5] perf stat: add --log-fd <N> option to redirect stderr elsewhere Jim Cromie
2011-09-07 23:14 ` [PATCH 2/5] perf-stat: fix +- nan% in --no-aggr runs Jim Cromie
2011-09-07 23:14 ` [PATCH 3/5] perf stat: suppress printing std-dev when its 0 Jim Cromie
2011-09-07 23:14 ` [PATCH 4/5] perf stat: allow tab as cvs delimiter Jim Cromie
2011-09-07 23:14 ` [PATCH 5/5] perf stat: fix spelling in comment Jim Cromie
2011-05-21 8:56 ` [PATCH 2/2] dont commify big numbers by default, let -B do it Jim Cromie
2011-05-21 10:33 ` Ingo Molnar
2011-05-24 22:05 ` Jim Cromie
2011-05-24 22:11 ` [PATCH] add --simple output mode to perf-stat, based upon csv-output Jim Cromie
2011-05-25 12:44 ` Ingo Molnar [this message]
2011-05-26 19:41 ` [PATCH 2/2] dont commify big numbers by default, let -B do it Jim Cromie
2011-05-26 19:50 ` [PATCH 0/3] perf-stat: refactor print/formatting into print-ops Jim Cromie
2011-05-26 19:50 ` [PATCH 1/3] perf-stat: refactor print/formatting into print-ops for pretty, csv Jim Cromie
2011-05-26 19:50 ` [PATCH 2/3] perf-stat: fix +- nan% in -Aa runs Jim Cromie
2011-05-26 19:50 ` [PATCH 3/3] perf-stat: clean up <no count> handling, CPUx prefixing Jim Cromie
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=20110525124401.GB29300@elte.hu \
--to=mingo@elte.hu \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=jim.cromie@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=paulus@samba.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.