Date: Wed, 4 Nov 2015 10:54:33 +0900
From: Namhyung Kim <namhyung@kernel.org>
To: Brendan Gregg <brendan.d.gregg@gmail.com>
CC: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
        Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>, Jiri Olsa <jolsa@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>, David Ahern <dsahern@gmail.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Andi Kleen <andi@firstfloor.org>, Kan Liang <kan.liang@intel.com>
Subject: Re: [PATCHSET 0/4] perf report: Support folded callchain output (v4)
Message-ID: <20151104015433.GA19283@sejong>
References: <1446555131-25495-1-git-send-email-namhyung@kernel.org>
 <20151103144053.GL21609@kernel.org>
 <CAE40pdexwH=iwb5WhwGrQuzrBg=fys1A1YgA593oYVeABaVJgQ@mail.gmail.com>
MIME-Version: 1.0
In-Reply-To: <CAE40pdexwH=iwb5WhwGrQuzrBg=fys1A1YgA593oYVeABaVJgQ@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4448
Lines: 126

Hi Brendan,

On Tue, Nov 03, 2015 at 01:33:43PM -0800, Brendan Gregg wrote:
> On Tue, Nov 3, 2015 at 6:40 AM, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > Em Tue, Nov 03, 2015 at 09:52:07PM +0900, Namhyung Kim escreveu:
> >> Hello,
> >>
> >> This is what Brendan requested on the perf-users mailing list [1] to
> >> support FlameGraphs [2] more efficiently.  This patchset adds a few
> >> more callchain options to adjust the output for it.
> >>
> >>  * changes in v4)
> >>   - add missing doc update
> >>   - cleanup/fix callchain value print code
> >>   - add Acked-by from Brendan and Jiri
> >
> > Do those Acked-by stand? Things changed, the values moved from the end
> > of the line to the start, etc.
> >
> [...]
> 
> I'd Ack this change as it's a useful addition. It doesn't quite
> address the folded-only output, but it's a step in that direction. I
> think having the value at the start of a line only makes sense for the
> perf report output containing the hist summary lines, for consistency.

Right, thanks!


> 
> Here's how I'd shuffle the output of this patch (ignore word wrap
> issues with this email):
> 
> # ./perf report --stdio -g folded,count,caller -F pid | \
>     awk '/^ / { n = $1 }
>     /^[0-9]/ { split(n,a,":"); print a[2] "-" a[1] ";" $2,$1 }'
> swapper-0;cpu_bringup_and_idle;cpu_startup_entry;default_idle_call;arch_cpu_idle;default_idle;xen_hypercall_sched_op
> 809
> swapper-0;xen_start_kernel;x86_64_start_reservations;start_kernel;rest_init;cpu_startup_entry;default_idle_call;arch_cpu_idle;default_idle;xen_hypercall_sched_op
> 135
> dd-30551;__GI___libc_read;entry_SYSCALL_64_fastpath;sys_read;vfs_read;__vfs_read;urandom_read;extract_entropy_user;extract_buf;check_events;xen_hypercall_xen_version
> 63
> dd-30551;__GI___libc_read;entry_SYSCALL_64_fastpath;sys_read;vfs_read;__vfs_read;urandom_read;extract_entropy_user;extract_buf
> 54
> dd-30551;__GI___libc_read;entry_SYSCALL_64_fastpath;sys_read;vfs_read;__vfs_read;urandom_read;extract_entropy_user;extract_buf;memset_erms
> 3
> dd-30551;xen_irq_enable_direct_end;check_events;xen_hypercall_xen_version 3
> 
> So the output is folded stacks, prefixed by comm-PID. Shuffling the
> summarized output is a lot better than doing a "perf script" dump and
> re-processing call chains. (Note that since I'm using -F, I didn't
> need --no-children;

Nope.  The '-F pid' doesn't affect --children.  It doesn't show the
children overhead column but we still have hist entries for
(synthesized) children..

  $ perf report --no-children | wc -l
  998

  $ perf report --no-children -F pid,dso,sym | wc -l
  998

  $ perf report --children | wc -l
  3229

  $ perf report --children -F pid,dso,sym | wc -l
  3202

So I think you still need to use --no-children (or set report.children
config variable to false) for your script.


> and with "-g count", I didn't need --show-nr-samples.)

Yes, I used -n/--show-nr-samples just to check the number is correct.


> 
> I notice the fields (-F) option already has this precedent:
> 
> - "comm": prints PID:comm
> - "pid": prints PID

It's opposite:  "comm" prints comm, "pid" prints PID:comm. :)


> 
> If these were added to -g, along with a no-hists, then the two types
> of folded-only output could be generated using:
> 
> perf report --stdio -g folded,count,comm,no-hists,caller
> perf report --stdio -g folded,count,pid,no-hists,caller

As I said, using fields like comm, pid requires to have same keys in
--sort option.  So it's basically unreliable to use those specific
field names in the -g option IMHO.  I suggested to use 'info' (yes, it
needs better name) to print all sort keys.


> 
> ... although "no-hists" doesn't hit me as intuitive. How about "-F
> none" to specify zero columns? ie:
> 
> perf report --stdio -g folded,count,comm,caller -F none
> perf report --stdio -g folded,count,pid,caller -F none

Ah, makes sense.  So it'd look like

  $ perf report --stdio -g folded,count,info -F none -s comm
  $ perf report --stdio -g folded,count,info -F none -s pid

The output would be

  809 swapper-0 cpu_bringup_and_idle;cpu_startup_entry;default_idle_call;arch_cpu_idle;default_idle;xen_hypercall_sched_op

Thoughts?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/