perf tools: Support hierarchy report with event group (v2)
Namhyung Kim
maintainercover perf tools: Support hierarchy report with event group (v2)
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(-)
Signature · 1 line
2.9.3
Namhyung Kim
maintainerPATCH 1/7 perf hist: Introduce hists__match_hierarchy()
The hists__match_hierarchy() is to find matching hist entries in a group. A matching entry has same values for all sort keys given. With event group, a leader event should show other members in a group. So each entry in the leader should be able to find its pair entries which have same values. With hierarchy mode, it needs to search all matching children in a hierarchy.
An example output would look like below:
# Overhead Command / Shared Object / Symbol
# ...................... ..................................
#
25.74% 27.18% sh
19.96% 24.14% libc-2.24.so
9.55% 14.64% [.] __strcmp_sse2
1.54% 0.00% [.] __tfind
1.07% 1.13% [.] _int_malloc
...In the above example, two overhead were shown - one for leader and another for member. They were matched since their command, dso and symbol have same values.
--- tools/perf/util/hist.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
tools/perf/util/hist.c +51 -0
@@ -2174,6 +2174,51 @@ static struct hist_entry *hists__find_entry(struct hists *hists,return NULL ; } +static struct hist_entry *hists__find_hierarchy_entry (struct rb_root *root, +struct hist_entry *he) +{ +struct rb_node *n = root->rb_node; + +while (n) { +struct hist_entry *iter; +struct perf_hpp_fmt *fmt; +int64_t cmp =0 ; + + iter = rb_entry(n, struct hist_entry, rb_node_in); +perf_hpp_list__for_each_sort_list (he->hpp_list, fmt) { + cmp = fmt->collapse (fmt, iter, he); +if (cmp) +break ; + } + +if (cmp <0 ) + n = n->rb_left; +else if (cmp >0 ) + n = n->rb_right; +else +return iter; + } + +return NULL ; +} + +static void hists__match_hierarchy (struct rb_root *leader_root, +struct rb_root *other_root) +{ +struct rb_node *nd; +struct hist_entry *pos, *pair; + +for (nd =rb_first (leader_root); nd; nd =rb_next (nd)) { + pos = rb_entry(nd, struct hist_entry, rb_node_in); + pair =hists__find_hierarchy_entry (other_root, pos); + +if (pair) { +hist_entry__add_pair (pair, pos); +hists__match_hierarchy (&pos->hroot_in, &pair->hroot_in); + } + } +} +/* * Look for pairs to link to the leader buckets (hist_entries): */
@@ -2183,6 +2228,12 @@ void hists__match(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__match_hierarchy (&leader->entries_collapsed, + &other->entries_collapsed); + } +if (hists__has (leader, need_collapse)) root = &leader->entries_collapsed;else
Signature · 1 line
2.9.3
Re: [PATCH 1/7] perf hist: Introduce hists__match_hierarchy()
On Tue, Sep 13, 2016 at 04:45:46PM +0900, Namhyung Kim wrote:
SNIP
Quoted 6 lines
tools/perf/util/hist.c +6
-0+static struct hist_entry *hists__find_hierarchy_entry (struct rb_root *root, +struct hist_entry *he) +{ +struct rb_node *n = root->rb_node; + +while (n) {
tools/perf/util/hist.c static struct hist_entry *hists__find_entry(struct hists *hists, · line 2192
Quoted 10 lines
tools/perf/util/hist.c +10
-0+struct hist_entry *iter; +struct perf_hpp_fmt *fmt; +int64_t cmp =0 ; + + iter = rb_entry(n, struct hist_entry, rb_node_in); +perf_hpp_list__for_each_sort_list (he->hpp_list, fmt) { + cmp = fmt->collapse (fmt, iter, he); +if (cmp) +break ; + }
could you call hist_entry__collapse in here instead of above code?
thanks,
jirka
Quoted 12 lines
tools/perf/util/hist.c +12
-0+ +if (cmp <0 ) + n = n->rb_left; +else if (cmp >0 ) + n = n->rb_right; +else +return iter; + } + +return NULL ; +} +
Namhyung Kim
maintainerRe: [PATCH 1/7] perf hist: Introduce hists__match_hierarchy()
Hi Jiri,
Quoted 14 lines · Jiri Olsa
On Tue, Sep 13, 2016 at 04:45:46PM +0900, Namhyung Kim wrote:
SNIP
Quoted 10 lines
tools/perf/util/hist.c +10
-0+static struct hist_entry *hists__find_hierarchy_entry (struct rb_root *root, +struct hist_entry *he) +{ +struct rb_node *n = root->rb_node; + +while (n) { +struct hist_entry *iter; +struct perf_hpp_fmt *fmt; +int64_t cmp =0 ; +
Quoted 8 lines
tools/perf/util/hist.c static struct hist_entry *hists__find_entry(struct hists *hists, · line 2192
Quoted 6 lines
tools/perf/util/hist.c +6
-0+ iter = rb_entry(n, struct hist_entry, rb_node_in); +perf_hpp_list__for_each_sort_list (he->hpp_list, fmt) { + cmp = fmt->collapse (fmt, iter, he); +if (cmp) +break ; + }could you call
hist_entry__collapsein here instead of above code?
Nope. The hist_entry__collapse() traverses the whole sort list of the event (hists->hpp_list) while this function only traverses its own sort list (he->hpp_list). These are different when hierarchy is used.
Thanks,
Namhyung
Quoted 13 lines
Quoted 13 lines
tools/perf/util/hist.c +12
-0+ +if (cmp <0 ) + n = n->rb_left; +else if (cmp >0 ) + n = n->rb_right; +else +return iter; + } + +return NULL ; +} +
Signature · 2 lines
Thanks, Namhyung
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf hists: Introduce hists__match_hierarchy()
In reply to Namhyung Kimperf hists: Introduce hists__match_hierarchy()
Commit-ID: 09034de63e427a86ba96bedf39410eef7c9014a5 Gitweb: http://git.kernel.org/tip/09034de63e427a86ba96bedf39410eef7c9014a5 Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:46 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 13 Sep 2016 16:31:24 -0300
perf hists: Introduce hists__match_hierarchy()
The hists__match_hierarchy() is to find matching hist entries in a group. A matching entry has the same values for all sort keys given.
With an event group (e.g.: -e "{cycles,instructions}"), a leader event should show other members in a group. So each entry in the leader should be able to find its pair entries which have same values.
With hierarchy mode, it needs to search all matching children in a hierarchy.
An example output looks like:
# Overhead Command / Shared Object / Symbol
# ...................... ..................................
#
25.74% 27.18% sh
19.96% 24.14% libc-2.24.so
9.55% 14.64% [.] __strcmp_sse2
1.54% 0.00% [.] __tfind
1.07% 1.13% [.] _int_malloc
...In the above example, two overheads are shown - one for the leader and another for the other group member. They were matched since their command, dso and symbol have the same values.
--- tools/perf/util/hist.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)
tools/perf/util/hist.c +51 -0
@@ -2174,6 +2174,51 @@ static struct hist_entry *hists__find_entry(struct hists *hists,return NULL ; } +static struct hist_entry *hists__find_hierarchy_entry (struct rb_root *root, +struct hist_entry *he) +{ +struct rb_node *n = root->rb_node; + +while (n) { +struct hist_entry *iter; +struct perf_hpp_fmt *fmt; +int64_t cmp =0 ; + + iter = rb_entry(n, struct hist_entry, rb_node_in); +perf_hpp_list__for_each_sort_list (he->hpp_list, fmt) { + cmp = fmt->collapse (fmt, iter, he); +if (cmp) +break ; + } + +if (cmp <0 ) + n = n->rb_left; +else if (cmp >0 ) + n = n->rb_right; +else +return iter; + } + +return NULL ; +} + +static void hists__match_hierarchy (struct rb_root *leader_root, +struct rb_root *other_root) +{ +struct rb_node *nd; +struct hist_entry *pos, *pair; + +for (nd =rb_first (leader_root); nd; nd =rb_next (nd)) { + pos = rb_entry(nd, struct hist_entry, rb_node_in); + pair =hists__find_hierarchy_entry (other_root, pos); + +if (pair) { +hist_entry__add_pair (pair, pos); +hists__match_hierarchy (&pos->hroot_in, &pair->hroot_in); + } + } +} +/* * Look for pairs to link to the leader buckets (hist_entries): */
@@ -2183,6 +2228,12 @@ void hists__match(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__match_hierarchy (&leader->entries_collapsed, + &other->entries_collapsed); + } +if (hists__has (leader, need_collapse)) root = &leader->entries_collapsed;else
Namhyung Kim
maintainerPATCH 2/7 perf hist: Introduce hists__link_hierarchy()
In reply to Namhyung KimThe 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.
--- tools/perf/util/hist.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
tools/perf/util/hist.c +95 -0
@@ -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
Signature · 1 line
2.9.3
Re: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy()
Quoted 4 lines · Namhyung Kim
The
hists__link_hierarchy()is to support hierarchy report with event group. When it matches leader event and other members (with thehists__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
Quoted 16 lines
tools/perf/util/hist.c +15
-0+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) {
tools/perf/util/hist.c void hists__match(struct hists *leader, struct hists *other) · line 2319
Quoted 10 lines
tools/perf/util/hist.c +10
-0+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
Namhyung Kim
maintainerRe: [PATCH 2/7] perf hist: Introduce hists__link_hierarchy()
Quoted 29 lines · Jiri Olsa
Quoted 4 lines · Namhyung Kim
The
hists__link_hierarchy()is to support hierarchy report with event group. When it matches leader event and other members (with thehists__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
Quoted 21 lines
tools/perf/util/hist.c +20
-0+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 ; + } + }
Quoted 8 lines
tools/perf/util/hist.c void hists__match(struct hists *leader, struct hists *other) · line 2319
Quoted 5 lines
tools/perf/util/hist.c +5
-0+if (!found) +return -1 ; + }else { + leader =add_dummy_hierarchy_entry (leader_hists, + leader_root, pos);could we call
hists__add_dummy_entryin 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
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf hists: Introduce hists__link_hierarchy()
In reply to Namhyung Kimperf hists: Introduce hists__link_hierarchy()
Commit-ID: 9d97b8f512a0dd41819b8e3d9cdc7a59199e1b0c Gitweb: http://git.kernel.org/tip/9d97b8f512a0dd41819b8e3d9cdc7a59199e1b0c Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:47 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> 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.
--- tools/perf/util/hist.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+)
tools/perf/util/hist.c +95 -0
@@ -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
Namhyung Kim
maintainerPATCH 3/7 perf hist: Initialize hierachy tree explicitly
In reply to Namhyung KimThe 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.
--- tools/perf/util/hist.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/hist.c +2 -0
@@ -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;
Signature · 1 line
2.9.3
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf hist: Initialize hierarchy tree explicitly
perf hist: Initialize hierarchy tree explicitly
Commit-ID: d2580c7a5b4e78bffda1e53cfd583e7a2c7383a5 Gitweb: http://git.kernel.org/tip/d2580c7a5b4e78bffda1e53cfd583e7a2c7383a5 Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:48 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> 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.
--- tools/perf/util/hist.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/hist.c +2 -0
@@ -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;
Namhyung Kim
maintainerPATCH 4/7 perf ui/stdio: Always reset output width for hierarchy
In reply to Namhyung KimWhen --hierarchy option is used, each entry has its own hpp_list to show the result. But it missed to update width of each column for perf-top. The perf-report has no problem since it resets during header display.
$ sudo perf top --hierarchy --stdio
PerfTop: 160 irqs/sec kernel:38.8% exact: 100.0%
[4000Hz cycles:pp], (all, 12 CPUs)
----------------------------------------------------------------------
52.32% perf
24.74% [.] __symbols__insert
5.62% [.] rb_next
5.14% [.] dso__load_symMove the code into hists__fprintf() so that it can be called always. Also it'd be better to put similar code together.
--- tools/perf/ui/stdio/hist.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
tools/perf/ui/stdio/hist.c +6 -8
@@ -628,14 +628,6 @@ hists__fprintf_hierarchy_headers(struct hists *hists,struct perf_hpp *hpp, FILE *fp) { - struct perf_hpp_list_node *fmt_node; -struct perf_hpp_fmt *fmt; - -list_for_each_entry (fmt_node, &hists->hpp_formats, list) { -perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) -perf_hpp__reset_width (fmt, hists); - } - return print_hierarchy_header(hists, hpp, symbol_conf.field_sep, fp); }
@@ -733,6 +725,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows, bool use_callchain) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node;struct rb_node *nd;size_t ret =0 ;const char *sep = symbol_conf.field_sep;
@@ -745,6 +738,11 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,hists__for_each_format (hists, fmt)perf_hpp__reset_width (fmt, hists); +/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + }if (symbol_conf.col_width_list_str)perf_hpp__set_user_width (symbol_conf.col_width_list_str);
Signature · 1 line
2.9.3
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf ui/stdio: Always reset output width for hierarchy
perf ui/stdio: Always reset output width for hierarchy
Commit-ID: 9a6ad25b5a2026ba1399abc879ec623957867e79 Gitweb: http://git.kernel.org/tip/9a6ad25b5a2026ba1399abc879ec623957867e79 Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:49 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 13 Sep 2016 16:41:21 -0300
perf ui/stdio: Always reset output width for hierarchy
When the --hierarchy option is used, each entry has its own hpp_list to show the result. But it is not updating the width of each column for perf-top. The perf-report command has no problem since it resets it during header display.
$ sudo perf top --hierarchy --stdio
PerfTop: 160 irqs/sec kernel:38.8% exact: 100.0%
[4000Hz cycles:pp], (all, 12 CPUs)
----------------------------------------------------------------------
52.32% perf
24.74% [.] __symbols__insert
5.62% [.] rb_next
5.14% [.] dso__load_symMove the code into hists__fprintf() so that it can be called always. Also it'd be better to put similar code together.
--- tools/perf/ui/stdio/hist.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
tools/perf/ui/stdio/hist.c +6 -8
@@ -628,14 +628,6 @@ hists__fprintf_hierarchy_headers(struct hists *hists,struct perf_hpp *hpp, FILE *fp) { - struct perf_hpp_list_node *fmt_node; -struct perf_hpp_fmt *fmt; - -list_for_each_entry (fmt_node, &hists->hpp_formats, list) { -perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) -perf_hpp__reset_width (fmt, hists); - } - return print_hierarchy_header(hists, hpp, symbol_conf.field_sep, fp); }
@@ -733,6 +725,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows, bool use_callchain) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node;struct rb_node *nd;size_t ret =0 ;const char *sep = symbol_conf.field_sep;
@@ -745,6 +738,11 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,hists__for_each_format (hists, fmt)perf_hpp__reset_width (fmt, hists); +/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + }if (symbol_conf.col_width_list_str)perf_hpp__set_user_width (symbol_conf.col_width_list_str);
Namhyung Kim
maintainerPATCH 5/7 perf ui/stdio: Rename print_hierarchy_header()
In reply to Namhyung KimNow the hists__fprintf_hierarchy_headers() is a simple wrapper passing field separator. Let's do it directly.
--- tools/perf/ui/stdio/hist.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
tools/perf/ui/stdio/hist.c +3 -10
@@ -528,8 +528,8 @@ static int print_hierarchy_indent(const char *sep, int indent,return fprintf (fp,"%-.*s" , (indent -2 ) *HIERARCHY_INDENT , line); } -static int print_hierarchy_header (struct hists *hists,struct perf_hpp *hpp, -const char *sep,FILE *fp) +static int hists__fprintf_hierarchy_headers (struct hists *hists, +struct perf_hpp *hpp,FILE *fp) {bool first_node, first_col;int indent;
@@ -538,6 +538,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,unsigned header_width =0 ;struct perf_hpp_fmt *fmt;struct perf_hpp_list_node *fmt_node; +const char *sep = symbol_conf.field_sep; indent = hists->nr_hpp_node;
@@ -623,14 +624,6 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,return 2 ; } -static int -hists__fprintf_hierarchy_headers (struct hists *hists, -struct perf_hpp *hpp, -FILE *fp) -{ -return print_hierarchy_header (hists, hpp, symbol_conf.field_sep, fp); -} -static void fprintf_line (struct hists *hists,struct perf_hpp *hpp,int line,FILE *fp) {
Signature · 1 line
2.9.3
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf ui/stdio: Rename print_hierarchy_header()
perf ui/stdio: Rename print_hierarchy_header()
Commit-ID: 195bc0f8443d8d564ae95d2e9c19ac0edfd647c3 Gitweb: http://git.kernel.org/tip/195bc0f8443d8d564ae95d2e9c19ac0edfd647c3 Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:50 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 13 Sep 2016 16:41:36 -0300
perf ui/stdio: Rename print_hierarchy_header()
Now the hists__fprintf_hierarchy_headers() is a simple wrapper passing field separator. Let's do it directly.
--- tools/perf/ui/stdio/hist.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
tools/perf/ui/stdio/hist.c +3 -10
@@ -528,8 +528,8 @@ static int print_hierarchy_indent(const char *sep, int indent,return fprintf (fp,"%-.*s" , (indent -2 ) *HIERARCHY_INDENT , line); } -static int print_hierarchy_header (struct hists *hists,struct perf_hpp *hpp, -const char *sep,FILE *fp) +static int hists__fprintf_hierarchy_headers (struct hists *hists, +struct perf_hpp *hpp,FILE *fp) {bool first_node, first_col;int indent;
@@ -538,6 +538,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,unsigned header_width =0 ;struct perf_hpp_fmt *fmt;struct perf_hpp_list_node *fmt_node; +const char *sep = symbol_conf.field_sep; indent = hists->nr_hpp_node;
@@ -623,14 +624,6 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,return 2 ; } -static int -hists__fprintf_hierarchy_headers (struct hists *hists, -struct perf_hpp *hpp, -FILE *fp) -{ -return print_hierarchy_header (hists, hpp, symbol_conf.field_sep, fp); -} -static void fprintf_line (struct hists *hists,struct perf_hpp *hpp,int line,FILE *fp) {
Namhyung Kim
maintainerPATCH 6/7 perf ui/tui: Reset output width for hierarchy
In reply to Namhyung KimWhen --hierarchy option is used, each entry has its own hpp_list to show the result. But it missed to update width of each column.
--- tools/perf/ui/browsers/hists.c | 6 ++++++ 1 file changed, 6 insertions(+)
tools/perf/ui/browsers/hists.c +6 -0
@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser, struct hists *hists) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node; browser->hists = hists; browser->b.refresh = hist_browser__refresh;
@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,perf_hpp__reset_width (fmt, hists); ++browser->b.columns; } +/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + } }struct hist_browser *hist_browser__new (struct hists *hists)
Signature · 1 line
2.9.3
Re: [PATCH 6/7] perf ui/tui: Reset output width for hierarchy
Quoted 22 lines · Namhyung Kim
When --hierarchy option is used, each entry has its own
hpp_listto show the result. But it missed to update width of each column.Fixes 1b2dbbf41a0f ("perf hists: Use own hpp_list for hierarchy mode")Signed-off-by Namhyung Kim <[email protected]>--- tools/perf/ui/browsers/hists.c | 6 ++++++ 1 file changed, 6 insertions(+)tools/perf/ui/browsers/hists.c +1
-0@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser, struct hists *hists) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node; browser->hists = hists; browser->b.refresh = hist_browser__refresh;
tools/perf/ui/browsers/hists.c void hist_browser__init(struct hist_browser *browser, · line 2087
Quoted 8 lines
tools/perf/ui/browsers/hists.c +5
-0@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,perf_hpp__reset_width (fmt, hists); ++browser->b.columns; } +/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + }
we could add new function hists__reset_width and use it from both std and tui code
thanks,
jirka
Namhyung Kim
maintainerRe: [PATCH 6/7] perf ui/tui: Reset output width for hierarchy
Quoted 20 lines · Jiri Olsa
Quoted 19 lines · Namhyung Kim
When --hierarchy option is used, each entry has its own
hpp_listto show the result. But it missed to update width of each column.Fixes 1b2dbbf41a0f ("perf hists: Use own hpp_list for hierarchy mode")Signed-off-by Namhyung Kim <[email protected]>--- tools/perf/ui/browsers/hists.c | 6 ++++++ 1 file changed, 6 insertions(+)tools/perf/ui/browsers/hists.c +1
-0@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser, struct hists *hists) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node;
Quoted 14 lines
tools/perf/ui/browsers/hists.c void hist_browser__init(struct hist_browser *browser, · line 2087
Quoted 11 lines
tools/perf/ui/browsers/hists.c +5
-0browser->hists = hists; browser->b.refresh = hist_browser__refresh;@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,perf_hpp__reset_width (fmt, hists); ++browser->b.columns; } +/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + }we could add new function
hists__reset_widthand use it from both std and tui code
Will do as a follow-up patch!
Thanks,
Namhyung
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 6/7] perf ui/tui: Reset output width for hierarchy
Em Tue, Sep 20, 2016 at 10:18:50AM +0900, Namhyung Kim escreveu:
Quoted 8 lines · Jiri Olsa
Quoted 7 lines · Namhyung Kim
Quoted 5 lines
tools/perf/ui/browsers/hists.c +5
-0+/* hierarchy entries have their own hpp list */ +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format (&node->hpp, fmt) +perf_hpp__reset_width (fmt, hists); + }
Quoted 4 lines
Quoted 2 lines
we could add new function
hists__reset_widthand use it from both std and tui codeWill do as a follow-up patch!
Great, I saw no problems with the patches, merged them already, should be on their way to Ingo as soon as my test suite passes.
- Arnaldo
Namhyung Kim
maintainerPATCH 7/7 perf report: Enable group view with hierarchy
In reply to Namhyung KimNow all missing pieces are implemented, let's enable it. An example output below:
$ perf record -e '{cycles,instructions}' make $ perf report --hierarchy --stdio
...
# Overhead Command / Shared Object / Symbol
# ...................... ..................................
#
...
25.74% 27.18% sh
19.96% 24.14% libc-2.24.so
9.55% 14.64% [.] __strcmp_sse2
1.54% 0.00% [.] __tfind
1.07% 1.13% [.] _int_malloc
0.95% 0.00% [.] __strchr_sse2
0.89% 1.39% [.] __tsearch
0.76% 0.00% [.] strlen
...--- tools/perf/builtin-report.c | 1 - 1 file changed, 1 deletion(-)
tools/perf/builtin-report.c +0 -1
@@ -935,7 +935,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)if (symbol_conf.report_hierarchy) {/* disable incompatible options */ - symbol_conf.event_group = false; symbol_conf.cumulate_callchain = false;if (field_order) {
Signature · 1 line
2.9.3
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf report: Enable group view with hierarchy
perf report: Enable group view with hierarchy
Commit-ID: 30d476ae738d1ce33f170dd79398ecd211274df6 Gitweb: http://git.kernel.org/tip/30d476ae738d1ce33f170dd79398ecd211274df6 Author: Namhyung Kim <[email protected]> AuthorDate: Tue, 13 Sep 2016 16:45:52 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Tue, 13 Sep 2016 16:43:41 -0300
perf report: Enable group view with hierarchy
Now that all the missing pieces are implemented, let's enable it. An example output below:
$ perf record -e '{cycles,instructions}' make
$ perf report --hierarchy --stdio
...
# Overhead Command / Shared Object / Symbol
# ...................... ..................................
#
...
25.74% 27.18% sh
19.96% 24.14% libc-2.24.so
9.55% 14.64% [.] __strcmp_sse2
1.54% 0.00% [.] __tfind
1.07% 1.13% [.] _int_malloc
0.95% 0.00% [.] __strchr_sse2
0.89% 1.39% [.] __tsearch
0.76% 0.00% [.] strlen
...--- tools/perf/builtin-report.c | 1 - 1 file changed, 1 deletion(-)
tools/perf/builtin-report.c +0 -1
@@ -935,7 +935,6 @@ repeat:if (symbol_conf.report_hierarchy) {/* disable incompatible options */ - symbol_conf.event_group = false; symbol_conf.cumulate_callchain = false;if (field_order) {