All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>, Pekka Enberg <penberg@kernel.org>
Cc: 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: Fri, 1 Nov 2013 08:55:02 +0100	[thread overview]
Message-ID: <20131101075502.GA25547@gmail.com> (raw)
In-Reply-To: <87vc0c4ny2.fsf@sejong.aot.lge.com>


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

> > A couple of details:
> >
> > 1)
> >
> > This is pretty close to SysProf output, right? So why not use the 
> > well-known SysProf naming and call the first column 'self' and the 
> > second column 'total'? I think those names are pretty intuitive and 
> > it would help people who come from SysProf over to perf.
> 
> Okay, I can do it. (Although sysprof seems to call it 'cumulative'
> rather than 'total' - but I think the 'total' is better since it's
> simpler and shorter.)

So sysprof-1.2 has the following two windows:

 'functions',   with 'self' and 'total' fields
 'descendants', with 'self' and 'cumulative' fields

'descendants' appears to be similar to the perf 'dso' concept.

> > 2)
> >
> > Is it possible to configure the default 'report -g' style, so that 
> > people who'd like to use it all the time don't have to type '-g 
> > cumulative' all the time?
> 
> Hmm.. maybe I can add support for the 'report.call-graph' config option.

If we display your new 'total' field by default then it's not as 
pressing to me :)

> > 3)
> >
> > I'd even argue that we enable this reporting feature by default, if 
> > a data file includes call-chain data: the first column will still 
> > show the well-known percentage that perf report produces today, the 
> > second column will be a new feature in essence.
> >
> > The only open question would be, by which column should we sort: 
> > 'sysprof style' sorts by 'total', 'perf style' sorts by 'self'. 
> > Agreed?
> 
> Right, I defaulted to go by 'total'.  But we can add an option for 
> it.

The purpose would be to allow people to do old-style 'sort by 
function overhead' output, while still seeing the 'total' field as 
well.

Btw., if anyone is interested in improving the GTK front-end, it 
would be _really_ nice if it had a 'start profiling' button like 
sysprof has today, with a 'samples' field showing the current number 
of samples. (We could even improve upon sysprof by adding 'stop' 
functionality as well ;-)

A bit like perf top, except the reporting session is hidden until 
the user actively requests the profile.

Maybe it could even be called a gtk version of 'perf top', with a 
button to start/stop collection, with another button to 
activate/deactivate reporting output, and yet another button to 
reset the profiling buffer.

With that feature set perf would be a ready sysprof workflow 
replacement I think. (I've Cc:-ed Pekka, just in case! :-)

> > 4)
> >
> > This is not directly related to the new feature you added: 
> > call-graph profiling still takes quite a bit of time. It might 
> > make sense to save the ordered histogram to a perf.data.ordered 
> > file, so that repeat invocations of 'perf report' don't have to 
> > recalculate everything again and again?
> >
> > This file would be maintained transparently and would only be 
> > re-created when the perf.data file changes, or something like 
> > that.
> 
> Hmm.. good idea.  We may discuss it along with Jiri's multiple 
> file storage patches.  I haven't had a time to review - maybe next 
> week.

So Arnaldo tells me that with your and Frederic's latest 
callgraph-speedup patches the parsing of perf.data got _really_ 
fast, so maybe my performance complaint is moot and we should delay 
complicating the primary perf.data file model with a 'cache' until 
your patches are in and we see the full impact.

Thanks,

	Ingo

  reply	other threads:[~2013-11-01  7:55 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 [this message]
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
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=20131101075502.GA25547@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.