Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1482218 > unrolled thread
| Started by | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| First post | 2016-09-13 09:50 +0200 |
| Last post | 2016-09-20 23:50 +0200 |
| Articles | 7 — 3 participants |
Back to article view | Back to linux.kernel
[PATCHSET 0/7] perf tools: Support hierarchy report with event group (v2) Namhyung Kim <namhyung@kernel.org> - 2016-09-13 09:50 +0200
[PATCH 3/7] perf hist: Initialize hierachy tree explicitly Namhyung Kim <namhyung@kernel.org> - 2016-09-13 09:50 +0200
[tip:perf/core] perf hist: Initialize hierarchy tree explicitly tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-09-20 23:50 +0200
[PATCH 2/7] perf hist: Introduce hists__link_hierarchy() Namhyung Kim <namhyung@kernel.org> - 2016-09-13 09:50 +0200
Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy() Jiri Olsa <jolsa@redhat.com> - 2016-09-19 10:10 +0200
Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy() Namhyung Kim <namhyung@kernel.org> - 2016-09-20 03:20 +0200
[tip:perf/core] perf hists: Introduce hists__link_hierarchy() tip-bot for Namhyung Kim <tipbot@zytor.com> - 2016-09-20 23:50 +0200
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-09-13 09:50 +0200 |
| Subject | [PATCHSET 0/7] perf tools: Support hierarchy report with event group (v2) |
| Message-ID | <sgQDD-7fb-3@gated-at.bofh.it> |
Hello,
This patchset implements hierarchy mode with event group. It was
disabled due to complexity when I wrote the hierarchy code, but
there's no fundamental reason to do it.
* changes in v2)
- the first bug fix patch applied to acme/perf/core
- update changelog, add Fixes: tags (Arnaldo)
- rename print_hierarchy_header
For example, following command line used to show each event
separately. As event group view is enabled by default, now hierarchy
mode shows the two event together like below:
$ perf record -e '{cycles,instructions}' make
$ perf report --hierarchy --stdio
...
# Overhead Command / Shared Object / Symbol
# ...................... ..................................
#
89.22% 87.73% cc1
80.69% 79.46% cc1
1.41% 2.00% [.] _cpp_lex_direct
1.26% 1.70% [.] bitmap_set_bit
1.11% 1.13% [.] ggc_internal_alloc
...
0.25% 0.01% [kernel.vmlinux]
0.23% 0.01% [k] page_fault
0.01% 0.00% [k] apic_timer_interrupt
0.01% 0.00% [k] entry_SYSCALL_64
...
5.03% 5.97% as
3.10% 3.84% as
0.14% 0.21% [.] 0x00000000000114f4
0.12% 0.01% [.] 0x0000000000011510
0.04% 0.00% [.] 0x000000000001c639
0.04% 0.00% [.] 0x000000000000cff6
...
3.32% 3.19% ld
2.34% 2.13% libbfd-2.27.so
0.50% 0.25% [.] bfd_link_hash_traverse
0.42% 0.30% [.] bfd_elf_link_add_symbols
0.41% 0.59% [.] bfd_hash_lookup
0.09% 0.04% [.] bfd_hash_insert
...
It's also available on 'perf/hierarchy-group-v2' branch in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (7):
perf hist: Introduce hists__match_hierarchy()
perf hist: Introduce hists__link_hierarchy()
perf hist: Initialize hierachy tree explicitly
perf ui/stdio: Always reset output width for hierarchy
perf ui/stdio: Rename print_hierarchy_header()
perf ui/tui: Reset output width for hierarchy
perf report: Enable group view with hierarchy
tools/perf/builtin-report.c | 1 -
tools/perf/ui/browsers/hists.c | 6 ++
tools/perf/ui/stdio/hist.c | 27 +++-----
tools/perf/util/hist.c | 148 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 163 insertions(+), 19 deletions(-)
--
2.9.3
[toc] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-09-13 09:50 +0200 |
| Subject | [PATCH 3/7] perf hist: Initialize hierachy tree explicitly |
| Message-ID | <sgQDD-7fb-13@gated-at.bofh.it> |
| In reply to | #1482218 |
The hroot_in and hroot_out are root of hiearchy tree of hist entry. But as hist entry is initialized by copying existing template entry, it sometimes has non-empty tree and copied it incorrectly. This is a problem especially when event group is used since it creates dummy entries from already-processed entries in other event members. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/hist.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 702ba3a8ead6..37a08f20730a 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -417,6 +417,8 @@ static int hist_entry__init(struct hist_entry *he, } INIT_LIST_HEAD(&he->pairs.node); thread__get(he->thread); + he->hroot_in = RB_ROOT; + he->hroot_out = RB_ROOT; if (!symbol_conf.report_hierarchy) he->leaf = true; -- 2.9.3
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Namhyung Kim <tipbot@zytor.com> |
|---|---|
| Date | 2016-09-20 23:50 +0200 |
| Subject | [tip:perf/core] perf hist: Initialize hierarchy tree explicitly |
| Message-ID | <sjB5o-8g-35@gated-at.bofh.it> |
| In reply to | #1482220 |
Commit-ID: d2580c7a5b4e78bffda1e53cfd583e7a2c7383a5 Gitweb: http://git.kernel.org/tip/d2580c7a5b4e78bffda1e53cfd583e7a2c7383a5 Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Tue, 13 Sep 2016 16:45:48 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 13 Sep 2016 16:36:46 -0300 perf hist: Initialize hierarchy tree explicitly The hroot_in and hroot_out are roots of hierarchy trees of hist entries. But when a hist entry is initialized by copying existing template entry, it sometimes has non-empty tree and copies it incorrectly. This is a problem especially when an event group is used since it creates dummy entries from already-processed entries in other event members. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Andi Kleen <andi@firstfloor.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/20160913074552.13284-4-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/hist.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 702ba3a..37a08f2 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -417,6 +417,8 @@ static int hist_entry__init(struct hist_entry *he, } INIT_LIST_HEAD(&he->pairs.node); thread__get(he->thread); + he->hroot_in = RB_ROOT; + he->hroot_out = RB_ROOT; if (!symbol_conf.report_hierarchy) he->leaf = true;
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-09-13 09:50 +0200 |
| Subject | [PATCH 2/7] perf hist: Introduce hists__link_hierarchy() |
| Message-ID | <sgQDE-7fb-29@gated-at.bofh.it> |
| In reply to | #1482218 |
The hists__link_hierarchy() is to support hierarchy report with event
group. When it matches leader event and other members (with the
hists__match_hierarchy), it also needs to link unmatched member entries
with a dummy leader event so that it can show up in the output.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/hist.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index be3f5ce31303..702ba3a8ead6 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2149,6 +2149,50 @@ static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
return he;
}
+static struct hist_entry *add_dummy_hierarchy_entry(struct hists *hists,
+ struct rb_root *root,
+ struct hist_entry *pair)
+{
+ struct rb_node **p;
+ struct rb_node *parent = NULL;
+ struct hist_entry *he;
+ struct perf_hpp_fmt *fmt;
+
+ p = &root->rb_node;
+ while (*p != NULL) {
+ int64_t cmp = 0;
+
+ parent = *p;
+ he = rb_entry(parent, struct hist_entry, rb_node_in);
+
+ perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
+ cmp = fmt->collapse(fmt, he, pair);
+ if (cmp)
+ break;
+ }
+ if (!cmp)
+ goto out;
+
+ if (cmp < 0)
+ p = &parent->rb_left;
+ else
+ p = &parent->rb_right;
+ }
+
+ he = hist_entry__new(pair, true);
+ if (he) {
+ rb_link_node(&he->rb_node_in, parent, p);
+ rb_insert_color(&he->rb_node_in, root);
+
+ he->dummy = true;
+ he->hists = hists;
+ memset(&he->stat, 0, sizeof(he->stat));
+ hists__inc_stats(hists, he);
+ }
+out:
+ return he;
+}
+
static struct hist_entry *hists__find_entry(struct hists *hists,
struct hist_entry *he)
{
@@ -2248,6 +2292,50 @@ void hists__match(struct hists *leader, struct hists *other)
}
}
+static int hists__link_hierarchy(struct hists *leader_hists,
+ struct hist_entry *parent,
+ struct rb_root *leader_root,
+ struct rb_root *other_root)
+{
+ struct rb_node *nd;
+ struct hist_entry *pos, *leader;
+
+ for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
+ pos = rb_entry(nd, struct hist_entry, rb_node_in);
+
+ if (hist_entry__has_pairs(pos)) {
+ bool found = false;
+
+ list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
+ if (leader->hists == leader_hists) {
+ found = true;
+ break;
+ }
+ }
+ if (!found)
+ return -1;
+ } else {
+ leader = add_dummy_hierarchy_entry(leader_hists,
+ leader_root, pos);
+ if (leader == NULL)
+ return -1;
+
+ /* do not point parent in the pos */
+ leader->parent_he = parent;
+
+ hist_entry__add_pair(pos, leader);
+ }
+
+ if (!pos->leaf) {
+ if (hists__link_hierarchy(leader_hists, leader,
+ &leader->hroot_in,
+ &pos->hroot_in) < 0)
+ return -1;
+ }
+ }
+ return 0;
+}
+
/*
* Look for entries in the other hists that are not present in the leader, if
* we find them, just add a dummy entry on the leader hists, with period=0,
@@ -2259,6 +2347,13 @@ int hists__link(struct hists *leader, struct hists *other)
struct rb_node *nd;
struct hist_entry *pos, *pair;
+ if (symbol_conf.report_hierarchy) {
+ /* hierarchy report always collapses entries */
+ return hists__link_hierarchy(leader, NULL,
+ &leader->entries_collapsed,
+ &other->entries_collapsed);
+ }
+
if (hists__has(other, need_collapse))
root = &other->entries_collapsed;
else
--
2.9.3
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2016-09-19 10:10 +0200 |
| Subject | Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy() |
| Message-ID | <sj1Oi-2Ug-19@gated-at.bofh.it> |
| In reply to | #1482222 |
On Tue, Sep 13, 2016 at 04:45:47PM +0900, Namhyung Kim wrote:
> The hists__link_hierarchy() is to support hierarchy report with event
> group. When it matches leader event and other members (with the
> hists__match_hierarchy), it also needs to link unmatched member entries
> with a dummy leader event so that it can show up in the output.
SNIP
>
> +static int hists__link_hierarchy(struct hists *leader_hists,
> + struct hist_entry *parent,
> + struct rb_root *leader_root,
> + struct rb_root *other_root)
> +{
> + struct rb_node *nd;
> + struct hist_entry *pos, *leader;
> +
> + for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
> + pos = rb_entry(nd, struct hist_entry, rb_node_in);
> +
> + if (hist_entry__has_pairs(pos)) {
> + bool found = false;
> +
> + list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
> + if (leader->hists == leader_hists) {
> + found = true;
> + break;
> + }
> + }
> + if (!found)
> + return -1;
> + } else {
> + leader = add_dummy_hierarchy_entry(leader_hists,
> + leader_root, pos);
could we call hists__add_dummy_entry in here? seems like there's only
difference in function arguments.. we could safe one function ;-)
thanks,
jirka
[toc] | [prev] | [next] | [standalone]
| From | Namhyung Kim <namhyung@kernel.org> |
|---|---|
| Date | 2016-09-20 03:20 +0200 |
| Subject | Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy() |
| Message-ID | <sjhT4-4IM-15@gated-at.bofh.it> |
| In reply to | #1486233 |
On Mon, Sep 19, 2016 at 4:59 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Tue, Sep 13, 2016 at 04:45:47PM +0900, Namhyung Kim wrote:
>> The hists__link_hierarchy() is to support hierarchy report with event
>> group. When it matches leader event and other members (with the
>> hists__match_hierarchy), it also needs to link unmatched member entries
>> with a dummy leader event so that it can show up in the output.
>
> SNIP
>
>>
>> +static int hists__link_hierarchy(struct hists *leader_hists,
>> + struct hist_entry *parent,
>> + struct rb_root *leader_root,
>> + struct rb_root *other_root)
>> +{
>> + struct rb_node *nd;
>> + struct hist_entry *pos, *leader;
>> +
>> + for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
>> + pos = rb_entry(nd, struct hist_entry, rb_node_in);
>> +
>> + if (hist_entry__has_pairs(pos)) {
>> + bool found = false;
>> +
>> + list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
>> + if (leader->hists == leader_hists) {
>> + found = true;
>> + break;
>> + }
>> + }
>> + if (!found)
>> + return -1;
>> + } else {
>> + leader = add_dummy_hierarchy_entry(leader_hists,
>> + leader_root, pos);
>
> could we call hists__add_dummy_entry in here? seems like there's only
> difference in function arguments.. we could safe one function ;-)
Maybe, but I don't see much value doing it. IMHO this hists
match/link thing is already complex. Combining normal and hierarchy
case in a single function will increase the complexity so I'd like to
keep it separate for hierarchy case.
Thanks,
Namhyung
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Namhyung Kim <tipbot@zytor.com> |
|---|---|
| Date | 2016-09-20 23:50 +0200 |
| Subject | [tip:perf/core] perf hists: Introduce hists__link_hierarchy() |
| Message-ID | <sjB5o-8g-45@gated-at.bofh.it> |
| In reply to | #1482222 |
Commit-ID: 9d97b8f512a0dd41819b8e3d9cdc7a59199e1b0c
Gitweb: http://git.kernel.org/tip/9d97b8f512a0dd41819b8e3d9cdc7a59199e1b0c
Author: Namhyung Kim <namhyung@kernel.org>
AuthorDate: Tue, 13 Sep 2016 16:45:47 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 13 Sep 2016 16:35:46 -0300
perf hists: Introduce hists__link_hierarchy()
The hists__link_hierarchy() is to support hierarchy reports with an
event group. When it matches the leader event and the other members
(using hists__match_hierarchy()), it also needs to link unmatched member
entries with a dummy leader event so that it can show up in the output.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20160913074552.13284-3-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/hist.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index be3f5ce..702ba3a 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2149,6 +2149,50 @@ out:
return he;
}
+static struct hist_entry *add_dummy_hierarchy_entry(struct hists *hists,
+ struct rb_root *root,
+ struct hist_entry *pair)
+{
+ struct rb_node **p;
+ struct rb_node *parent = NULL;
+ struct hist_entry *he;
+ struct perf_hpp_fmt *fmt;
+
+ p = &root->rb_node;
+ while (*p != NULL) {
+ int64_t cmp = 0;
+
+ parent = *p;
+ he = rb_entry(parent, struct hist_entry, rb_node_in);
+
+ perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
+ cmp = fmt->collapse(fmt, he, pair);
+ if (cmp)
+ break;
+ }
+ if (!cmp)
+ goto out;
+
+ if (cmp < 0)
+ p = &parent->rb_left;
+ else
+ p = &parent->rb_right;
+ }
+
+ he = hist_entry__new(pair, true);
+ if (he) {
+ rb_link_node(&he->rb_node_in, parent, p);
+ rb_insert_color(&he->rb_node_in, root);
+
+ he->dummy = true;
+ he->hists = hists;
+ memset(&he->stat, 0, sizeof(he->stat));
+ hists__inc_stats(hists, he);
+ }
+out:
+ return he;
+}
+
static struct hist_entry *hists__find_entry(struct hists *hists,
struct hist_entry *he)
{
@@ -2248,6 +2292,50 @@ void hists__match(struct hists *leader, struct hists *other)
}
}
+static int hists__link_hierarchy(struct hists *leader_hists,
+ struct hist_entry *parent,
+ struct rb_root *leader_root,
+ struct rb_root *other_root)
+{
+ struct rb_node *nd;
+ struct hist_entry *pos, *leader;
+
+ for (nd = rb_first(other_root); nd; nd = rb_next(nd)) {
+ pos = rb_entry(nd, struct hist_entry, rb_node_in);
+
+ if (hist_entry__has_pairs(pos)) {
+ bool found = false;
+
+ list_for_each_entry(leader, &pos->pairs.head, pairs.node) {
+ if (leader->hists == leader_hists) {
+ found = true;
+ break;
+ }
+ }
+ if (!found)
+ return -1;
+ } else {
+ leader = add_dummy_hierarchy_entry(leader_hists,
+ leader_root, pos);
+ if (leader == NULL)
+ return -1;
+
+ /* do not point parent in the pos */
+ leader->parent_he = parent;
+
+ hist_entry__add_pair(pos, leader);
+ }
+
+ if (!pos->leaf) {
+ if (hists__link_hierarchy(leader_hists, leader,
+ &leader->hroot_in,
+ &pos->hroot_in) < 0)
+ return -1;
+ }
+ }
+ return 0;
+}
+
/*
* Look for entries in the other hists that are not present in the leader, if
* we find them, just add a dummy entry on the leader hists, with period=0,
@@ -2259,6 +2347,13 @@ int hists__link(struct hists *leader, struct hists *other)
struct rb_node *nd;
struct hist_entry *pos, *pair;
+ if (symbol_conf.report_hierarchy) {
+ /* hierarchy report always collapses entries */
+ return hists__link_hierarchy(leader, NULL,
+ &leader->entries_collapsed,
+ &other->entries_collapsed);
+ }
+
if (hists__has(other, need_collapse))
root = &other->entries_collapsed;
else
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web