mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jim Cromie <jim.cromie@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
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: Thu, 26 May 2011 13:41:12 -0600	[thread overview]
Message-ID: <BANLkTimJp9o5_P73eNn5rMJbfRC2sCRdJA@mail.gmail.com> (raw)
In-Reply-To: <20110525124401.GB29300@elte.hu>

On Wed, May 25, 2011 at 6:44 AM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * 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?

I think so, depending upon what exactly 'combining' means.

>>
>> 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.

ISTM this makes sense - make them 1st class objects,
rather than some crufty fiddling in builtin-stats.
Can you give me a few hints re what/where to add ?

Also, is it sensible/practical to add self / +children flavors

>
>> 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:
> 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?

added struct print_ops:
0001-perf-stat-refactor-print-formatting-into-print-ops-f.patch
0002-perf-stat-fix-nan-in-Aa-runs.patch
0003-perf-stat-clean-up-no-count-handling-CPUx-prefixing.patch

sending separately, will attempt to in-reply-to them here.

> 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.

I left it as -x <delim> , since simple is basically csv.
in csv, "<not counted>" becomes "<not-counted>"
since latter makes -x ' ' useful.
also added -x '\t'  --> tab


> 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).

emacs did it.  I didnt fuss to correct cuz patch wasnt for prime-time.
Now corrected with .emacs additions from CodingStyle.
and checkpatched.

thanks

>
> Thanks,
>
>        Ingo
>

  reply	other threads:[~2011-05-26 19:41 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       ` [PATCH 2/2] dont commify big numbers by default, let -B do it Ingo Molnar
2011-05-26 19:41         ` Jim Cromie [this message]
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=BANLkTimJp9o5_P73eNn5rMJbfRC2sCRdJA@mail.gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®