lkml.org 
[lkml]   [2015]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCHSET 0/4] perf report: Support folded callchain output (v4)
    On Tue, Nov 3, 2015 at 5:54 PM, Namhyung Kim <namhyung@kernel.org> wrote:
    > 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.

    Ok, good to know, thanks.

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

    Ah, right, sorry, I'd typed those the wrong way around. :)

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

    Thanks, looks almost right: a couple of minor changes:

    1. If perf already has the precedent of "PID:comm", instead of my
    "comm-PID", then maybe it should use "PID:comm" for perf consistency.
    Doesn't make much difference to me.
    2. The second space, delimiting "PID:comm" (or comm) and the stack...
    I'm nervous about using space as a delimiter any more than once, since
    it can also appear in comm (eg, "java main") and frames (eg,
    "JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*,
    Thread*)" -- that's direct from "perf script"!). I'd consider making
    it a semicolon:

    809 swapper-0;cpu_bringup_and_idle;cpu_startup_entry;...

    So the output is "value key", and key is a semicolon delimited stack
    with an optional comm or PID:comm frame at the start.

    Brendan


    \
     
     \ /
      Last update: 2015-11-04 07:41    [W:3.074 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site