All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>,
	Namhyung Kim <namhyung.kim@lge.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Stephane Eranian <eranian@google.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Rodrigo Campos <rodrigo@sdfg.com.ar>,
	Arun Sharma <asharma@fb.com>
Subject: Re: [RFC/PATCHSET 00/14] perf report: Add support to accumulate hist periods (v2)
Date: Wed, 6 Nov 2013 12:47:01 +0100	[thread overview]
Message-ID: <20131106114701.GA20249@gmail.com> (raw)
In-Reply-To: <87r4atx51i.fsf@sejong.aot.lge.com>


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

> On Wed, 6 Nov 2013 09:30:46 +0100, Ingo Molnar wrote:
> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >
> >> Hi Ingo,
> >> 
> >> On Tue, 5 Nov 2013 12:58:02 +0100, Ingo Molnar wrote:
> >> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >> >> But the 'cumulative' (btw, I feel a bit hard to type this word..) is 
> >> >> different in that it *generates* entries didn't get sampled originally. 
> >> >> And as it requires callchains, total field will not work if callchains 
> >> >> are missing.
> >> >
> >> > Well, 'total' should disappear if it's not available.
> >> 
> >> But what if it's the only sort key user gave?
> >
> > Do you mean something like:
> >
> >   -F self,name -s total
> >
> > i.e. if a sort key not displayed?
> 
> What I worry is when no -F option was given at all.

In that case the default list applied, plus whatever new fields are 
mentioned in -s would also be added (appended or prepended).

The display order of columns should _probably_ be something like:

  key1 key2 ... non-key1 non-key2

there's not much point in sorting and then displaying the key not in 
front, right?

> > I think sort keys should be automatically added to the displayed 
> > fields list.
> 
> Agreed.

> > This problem should be solved if all -s fields are displayed - i.e. 
> > they are added to the -F list, right?
> 
> But old users might not aware of the new -F option, and use -s option 
> only.  If so, she will get output like the first example, right?

Well, there's a default -F list that applies - so this shouldn't be a 
problem, agreed? So output should be like the second (expected) example.

> > Basically there's just a single concept: the -F list. The -s option 
> > simply modifies and extends the -F list but internally perf report 
> > would not know anything about '-s', it only knows about fields to 
> > display and it would know which of those fields are to be sorted and 
> > in what order.
> >
> > Does that make sense to you? Does it cover everything needed?
> 
> I like the concept.  I'm just looking for a way to add it without 
> upsetting old users. :)

If the default -F list matches our current displayed fields list then 
there should not be much change in behavior (beyond the addition of total 
for call-graph outputs - which can be kept completely separate).

I'm not too worried about call-graph 'legacies': it generates such huge 
perf.data files which is parsed so slowly at the moment that there's very 
little user base ... Anyone who absolutely needs call-graph profiling uses 
SysProf which performs well.

Thanks,

	Ingo

  reply	other threads:[~2013-11-06 11:47 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-31  6:56 [RFC/PATCHSET 00/14] perf report: Add support to accumulate hist periods (v2) Namhyung Kim
2013-10-31  6:56 ` [PATCH 01/14] perf tools: Consolidate __hists__add_*entry() Namhyung Kim
2013-11-01 11:56   ` Jiri Olsa
2013-11-06  5:43   ` [tip:perf/core] perf hists: " tip-bot for Namhyung Kim
2013-10-31  6:56 ` [PATCH 02/14] perf tools: Introduce struct add_entry_iter Namhyung Kim
2013-11-01 12:07   ` Jiri Olsa
2013-11-05  7:09     ` Namhyung Kim
2013-11-01 12:09   ` Jiri Olsa
2013-11-05  7:16     ` Namhyung Kim
2013-10-31  6:56 ` [PATCH 03/14] perf hists: Convert hist entry functions to use struct he_stat Namhyung Kim
2013-11-04 23:45   ` Arnaldo Carvalho de Melo
2013-11-05  7:17     ` Namhyung Kim
2013-10-31  6:56 ` [PATCH 04/14] perf hists: Add support for accumulated stat of hist entry Namhyung Kim
2013-10-31  6:56 ` [PATCH 05/14] perf hists: Check if accumulated when adding a " Namhyung Kim
2013-10-31  6:56 ` [PATCH 06/14] perf hists: Accumulate hist entry stat based on the callchain Namhyung Kim
2013-10-31  6:56 ` [PATCH 07/14] perf tools: Update cpumode for each cumulative entry Namhyung Kim
2013-11-01 12:55   ` Jiri Olsa
2013-11-05  7:41     ` Namhyung Kim
2013-10-31  6:56 ` [PATCH 08/14] perf report: Cache cumulative callchains Namhyung Kim
2013-10-31 11:13   ` Rodrigo Campos
2013-11-01  7:07     ` Namhyung Kim
2013-11-01 14:24       ` Rodrigo Campos
2013-11-01 15:16       ` Rodrigo Campos
2013-11-01 12:29   ` Jiri Olsa
2013-11-01 12:57     ` Jiri Olsa
2013-10-31  6:56 ` [PATCH 09/14] perf hists: Sort hist entries by accumulated period Namhyung Kim
2013-10-31  6:56 ` [PATCH 10/14] perf ui/hist: Add support to accumulated hist stat Namhyung Kim
2013-10-31  6:56 ` [PATCH 11/14] perf ui/browser: " Namhyung Kim
2013-10-31  6:56 ` [PATCH 12/14] perf ui/gtk: " Namhyung Kim
2013-10-31  6:56 ` [PATCH 13/14] perf tools: Apply percent-limit to cumulative percentage Namhyung Kim
2013-10-31  6:56 ` [PATCH 14/14] perf report: Add -g cumulative option Namhyung Kim
2013-11-01 13:17   ` Jiri Olsa
2013-11-05  7:44     ` Namhyung Kim
2013-10-31  8:09 ` [RFC/PATCHSET 00/14] perf report: Add support to accumulate hist periods (v2) Ingo Molnar
2013-11-01  6:48   ` Namhyung Kim
2013-11-01  7:55     ` Ingo Molnar
2013-11-01  9:18       ` Pekka Enberg
2013-11-01  9:22       ` Namhyung Kim
2013-11-01  9:27         ` Ingo Molnar
2013-11-05  7:31           ` Namhyung Kim
2013-11-05  7:46             ` Ingo Molnar
2013-11-05  9:05               ` Namhyung Kim
2013-11-05 11:58                 ` Ingo Molnar
2013-11-06  7:56                   ` Namhyung Kim
2013-11-06  8:30                     ` Ingo Molnar
2013-11-06  9:17                       ` Namhyung Kim
2013-11-06 11:47                         ` Ingo Molnar [this message]
2013-11-06 12:14                           ` Frederic Weisbecker
2013-11-11 12:13                             ` Ingo Molnar
2013-11-11 13:08                               ` Frederic Weisbecker
2013-11-11 13:56                                 ` Ingo Molnar
2013-11-11 15:45                                   ` Frederic Weisbecker
2013-11-06 15:33                           ` David Ahern
2013-11-11 12:19                             ` Ingo Molnar
2013-11-11 14:44                               ` David Ahern
2013-11-12 12:08                             ` Pekka Enberg
2013-11-06 16:09                           ` Peter Zijlstra
2013-11-11 12:17                             ` Ingo Molnar
2013-11-06 12:10                       ` Frederic Weisbecker
2013-11-11 12:12                         ` Ingo Molnar
2013-11-11 13:01                           ` Frederic Weisbecker
2013-11-04 13:27     ` Frederic Weisbecker
2013-11-04 13:23   ` Frederic Weisbecker
2013-11-04 13:34   ` Frederic Weisbecker

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=20131106114701.GA20249@gmail.com \
    --to=mingo@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=asharma@fb.com \
    --cc=eranian@google.com \
    --cc=fweisbec@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 \
    --cc=penberg@kernel.org \
    --cc=rodrigo@sdfg.com.ar \
    /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.