From: Namhyung Kim <namhyung@kernel.org>
To: Greg Price <price@MIT.EDU>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
linux-kernel@vger.kernel.org,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@redhat.com>,
Jiri Olsa <jolsa@redhat.com>, David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH] perf report: Add option to collapse undesired parts of call graph
Date: Wed, 26 Jun 2013 10:28:56 +0900 [thread overview]
Message-ID: <871u7p3bjb.fsf@sejong.aot.lge.com> (raw)
In-Reply-To: <20130623031720.GW22203@biohazard-cafe.mit.edu> (Greg Price's message of "Sat, 22 Jun 2013 23:17:20 -0400")
Hi Greg,
On Sat, 22 Jun 2013 23:17:20 -0400, Greg Price wrote:
> For example, in an application with an expensive function
> implemented with deeply nested recursive calls, the default
> call-graph presentation is dominated by the different callchains
> within that function. By treating the function as a black box,
> we can collect the callchains leading into the function and
> compactly identify what to blame for expensive calls.
>
> For example, in this report the callers of garbage_collect() are
s/callers/callees/ ?
And it'd be better it shows more lines after garbage_collect so that one
can see its callers also to understand what it does more clearly.
> scattered across the tree:
> $ perf report -d ruby 2>- | grep -m10 ^[^#]*[a-z]
> 22.03% ruby [.] gc_mark
> --- gc_mark
> |--59.40%-- mark_keyvalue
> | st_foreach
> | gc_mark_children
> | |--99.75%-- rb_gc_mark
> | | rb_vm_mark
> | | gc_mark_children
> | | gc_marks
> | | |--99.00%-- garbage_collect
>
> If we make garbage_collect() a black box, its callers are coalesced:
Again, s/callers/callees/ ?
> $ perf report --blackbox garbage_collect -d ruby 2>- | grep -m10 ^[^#]*[a-z]
> 72.92% ruby [.] garbage_collect
> --- garbage_collect
> vm_xmalloc
> |--47.08%-- ruby_xmalloc
> | st_insert2
> | rb_hash_aset
> | |--98.45%-- features_index_add
> | | rb_provide_feature
> | | rb_require_safe
> | | vm_call_method
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Greg Price <price@mit.edu>
> ---
>
> On Fri, Jan 11, 2013 at 02:27:36AM -0300, Arnaldo Carvalho de Melo wrote:
>> Looks like an interesting feature, will review this soon,
>
> Rebased on top of v3.10-rc7, please take a look at your convenience.
>
>
> tools/perf/builtin-report.c | 19 ++++++++++++++++---
> tools/perf/builtin-top.c | 3 +--
> tools/perf/util/machine.c | 26 +++++++++++++++++---------
> tools/perf/util/machine.h | 9 ++++++++-
> tools/perf/util/session.c | 3 +--
You need to update the doc too.
> 5 files changed, 43 insertions(+), 17 deletions(-)
[SNIP]
> @@ -1211,8 +1214,13 @@ static int machine__resolve_callchain_sample(struct machine *machine,
> MAP__FUNCTION, ip, &al, NULL);
> if (al.sym != NULL) {
> if (sort__has_parent && !*parent &&
> - symbol__match_parent_regex(al.sym))
> + symbol__match_regex(al.sym, &parent_regex))
> *parent = al.sym;
> + else if (have_blackbox && root_al &&
> + symbol__match_regex(al.sym, &blackbox_regex)) {
> + *root_al = al;
> + callchain_cursor_reset(&callchain_cursor);
Okay, this is where the magic happens. :)
So it overwrites the original 'al' in process_sample_event() to
blackboxed symbol and drop the callchain. Wouldn't it deserve a
comment? :)
> + }
> if (!symbol_conf.use_callchain)
> break;
pp
This is unrelated to this patch, but why is this line needed? I guess
this check should be done before calling this function.
Thanks,
Namhyung
next prev parent reply other threads:[~2013-06-26 1:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-07 7:30 [PATCH] perf report: Add option to collapse undesired parts of call graph Greg Price
2013-01-11 5:27 ` Arnaldo Carvalho de Melo
2013-02-25 4:28 ` Greg Price
2013-06-23 3:17 ` Greg Price
2013-06-23 21:53 ` Jiri Olsa
2013-06-24 8:32 ` Ingo Molnar
2013-06-24 23:14 ` Greg Price
2013-06-25 7:47 ` Ingo Molnar
2013-06-25 8:01 ` Namhyung Kim
2013-06-26 22:41 ` Greg Price
2013-06-24 22:50 ` Greg Price
2013-06-26 1:28 ` Namhyung Kim [this message]
2013-06-26 22:25 ` Greg Price
2013-06-27 4:58 ` Namhyung Kim
2013-07-01 14:05 ` Greg Price
2013-07-01 14:08 ` [PATCH] perf report: Fix bug in case "--no-call-graph -p foo" Greg Price
2013-07-01 14:28 [PATCH v2] perf report/top: Add option to collapse undesired parts of call graph Greg Price
2013-07-07 13:13 ` Jiri Olsa
2013-07-08 11:57 ` Greg Price
2013-07-08 16:24 ` Jiri Olsa
2013-07-08 16:47 ` Arnaldo Carvalho de Melo
2013-07-19 7:50 ` [tip:perf/core] " tip-bot for Greg Price
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=871u7p3bjb.fsf@sejong.aot.lge.com \
--to=namhyung@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=dsahern@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=paulus@samba.org \
--cc=price@MIT.EDU \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).