All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>,
	Namhyung Kim <namhyung.kim@lge.com>,
	LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCHSET 0/9] perf tools: Fixup for the --percentage change
Date: Wed, 23 Apr 2014 08:09:38 +0200	[thread overview]
Message-ID: <20140423060938.GA20455@gmail.com> (raw)
In-Reply-To: <87k3agsla6.fsf@sejong.aot.lge.com>


* Namhyung Kim <namhyung@kernel.org> wrote:

> Hi Ingo,
> 
> On Tue, 22 Apr 2014 11:55:57 +0200, Ingo Molnar wrote:
> > I gave it some quick testing and after fixing a trivial merge conflict 
> > in tools/lib/lockdep/Makefile all seems to be working fine.
> 
> Thanks for testing!
> 
> >
> > But while looking at it I remembered one of my old UI complains about 
> > perf top and report, the hard to read nature of:
> >
> >    Event count (approx.): 226958779
> >
> > the values displayed are typically way too large to be easily human 
> > readable. More importantly, they are also nonsensical! That we have a 
> > sampling interval and can sum up all the intervals sampled has very 
> > little meaning to the overwhelming majority of humans looking at the 
> > data.
> >
> > And printing that just spams the visual field and confuses people.
> >
> > People care about the quality and speed of sampling itself, not 
> > directly the interval of sampling (which will often be variable with 
> > auto-freq sampling).
> 
> You meant 'period' by 'interval', right?

Yeah.

> There's --show-total-period option (should be equivalent to -F period
> later) in perf report, so there might be people want to see the numbers
> IMHO.
> 
> >
> > So instead of:
> >
> >   Samples: 42K of event 'cycles', Event count (approx.): 226958779
> >
> > How about only printing this in 'perf top' and 'perf report':
> >
> >   Captured 42.1K 'cycles' event samples
> >
> > Note the extra decimal (which helps monitor smaller changes as well), 
> > and note the different wording.
> >
> > Thoughts?
> 
> Well, I'm okay to add the extra decimal, but it seems that it only makes
> sense when the unit is 'K'..
> 
> And I think it might be worth adding filtered sample count as well if
> filtering is enabled something like:
> 
>   Captured 13.2K/42.1K 'cycles' event samples

Yeah. Maybe make it:

    Filtered 13.2K out of 42.1K 'cycles' event samples

or so.

Thanks,

	Ingo

  reply	other threads:[~2014-04-23  6:09 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-22  8:49 [PATCHSET 0/9] perf tools: Fixup for the --percentage change Namhyung Kim
2014-04-22  8:49 ` [PATCH 1/9] perf report: Count number of entries and samples separately Namhyung Kim
2014-04-22 14:51   ` Jiri Olsa
2014-04-23  4:52     ` Namhyung Kim
2014-04-22 16:43   ` Jiri Olsa
2014-04-22  8:49 ` [PATCH 2/9] perf hists: Introduce hists__add_nr_events() Namhyung Kim
2014-04-22 14:52   ` Jiri Olsa
2014-04-23  4:53     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 3/9] perf tools: Account entry stats when it's added to the output tree Namhyung Kim
2014-04-22 14:54   ` Jiri Olsa
2014-04-23  4:58     ` Namhyung Kim
2014-04-22 17:10   ` Jiri Olsa
2014-04-23  5:14     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 4/9] perf tools: Introduce hists__inc_dump_events() Namhyung Kim
2014-04-22 16:53   ` Jiri Olsa
2014-04-23  5:58     ` Namhyung Kim
2014-04-22  8:49 ` [PATCH 5/9] perf hists: Add missing update on nr_non_filtered_entries Namhyung Kim
2014-04-22  8:49 ` [PATCH 6/9] perf ui/tui: Fix off-by-one in hist_browser__update_nr_entries() Namhyung Kim
2014-04-22  8:49 ` [PATCH 7/9] perf ui/tui: Rename hist_browser__update_nr_entries() Namhyung Kim
2014-04-22  8:49 ` [PATCH 8/9] perf top/tui: Update nr_entries properly after a filter is applied Namhyung Kim
2014-04-22  8:49 ` [PATCH 9/9] perf hists/tui: Count callchain rows separately Namhyung Kim
2014-04-22 17:39   ` Jiri Olsa
2014-04-22  9:55 ` [PATCHSET 0/9] perf tools: Fixup for the --percentage change Ingo Molnar
2014-04-23  4:49   ` Namhyung Kim
2014-04-23  6:09     ` Ingo Molnar [this message]
2014-04-25  7:53       ` Namhyung Kim

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=20140423060938.GA20455@gmail.com \
    --to=mingo@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=andi@firstfloor.org \
    --cc=dsahern@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=namhyung@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.