perf tools: Support multiple keys in a single hierarchy level (v2)
Namhyung Kim
maintainercover perf tools: Support multiple keys in a single hierarchy level (v2)
Hello,
This implements what Arnaldo suggested in previous discussion of hierarchy patchset [1]. Originally each level in a hierarchy can have a single sort key in it, but with this patches it's possible to have more than one sort keys using a syntax similar to event grouping. I added the struct perf_hpp_list_node and carry it to group output formats (hpp_fmt) in a single level.
* Changes from v1)
- use '{ }' to group sort keys (Arnaldo)
- cleanup hpp_list_node creation (Jiri)
Example below shows how 4 sort keys are used for 2 levels. As you can see, the first level shows pid and comm of previous (switched) task and the second level shows pid and comm of next task.
$ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
--percent-limit 1 -i perf.data.sched
...
# Overhead prev_pid+prev_comm / next_pid+next_comm
# ........... .......................................
#
22.36% 0 swapper/0
9.48% 17773 transmission-gt
5.25% 109 kworker/0:1H
1.53% 6524 Xephyr
21.39% 17773 transmission-gt
9.52% 0 swapper/0
9.04% 0 swapper/2
1.78% 0 swapper/3It's available on the 'perf/hierarchy-multi-v2' branch in my tree
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Any feedbacks are welcome
Thanks,
Namhyung
[1] https://lkml.org/lkml/2016/2/24/1041
Namhyung Kim (8):
perf tools: Add level field to struct perf_hpp_fmt perf tools: Introduce perf_hpp__setup_hists_formats() perf tools: Use own hpp_list for hierarchy mode perf tools: Support multiple sort keys in a hierarchy perf tools: Fix indent for multiple hierarchy sort key perf report: Use hierarchy hpp list on stdio perf hists browser: Use hierarchy hpp list perf report: Use hierarchy hpp list on gtk
tools/perf/ui/browsers/hists.c | 147 +++++++++++++++++++---------------- tools/perf/ui/gtk/hists.c | 73 +++++++++++------- tools/perf/ui/hist.c | 67 ++++++++++++++++ tools/perf/ui/stdio/hist.c | 171 +++++++++++++++++++++-------------------- tools/perf/util/hist.c | 81 +++++++++++++------ tools/perf/util/hist.h | 12 +++ tools/perf/util/sort.c | 123 ++++++++++++++++++++--------- tools/perf/util/sort.h | 1 + 8 files changed, 438 insertions(+), 237 deletions(-)
Signature · 27 lines
2.7.1 *** BLURB HERE *** Namhyung Kim (8): perf tools: Add level field to struct perf_hpp_fmt perf tools: Introduce perf_hpp__setup_hists_formats() perf tools: Use own hpp_list for hierarchy mode perf tools: Support multiple sort keys in a hierarchy level perf tools: Fix indent for multiple hierarchy sort key perf report: Use hierarchy hpp list on stdio perf hists browser: Use hierarchy hpp list perf report: Use hierarchy hpp list on gtk tools/perf/ui/browsers/hists.c | 147 +++++++++++++++++++---------------- tools/perf/ui/gtk/hists.c | 73 +++++++++++------- tools/perf/ui/hist.c | 66 ++++++++++++++++ tools/perf/ui/stdio/hist.c | 171 +++++++++++++++++++++-------------------- tools/perf/util/hist.c | 81 +++++++++++++------ tools/perf/util/hist.h | 13 ++++ tools/perf/util/sort.c | 146 +++++++++++++++++++++++++---------- tools/perf/util/sort.h | 1 + 8 files changed, 455 insertions(+), 243 deletions(-) -- 2.7.2
Namhyung Kim
maintainerPATCH v2 1/8 perf tools: Add level field to struct perf_hpp_fmt
The level field is to distinguish levels in the hierarchy mode. Currently each column (perf_hpp_fmt) has a different level.
--- tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 74 ++++++++++++++++++++++++++++---------------------- 2 files changed, 42 insertions(+), 33 deletions(-)
tools/perf/util/hist.h +1 -0
@@ -233,6 +233,7 @@ struct perf_hpp_fmt {int len;int user_len;int idx; +int level; };struct perf_hpp_list {
tools/perf/util/sort.c +41 -33
272 lines not on this page · raw message
Signature · 1 line
2.7.2
tip-bot for Namhyung Kim
merge bot[tip:perf/core] perf hists: Add level field to struct perf_hpp_fmt
perf hists: Add level field to struct perf_hpp_fmt
Commit-ID: 4b633eba14627bcb1ef5c7a498e7dc308cd6a5d6 Gitweb: http://git.kernel.org/tip/4b633eba14627bcb1ef5c7a498e7dc308cd6a5d6 Author: Namhyung Kim <[email protected]> AuthorDate: Mon, 7 Mar 2016 16:44:43 -0300 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 8 Mar 2016 10:11:18 +0100
perf hists: Add level field to struct perf_hpp_fmt
The level field is to distinguish levels in the hierarchy mode. Currently each column (perf_hpp_fmt) has a different level.
--- tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 74 ++++++++++++++++++++++++++++---------------------- 2 files changed, 42 insertions(+), 33 deletions(-)
tools/perf/util/hist.h +1 -0
@@ -233,6 +233,7 @@ struct perf_hpp_fmt {int len;int user_len;int idx; +int level; };struct perf_hpp_list {
tools/perf/util/sort.c +41 -33
272 lines not on this page · raw message
Namhyung Kim
maintainerPATCH v2 2/8 perf tools: Introduce perf_hpp__setup_hists_formats()
In reply to Namhyung KimThe perf_hpp__setup_hists_formats() is to build hists-specific output formats (and sort keys). Currently it's only used in order to build the output format in a hierarchy with same sort keys, but it could be used with different sort keys in non-hierarchy mode later.
--- tools/perf/ui/hist.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/hist.c | 12 ++++++++++ tools/perf/util/hist.h | 11 +++++++++ tools/perf/util/sort.c | 32 +++++++++++++++++++++++++ 4 files changed, 120 insertions(+)
tools/perf/ui/hist.c +65 -0
@@ -5,6 +5,7 @@#include "../util/util.h" #include "../util/sort.h" #include "../util/evsel.h" +#include "../util/evlist.h" /* hist period print (hpp) functions */
@@ -715,3 +716,67 @@ void perf_hpp__set_user_width(const char *width_list_str)break ; } } + +static int add_hierarchy_fmt (struct hists *hists,struct perf_hpp_fmt *fmt) +{ +struct perf_hpp_list_node *node =NULL ; +struct perf_hpp_fmt *fmt_copy; +bool found = false; + +list_for_each_entry (node, &hists->hpp_formats, list) { +if (node->level == fmt->level) { + found = true; +break ; + } + } + +if (!found) { + node =malloc (sizeof (*node)); +if (node ==NULL ) +return -1 ; + + node->level = fmt->level; + +INIT_LIST_HEAD (&node->hpp.fields); +INIT_LIST_HEAD (&node->hpp.sorts); + +list_add_tail (&node->list, &hists->hpp_formats); + } + + fmt_copy =perf_hpp_fmt__dup (fmt); +if (fmt_copy ==NULL ) +return -1 ; + +list_add_tail (&fmt_copy->list, &node->hpp.fields); +list_add_tail (&fmt_copy->sort_list, &node->hpp.sorts); + +return 0 ; +} + +int perf_hpp__setup_hists_formats (struct perf_hpp_list *list, +struct perf_evlist *evlist) +{ +struct perf_evsel *evsel; +struct perf_hpp_fmt *fmt; +struct hists *hists; +int ret; + +if (!symbol_conf.report_hierarchy) +return 0 ; + +evlist__for_each (evlist, evsel) { + hists =evsel__hists (evsel); + +perf_hpp_list__for_each_sort_list (list, fmt) { +if (perf_hpp__is_dynamic_entry (fmt) && + !perf_hpp__defined_dynamic_entry (fmt, hists)) +continue ; + + ret =add_hierarchy_fmt (hists, fmt); +if (ret <0 ) +return ret; + } + } + +return 0 ; +}
tools/perf/util/hist.c +12 -0
@@ -2105,6 +2105,7 @@ int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list)pthread_mutex_init (&hists->lock,NULL ); hists->socket_filter =-1 ; hists->hpp_list = hpp_list; +INIT_LIST_HEAD (&hists->hpp_formats);return 0 ; }
@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)static void hists_evsel__exit (struct perf_evsel *evsel) {struct hists *hists =evsel__hists (evsel); +struct perf_hpp_fmt *fmt, *pos; +struct perf_hpp_list_node *node;hists__delete_all_entries (hists); + +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format_safe (&node->hpp, fmt, pos) { +list_del (&fmt->list); +free (fmt); + } +list_del (&node->list); +free (node); + } }static int hists_evsel__init (struct perf_evsel *evsel)
tools/perf/util/hist.h +11 -0
@@ -78,6 +78,7 @@ struct hists {u16 col_len[HISTC_NR_COLS ];int socket_filter;struct perf_hpp_list *hpp_list; +struct list_head hpp_formats;int nr_sort_keys; };
@@ -244,6 +245,12 @@ struct perf_hpp_list {extern struct perf_hpp_list perf_hpp_list; +struct perf_hpp_list_node { +struct list_head list; +struct perf_hpp_list hpp; +int level; +}; +void perf_hpp_list__column_register (struct perf_hpp_list *list,struct perf_hpp_fmt *format); void perf_hpp_list__register_sort_field(struct perf_hpp_list *list,
@@ -299,6 +306,8 @@ void perf_hpp__cancel_cumulate(void);void perf_hpp__setup_output_field (struct perf_hpp_list *list);void perf_hpp__reset_output_field (struct perf_hpp_list *list);void perf_hpp__append_sort_keys (struct perf_hpp_list *list); +int perf_hpp__setup_hists_formats (struct perf_hpp_list *list, +struct perf_evlist *evlist);bool perf_hpp__is_sort_entry (struct perf_hpp_fmt *format);
@@ -308,6 +317,8 @@ bool perf_hpp__is_trace_entry(struct perf_hpp_fmt *fmt);bool perf_hpp__is_srcline_entry (struct perf_hpp_fmt *fmt);bool perf_hpp__is_srcfile_entry (struct perf_hpp_fmt *fmt); +struct perf_hpp_fmt *perf_hpp_fmt__dup (struct perf_hpp_fmt *fmt); +int hist_entry__filter (struct hist_entry *he,int type,const void *arg); static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format,
tools/perf/util/sort.c +32 -0
@@ -1908,6 +1908,34 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,return hde; } +struct perf_hpp_fmt *perf_hpp_fmt__dup (struct perf_hpp_fmt *fmt) +{ +struct perf_hpp_fmt *new_fmt =NULL ; + +if (perf_hpp__is_sort_entry (fmt)) { +struct hpp_sort_entry *hse, *new_hse; + + hse = container_of(fmt, struct hpp_sort_entry, hpp); + new_hse =memdup (hse,sizeof (*hse)); +if (new_hse) + new_fmt = &new_hse->hpp; + }else if (perf_hpp__is_dynamic_entry (fmt)) { +struct hpp_dynamic_entry *hde, *new_hde; + + hde = container_of(fmt, struct hpp_dynamic_entry, hpp); + new_hde =memdup (hde,sizeof (*hde)); +if (new_hde) + new_fmt = &new_hde->hpp; + }else { + new_fmt =memdup (fmt,sizeof (*fmt)); + } + +INIT_LIST_HEAD (&new_fmt->list); +INIT_LIST_HEAD (&new_fmt->sort_list); + +return new_fmt; +} +static int parse_field_name (char *str,char **event,char **field,char **opt) {char *event_name, *field_name, *opt_name;
@@ -2700,6 +2728,10 @@ int setup_sorting(struct perf_evlist *evlist)/* and then copy output fields to sort keys */ perf_hpp__append_sort_keys (&perf_hpp_list); +/* setup hists-specific output fields */ +if (perf_hpp__setup_hists_formats (&perf_hpp_list, evlist) <0 ) +return -1 ; +return 0 ; }
Signature · 1 line
2.7.2
Arnaldo Carvalho de Melo
maintainerRe: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Em Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim escreveu:
Quoted 4 lines
The
perf_hpp__setup_hists_formats()is to build hists-specific output formats (and sort keys). Currently it's only used in order to build the output format in a hierarchy with same sort keys, but it could be used with different sort keys in non-hierarchy mode later.
After applying this one:
[root@jouet ~]# perf top --hierarchy # press 'q' to exit and get this:
perf: Segmentation fault
-------- backtrace --------
perf[0x54b60b]
/lib64/libc.so.6(+0x34b20)[0x7fb7e06e9b20]
perf[0x4e9780]
perf(perf_evsel__delete+0x20)[0x498450] perf(perf_evlist__delete+0x182)[0x48fbe2] perf(cmd_top+0xc93)[0x440893]
perf[0x481c01]
perf(main+0x67a)[0x423dea] /lib64/libc.so.6(__libc_start_main+0xf0)[0x7fb7e06d5580] perf(_start+0x29)[0x423f09]
[0x0]
[root@jouet ~]#
rebuild it with DEBUG=1, run it on gdb, press 'q' to exit 'perf top':
(gdb) run top --stdio --hierarchy
Program received signal SIGSEGV, Segmentation fault.0x000000000051632b in hists_evsel__exit (evsel=0x19b95b0) at util/hist.c:2143
2143 perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos) {
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-19.fc23.x86_64 glibc-2.22-10.fc23.x86_64 libunwind-1.1-10.fc23.x86_64 nss-softokn-freebl-3.22.0-1.0.fc23.x86_64 numactl-libs-2.0.10-3.fc23.x86_64 perl-libs-5.22.1-350.fc23.x86_64 python-libs-2.7.10-8.fc23.x86_64 slang-2.3.0-4.fc23.x86_64 (gdb) bt
#0 0x000000000051632b in hists_evsel__exit (evsel=0x19b95b0) at util/hist.c:2143 #1 0x00000000004b5a2c in perf_evsel__exit (evsel=0x19b95b0) at util/evsel.c:1094 #2 0x00000000004b5a6a in perf_evsel__delete (evsel=0x19b95b0) at util/evsel.c:1099 #3 0x00000000004adf5d in perf_evlist__purge (evlist=0x19b8b90) at util/evlist.c:115 #4 0x00000000004ae090 in perf_evlist__delete (evlist=0x19b8b90) at util/evlist.c:135 #5 0x0000000000446547 in cmd_top (argc=0, argv=0x7fffffffe310, prefix=0x0) at builtin-top.c:1346 #6 0x000000000049e614 in run_builtin (p=0x90f5e0 <commands+288>, argc=3, argv=0x7fffffffe310) at perf.c:390 #7 0x000000000049e87c in handle_internal_command (argc=3, argv=0x7fffffffe310) at perf.c:451 #8 0x000000000049e9c1 in run_argv (argcp=0x7fffffffe16c, argv=0x7fffffffe160) at perf.c:497 #9 0x000000000049ed67 in main (argc=3, argv=0x7fffffffe310) at perf.c:624
(gdb)
Arnaldo Carvalho de Melo
maintainerRe: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Em Fri, Mar 04, 2016 at 06:58:22PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 8 lines
Em Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim escreveu:
Quoted 4 lines
The
perf_hpp__setup_hists_formats()is to build hists-specific output formats (and sort keys). Currently it's only used in order to build the output format in a hierarchy with same sort keys, but it could be used with different sort keys in non-hierarchy mode later.After applying this one:
Quoted 7 lines
rebuild it with DEBUG=1, run it on gdb, press 'q' to exit 'perf top':
(gdb) run top --stdio --hierarchy
Program received signal SIGSEGV, Segmentation fault.0x000000000051632binhists_evsel__exit(evsel=0x19b95b0) atutil/hist.c:2143
2143perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos){
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-19.fc23.x86_64 glibc-2.22-10.fc23.x86_64 libunwind-1.1-10.fc23.x86_64 nss-softokn-freebl-3.22.0-1.0.fc23.x86_64 numactl-libs-2.0.10-3.fc23.x86_64 perl-libs-5.22.1-350.fc23.x86_64 python-libs-2.7.10-8.fc23.x86_64 slang-2.3.0-4.fc23.x86_64
Please continue from acme/perf/core, the first patch is there already.
Thanks,
- Arnaldo
Namhyung Kim
maintainerRe: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Quoted 11 lines · Arnaldo Carvalho de Melo
Em Fri, Mar 04, 2016 at 06:58:22PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 9 lines
Em Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim escreveu:
Quoted 4 lines
The
perf_hpp__setup_hists_formats()is to build hists-specific output formats (and sort keys). Currently it's only used in order to build the output format in a hierarchy with same sort keys, but it could be used with different sort keys in non-hierarchy mode later.After applying this one:
rebuild it with DEBUG=1, run it on gdb, press 'q' to exit 'perf top':
Quoted 7 lines
Quoted 5 lines
(gdb) run top --stdio --hierarchy
Program received signal SIGSEGV, Segmentation fault.0x000000000051632binhists_evsel__exit(evsel=0x19b95b0) atutil/hist.c:2143
2143perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos){
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-19.fc23.x86_64 glibc-2.22-10.fc23.x86_64 libunwind-1.1-10.fc23.x86_64 nss-softokn-freebl-3.22.0-1.0.fc23.x86_64 numactl-libs-2.0.10-3.fc23.x86_64 perl-libs-5.22.1-350.fc23.x86_64 python-libs-2.7.10-8.fc23.x86_64 slang-2.3.0-4.fc23.x86_64Please continue from acme/perf/core, the first patch is there already.
OK, I'll take a look at the problem.
Thanks,
Namhyung
Namhyung Kim
maintainerRe: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
In reply to Arnaldo Carvalho de MeloQuoted 42 lines · Arnaldo Carvalho de Melo
Em Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim escreveu:
Quoted 4 lines
The
perf_hpp__setup_hists_formats()is to build hists-specific output formats (and sort keys). Currently it's only used in order to build the output format in a hierarchy with same sort keys, but it could be used with different sort keys in non-hierarchy mode later.After applying this one:
[root@jouet ~]# perf top --hierarchy # press 'q' to exit and get this:
perf: Segmentation fault
-------- backtrace --------
perf[0x54b60b]
/lib64/libc.so.6(+0x34b20)[0x7fb7e06e9b20]
perf[0x4e9780]
perf(perf_evsel__delete+0x20)[0x498450] perf(perf_evlist__delete+0x182)[0x48fbe2] perf(cmd_top+0xc93)[0x440893]
perf[0x481c01]
perf(main+0x67a)[0x423dea] /lib64/libc.so.6(__libc_start_main+0xf0)[0x7fb7e06d5580] perf(_start+0x29)[0x423f09]
[0x0]
[root@jouet ~]#rebuild it with DEBUG=1, run it on gdb, press 'q' to exit 'perf top':
(gdb) run top --stdio --hierarchy
Program received signal SIGSEGV, Segmentation fault.0x000000000051632binhists_evsel__exit(evsel=0x19b95b0) atutil/hist.c:2143
2143perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos){
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-19.fc23.x86_64 glibc-2.22-10.fc23.x86_64 libunwind-1.1-10.fc23.x86_64 nss-softokn-freebl-3.22.0-1.0.fc23.x86_64 numactl-libs-2.0.10-3.fc23.x86_64 perl-libs-5.22.1-350.fc23.x86_64 python-libs-2.7.10-8.fc23.x86_64 slang-2.3.0-4.fc23.x86_64 (gdb) bt#0 0x000000000051632b in hists_evsel__exit (evsel=0x19b95b0) at util/hist.c:2143 #1 0x00000000004b5a2c in perf_evsel__exit (evsel=0x19b95b0) at util/evsel.c:1094 #2 0x00000000004b5a6a in perf_evsel__delete (evsel=0x19b95b0) at util/evsel.c:1099 #3 0x00000000004adf5d in perf_evlist__purge (evlist=0x19b8b90) at util/evlist.c:115 #4 0x00000000004ae090 in perf_evlist__delete (evlist=0x19b8b90) at util/evlist.c:135 #5 0x0000000000446547 in cmd_top (argc=0, argv=0x7fffffffe310, prefix=0x0) at builtin-top.c:1346 #6 0x000000000049e614 in run_builtin (p=0x90f5e0 <commands+288>, argc=3, argv=0x7fffffffe310) at perf.c:390 #7 0x000000000049e87c in handle_internal_command (argc=3, argv=0x7fffffffe310) at perf.c:451 #8 0x000000000049e9c1 in run_argv (argcp=0x7fffffffe16c, argv=0x7fffffffe160) at perf.c:497 #9 0x000000000049ed67 in main (argc=3, argv=0x7fffffffe310) at perf.c:624
Quoted 1 line
(gdb)
Nah, it was silly of me to use list_for_each_entry() for deletion.. Will send a fix soon.
Thanks,
Namhyung
Re: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
In reply to Namhyung KimOn Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim wrote:
SNIP
Quoted 26 lines
tools/perf/util/hist.c +11
-0@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)static void hists_evsel__exit (struct perf_evsel *evsel) {struct hists *hists =evsel__hists (evsel); +struct perf_hpp_fmt *fmt, *pos; +struct perf_hpp_list_node *node;hists__delete_all_entries (hists); + +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format_safe (&node->hpp, fmt, pos) { +list_del (&fmt->list); +free (fmt); + } +list_del (&node->list); +free (node); + } }static int hists_evsel__init (struct perf_evsel *evsel)tools/perf/util/hist.h +0
-0
tools/perf/util/hist.h struct hists { · line 81
Quoted 4 lines
tools/perf/util/hist.h +1
-0@@ -78,6 +78,7 @@ struct hists {u16 col_len[HISTC_NR_COLS ];int socket_filter;struct perf_hpp_list *hpp_list; +struct list_head hpp_formats;
I've been thinking.. should hpp_formats and hpp_list be merged? something like:
struct perf_hpp_list {
struct list_head nodes;
int levels;
}sturct perf_hpp_list_node {
int level;
struct list_head fields;
struct list_head sorts;
int nr_sort_keys;
};it seems wrong to me that your hierarchy code and current one got in separate paths.. we could have the new hierarchy support struct above being used in current non-hierarchy code just by using single level
I haven't thought all this through.. just an idea ;-)
thanks,
jirka
Namhyung Kim
maintainerRe: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Quoted 49 lines · Jiri Olsa
On Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim wrote:
SNIP
Quoted 26 lines
tools/perf/util/hist.c +11
-0@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)static void hists_evsel__exit (struct perf_evsel *evsel) {struct hists *hists =evsel__hists (evsel); +struct perf_hpp_fmt *fmt, *pos; +struct perf_hpp_list_node *node;hists__delete_all_entries (hists); + +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format_safe (&node->hpp, fmt, pos) { +list_del (&fmt->list); +free (fmt); + } +list_del (&node->list); +free (node); + } }static int hists_evsel__init (struct perf_evsel *evsel)tools/perf/util/hist.h +0
-0tools/perf/util/hist.h struct hists { · line 81
Quoted 4 lines
tools/perf/util/hist.h +1
-0@@ -78,6 +78,7 @@ struct hists {u16 col_len[HISTC_NR_COLS ];int socket_filter;struct perf_hpp_list *hpp_list; +struct list_head hpp_formats;I've been thinking.. should
hpp_formatsandhpp_listbe merged? something like:struct perf_hpp_list { struct list_head nodes; int levels; }sturct perf_hpp_list_node { int level; struct list_head fields; struct list_head sorts; int nr_sort_keys; };
Quoted 6 lines
it seems wrong to me that your hierarchy code and current one got in separate paths.. we could have the new hierarchy support struct above being used in current non-hierarchy code just by using single level
I haven't thought all this through.. just an idea ;-)
Yes, I also think that we need to converge on the same data structure eventually. But I think it's too intrusive to be merged at once. Also keeping up with the latest perf changes is a PITA. ;-)
So I decided to use it on the hierarchy first, and get tested in the wild for some time. After that we can convert the non-hierarchy code to use same path and data structure gradually IMHO.
Thanks,
Namhyung
Re: [PATCH v2 2/8] perf tools: Introduce perf_hpp__setup_hists_formats()
Quoted 55 lines · Namhyung Kim
Quoted 54 lines · Jiri Olsa
On Fri, Mar 04, 2016 at 11:59:36PM +0900, Namhyung Kim wrote:
SNIP
Quoted 26 lines
tools/perf/util/hist.c +11
-0@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)static void hists_evsel__exit (struct perf_evsel *evsel) {struct hists *hists =evsel__hists (evsel); +struct perf_hpp_fmt *fmt, *pos; +struct perf_hpp_list_node *node;hists__delete_all_entries (hists); + +list_for_each_entry (node, &hists->hpp_formats, list) { +perf_hpp_list__for_each_format_safe (&node->hpp, fmt, pos) { +list_del (&fmt->list); +free (fmt); + } +list_del (&node->list); +free (node); + } }static int hists_evsel__init (struct perf_evsel *evsel)tools/perf/util/hist.h +0
-0tools/perf/util/hist.h struct hists { · line 81
Quoted 4 lines
tools/perf/util/hist.h +1
-0@@ -78,6 +78,7 @@ struct hists {u16 col_len[HISTC_NR_COLS ];int socket_filter;struct perf_hpp_list *hpp_list; +struct list_head hpp_formats;I've been thinking.. should
hpp_formatsandhpp_listbe merged? something like:struct perf_hpp_list { struct list_head nodes; int levels; }sturct perf_hpp_list_node { int level; struct list_head fields; struct list_head sorts; int nr_sort_keys; };it seems wrong to me that your hierarchy code and current one got in separate paths.. we could have the new hierarchy support struct above being used in current non-hierarchy code just by using single level
Quoted 5 lines
Quoted 1 line
I haven't thought all this through.. just an idea ;-)
Yes, I also think that we need to converge on the same data structure eventually. But I think it's too intrusive to be merged at once. Also keeping up with the latest perf changes is a PITA. ;-)
heh, amen ;-)
Quoted 4 lines
So I decided to use it on the hierarchy first, and get tested in the wild for some time. After that we can convert the non-hierarchy code to use same path and data structure gradually IMHO.
ok, let's keep it in mind at least..
thanks.
jirka
Namhyung Kim
maintainerPATCH v2 3/8 perf tools: Use own hpp_list for hierarchy mode
In reply to Namhyung KimNow each hists has its own hpp lists in hierarchy. So instead of having a pointer to a single perf_hpp_fmt in a hist entry, make it point the hpp_list for its level. This will be used to support multiple sort keys in a single hierarchy level.
--- tools/perf/ui/browsers/hists.c | 45 +++++++++++++++------------ tools/perf/ui/gtk/hists.c | 20 ++++++++---- tools/perf/ui/stdio/hist.c | 44 +++++++++++++-------------- tools/perf/util/hist.c | 69 +++++++++++++++++++++++++++--------------- tools/perf/util/sort.h | 1 + 5 files changed, 107 insertions(+), 72 deletions(-)
tools/perf/ui/browsers/hists.c +25 -20
@@ -1388,25 +1388,26 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser, HE_COLORSET_NORMAL); } -ui_browser__write_nstring (&browser->b,"" ,2 ); - width -=2 ; +perf_hpp_list__for_each_format (entry->hpp_list, fmt) { +ui_browser__write_nstring (&browser->b,"" ,2 ); + width -=2 ; -/* -* No need to call hist_entry__snprintf_alignment() -* since this fmt is always the last column in the -* hierarchy mode. -*/ - fmt = entry->fmt; -if (fmt->color) { - width -= fmt->color (fmt, &hpp, entry); - }else { -int i =0 ; +/* +* No need to call hist_entry__snprintf_alignment() +* since this fmt is always the last column in the +* hierarchy mode. +*/ +if (fmt->color) { + width -= fmt->color (fmt, &hpp, entry); + }else { +int i =0 ; - width -= fmt->entry (fmt, &hpp, entry); -ui_browser__printf (&browser->b,"%s" ,ltrim (s)); + width -= fmt->entry (fmt, &hpp, entry); +ui_browser__printf (&browser->b,"%s" ,ltrim (s)); -while (isspace (s[i++])) - width++; +while (isspace (s[i++])) + width++; + } } }
@@ -1934,7 +1935,7 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,struct perf_hpp_fmt *fmt;bool first = true;int ret; -int hierarchy_indent = (nr_sort_keys +1 ) *HIERARCHY_INDENT ; +int hierarchy_indent = nr_sort_keys *HIERARCHY_INDENT ; printed =fprintf (fp,"%*s" , level *HIERARCHY_INDENT ,"" );
@@ -1962,9 +1963,13 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser, ret =scnprintf (hpp.buf, hpp.size,"%*s" , hierarchy_indent,"" );advance_hpp (&hpp, ret); - fmt = he->fmt; - ret = fmt->entry (fmt, &hpp, he); -advance_hpp (&hpp, ret); +perf_hpp_list__for_each_format (he->hpp_list, fmt) { + ret =scnprintf (hpp.buf, hpp.size," " ); +advance_hpp (&hpp, ret); + + ret = fmt->entry (fmt, &hpp, he); +advance_hpp (&hpp, ret); + } printed +=fprintf (fp,"%s\n" ,rtrim (s));
tools/perf/ui/gtk/hists.c +14 -6
@@ -412,6 +412,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,for (node =rb_first (root); node; node =rb_next (node)) {GtkTreeIter iter;float percent; +char *bf; he = rb_entry(node, struct hist_entry, rb_node);if (he->filtered)
@@ -437,13 +438,20 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,gtk_tree_store_set (store, &iter, col_idx++, hpp->buf,-1 ); } - fmt = he->fmt; -if (fmt->color) - fmt->color (fmt, hpp, he); -else - fmt->entry (fmt, hpp, he); + bf = hpp->buf; +perf_hpp_list__for_each_format (he->hpp_list, fmt) { +int ret; + +if (fmt->color) + ret = fmt->color (fmt, hpp, he); +else + ret = fmt->entry (fmt, hpp, he); + +snprintf (hpp->buf + ret, hpp->size - ret," " ); +advance_hpp (hpp, ret +2 ); + } -gtk_tree_store_set (store, &iter, col_idx,rtrim (hpp->buf),-1 ); +gtk_tree_store_set (store, &iter, col_idx,rtrim (bf),-1 );if (!he->leaf) { perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
tools/perf/ui/stdio/hist.c +22 -22
55 lines not on this page · raw message
tools/perf/util/hist.c +45 -24
121 lines not on this page · raw message
tools/perf/util/sort.h +1 -0
@@ -130,6 +130,7 @@ struct hist_entry {u32 raw_size;void *trace_output;struct perf_hpp_fmt *fmt; +struct perf_hpp_list *hpp_list;struct hist_entry *parent_he;union {/* this is for hierarchical entry structure */
Signature · 1 line
2.7.2
Re: [PATCH v2 3/8] perf tools: Use own hpp_list for hierarchy mode
On Fri, Mar 04, 2016 at 11:59:37PM +0900, Namhyung Kim wrote:
SNIP
Quoted 16 lines
tools/perf/util/hist.c +2
-5@@ -1150,20 +1159,29 @@ static int hists__hierarchy_insert_entry(struct hists *hists, struct hist_entry *he) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node;struct hist_entry *new_he =NULL ;struct hist_entry *parent =NULL ;int depth =0 ;int ret =0 ; -hists__for_each_sort_list (hists, fmt) { -if (!perf_hpp__is_sort_entry (fmt) && - !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) +list_for_each_entry (node, &hists->hpp_formats, list) {
tools/perf/util/hist.c static int hists__hierarchy_insert_entry(struct hists *hists, · line 1178
Quoted 10 lines
tools/perf/util/hist.c +10
-0+bool skip = false; + +perf_hpp_list__for_each_sort_list (&node->hpp, fmt) { +if (!perf_hpp__is_sort_entry (fmt) && + !perf_hpp__is_dynamic_entry (fmt)) + skip = true; +if (perf_hpp__should_skip (fmt, hists)) + skip = true; +if (skip) +break ;
could we add skip bool into node and initialize it gradually in add_hierarchy_fmt?
jirka
Namhyung Kim
maintainerRe: [PATCH v2 3/8] perf tools: Use own hpp_list for hierarchy mode
Hi Jiri,
Quoted 24 lines · Jiri Olsa
On Fri, Mar 04, 2016 at 11:59:37PM +0900, Namhyung Kim wrote:
SNIP
Quoted 20 lines
tools/perf/util/hist.c +6
-5@@ -1150,20 +1159,29 @@ static int hists__hierarchy_insert_entry(struct hists *hists, struct hist_entry *he) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *node;struct hist_entry *new_he =NULL ;struct hist_entry *parent =NULL ;int depth =0 ;int ret =0 ; -hists__for_each_sort_list (hists, fmt) { -if (!perf_hpp__is_sort_entry (fmt) && - !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) +list_for_each_entry (node, &hists->hpp_formats, list) { +bool skip = false; + +perf_hpp_list__for_each_sort_list (&node->hpp, fmt) { + if (!perf_hpp__is_sort_entry(fmt) &&
Quoted 8 lines
tools/perf/util/hist.c static int hists__hierarchy_insert_entry(struct hists *hists, · line 1178
Quoted 6 lines
tools/perf/util/hist.c +6
-0+ !perf_hpp__is_dynamic_entry(fmt)) + skip = true; +if (perf_hpp__should_skip (fmt, hists)) + skip = true; +if (skip) +break ;could we add skip bool into node and initialize it gradually in
add_hierarchy_fmt?
Right.
We can simply skip the node if all fmts it contains should be skipped.
Thanks,
Namhyung
Namhyung Kim
maintainerPATCH v2 4/8 perf tools: Support multiple sort keys in a hierarchy level
In reply to Namhyung KimThis implements having multiple sort keys in a single hierarchy level. Originally only single sort key is supported for each level, but now using the group syntax with '{ }', it can set more than one sort key in one level. Note that now it needs to quote in order to prevent shell interpretation.
For example:
$ perf report --hierarchy -s '{comm,dso},sym'
...
# Overhead Command / Shared Object / Symbol
# .............. ..........................................
#
48.67% swapper [kernel.vmlinux]
34.42% [k] intel_idle
1.30% [k] __tick_nohz_idle_enter
1.03% [k] cpuidle_reflect
8.87% firefox libpthread-2.22.so
6.60% [.] __GI___libc_recvmsg
1.18% [.] pthread_cond_signal@@GLIBC_2.3.2
1.09% [.] 0x000000000000ff4b
6.11% Xorg libc-2.22.so
5.27% [.] __memcpy_sse2_unalignedIn the above example, the command name and the shared object name are shown on the same line but the symbol name is on the different line. Since the first two are grouped by '{}', they are in the same level.
--- tools/perf/util/sort.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-)
tools/perf/util/sort.c +32 -10
@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)char *tmp, *tok;int ret =0 ;int level =0 ; +int next_level =1 ; +bool in_group = false; + +do { + tok = str; + tmp =strpbrk (str,"{}, " ); +if (tmp) { +if (in_group) + next_level = level; +else + next_level = level +1 ; + +if (*tmp =='{' ) + in_group = true; +else if (*tmp =='}' ) + in_group = false; + + *tmp ='\0' ; + str = tmp +1 ; + } -for (tok =strtok_r (str,", " , &tmp); - tok; tok =strtok_r (NULL ,", " , &tmp)) { - ret =sort_dimension__add (tok, evlist, level++); -if (ret == -EINVAL ) { -error ("Invalid --sort key: `%s'" , tok); -break ; - }else if (ret == -ESRCH ) { -error ("Unknown --sort key: `%s'" , tok); -break ; +if (*tok) { + ret =sort_dimension__add (tok, evlist, level); +if (ret == -EINVAL ) { +error ("Invalid --sort key: `%s'" , tok); +break ; + }else if (ret == -ESRCH ) { +error ("Unknown --sort key: `%s'" , tok); +break ; + } } - } + + level = next_level; + }while (tmp);return ret; }
Signature · 1 line
2.7.2
Arnaldo Carvalho de Melo
maintainerRe: [PATCH v2 4/8] perf tools: Support multiple sort keys in a hierarchy level
Em Fri, Mar 04, 2016 at 11:59:38PM +0900, Namhyung Kim escreveu:
tools/perf/util/sort.c static int setup_sort_list(char *str, struct perf_evlist *evlist) · line 2318
Quoted 13 lines
This implements having multiple sort keys in a single hierarchy level.
+++
b/tools/perf/util/sort.ctools/perf/util/sort.c +6
-0@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)char *tmp, *tok;int ret =0 ;int level =0 ; +int next_level =1 ; +bool in_group = false; + + do { + tok = str; + tmp = strpbrk(str, "{}, ");
Ok, I'll test this, but since we use lex/yacc for parsing event descriptions, perhaps we should use it here as well? Or do you think we're stopping at this level of complexity (I doubt) :-)
Quoted 43 lines
tools/perf/util/sort.c +26
-10+if (tmp) { +if (in_group) + next_level = level; +else + next_level = level +1 ; + +if (*tmp =='{' ) + in_group = true; +else if (*tmp =='}' ) + in_group = false; + + *tmp ='\0' ; + str = tmp +1 ; + } -for (tok =strtok_r (str,", " , &tmp); - tok; tok =strtok_r (NULL ,", " , &tmp)) { - ret =sort_dimension__add (tok, evlist, level++); -if (ret == -EINVAL ) { -error ("Invalid --sort key: `%s'" , tok); -break ; - }else if (ret == -ESRCH ) { -error ("Unknown --sort key: `%s'" , tok); -break ; +if (*tok) { + ret =sort_dimension__add (tok, evlist, level); +if (ret == -EINVAL ) { +error ("Invalid --sort key: `%s'" , tok); +break ; + }else if (ret == -ESRCH ) { +error ("Unknown --sort key: `%s'" , tok); +break ; + } } - } + + level = next_level; + }while (tmp);return ret; }Signature · 1 line
2.7.2
Namhyung Kim
maintainerRe: [PATCH v2 4/8] perf tools: Support multiple sort keys in a hierarchy level
Hi Arnaldo,
Quoted 18 lines · Arnaldo Carvalho de Melo
Em Fri, Mar 04, 2016 at 11:59:38PM +0900, Namhyung Kim escreveu:
tools/perf/util/sort.c static int setup_sort_list(char *str, struct perf_evlist *evlist) · line 2318
Quoted 13 lines
This implements having multiple sort keys in a single hierarchy level.
+++
b/tools/perf/util/sort.ctools/perf/util/sort.c +6
-0@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)char *tmp, *tok;int ret =0 ;int level =0 ; +int next_level =1 ; +bool in_group = false; + + do { + tok = str; + tmp = strpbrk(str, "{}, ");Ok, I'll test this, but since we use lex/yacc for parsing event descriptions, perhaps we should use it here as well? Or do you think we're stopping at this level of complexity (I doubt) :-)
I'm not sure how the syntax of sort keys will become more complex. But I don't think we should use lex/yacc at this level of complexity
Thanks,
Namhyung
Quoted 44 lines
Quoted 44 lines
tools/perf/util/sort.c +26
-10+if (tmp) { +if (in_group) + next_level = level; +else + next_level = level +1 ; + +if (*tmp =='{' ) + in_group = true; +else if (*tmp =='}' ) + in_group = false; + + *tmp ='\0' ; + str = tmp +1 ; + } -for (tok =strtok_r (str,", " , &tmp); - tok; tok =strtok_r (NULL ,", " , &tmp)) { - ret =sort_dimension__add (tok, evlist, level++); -if (ret == -EINVAL ) { -error ("Invalid --sort key: `%s'" , tok); -break ; - }else if (ret == -ESRCH ) { -error ("Unknown --sort key: `%s'" , tok); -break ; +if (*tok) { + ret =sort_dimension__add (tok, evlist, level); +if (ret == -EINVAL ) { +error ("Invalid --sort key: `%s'" , tok); +break ; + }else if (ret == -ESRCH ) { +error ("Unknown --sort key: `%s'" , tok); +break ; + } } - } + + level = next_level; + }while (tmp);return ret; }Signature · 1 line
2.7.2
Namhyung Kim
maintainerPATCH v2 5/8 perf tools: Fix indent for multiple hierarchy sort key
In reply to Namhyung KimWhen multiple sort keys are used in a single hierarchy, it should indent using number of hierarchy levels instead of number of sort keys.
--- tools/perf/ui/browsers/hists.c | 23 ++++++++++------------- tools/perf/ui/hist.c | 1 + tools/perf/ui/stdio/hist.c | 26 +++++++++++--------------- tools/perf/util/hist.h | 1 + 4 files changed, 23 insertions(+), 28 deletions(-)
tools/perf/ui/browsers/hists.c +10 -13
@@ -1280,7 +1280,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,static int hist_browser__show_hierarchy_entry (struct hist_browser *browser,struct hist_entry *entry,unsigned short row, -int level,int nr_sort_keys) +int level) {int printed =0 ;int width = browser->b.width;
@@ -1294,7 +1294,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser, .current_entry = current_entry, };int column =0 ; -int hierarchy_indent = (nr_sort_keys -1 ) *HIERARCHY_INDENT ; +int hierarchy_indent = (entry->hists->nr_hpp_node -2 ) *HIERARCHY_INDENT ;if (current_entry) { browser->he_selection = entry;
@@ -1436,8 +1436,7 @@ show_callchain: }static int hist_browser__show_no_entry (struct hist_browser *browser, -unsigned short row, -int level,int nr_sort_keys) +unsigned short row,int level) {int width = browser->b.width;bool current_entry =ui_browser__is_current_entry (&browser->b, row);
@@ -1445,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,int column =0 ;int ret;struct perf_hpp_fmt *fmt; +int indent = browser->hists->nr_hpp_node -2 ;if (current_entry) { browser->he_selection =NULL ;
@@ -1485,8 +1485,8 @@ static int hist_browser__show_no_entry(struct hist_browser *browser, width -= ret; } -ui_browser__write_nstring (&browser->b,"" , nr_sort_keys *HIERARCHY_INDENT ); - width -= nr_sort_keys *HIERARCHY_INDENT ; +ui_browser__write_nstring (&browser->b,"" , indent *HIERARCHY_INDENT ); + width -= indent *HIERARCHY_INDENT ;if (column >= browser->b.horiz_scroll) {char buf[32 ];
@@ -1553,7 +1553,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *browsstruct perf_hpp_fmt *fmt;size_t ret =0 ;int column =0 ; -int nr_sort_keys = hists->nr_sort_keys; +int indent = hists->nr_hpp_node -2 ;bool first = true; ret =scnprintf (buf, size," " );
@@ -1577,7 +1577,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows } ret =scnprintf (dummy_hpp.buf, dummy_hpp.size,"%*s" , - (nr_sort_keys -1 ) *HIERARCHY_INDENT ,"" ); + indent *HIERARCHY_INDENT ,"" );if (advance_hpp_check (&dummy_hpp, ret))return ret;
@@ -1645,7 +1645,6 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)u16 header_offset =0 ;struct rb_node *nd; struct hist_browser *hb = container_of(browser, struct hist_browser, b); -int nr_sort = hb->hists->nr_sort_keys;if (hb->show_headers) {hist_browser__show_headers (hb);
@@ -1672,14 +1671,12 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)if (symbol_conf.report_hierarchy) { row +=hist_browser__show_hierarchy_entry (hb, h, row, - h->depth, - nr_sort); + h->depth);if (row == browser->rows)break ;if (h->has_no_entry) { -hist_browser__show_no_entry (hb, row, h->depth, - nr_sort); +hist_browser__show_no_entry (hb, row, h->depth); row++; } }else {
tools/perf/ui/hist.c +1 -0
@@ -740,6 +740,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)INIT_LIST_HEAD (&node->hpp.fields);INIT_LIST_HEAD (&node->hpp.sorts); + hists->nr_hpp_node++;list_add_tail (&node->list, &hists->hpp_formats); }
tools/perf/ui/stdio/hist.c +11 -15
@@ -412,7 +412,7 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)static int hist_entry__hierarchy_fprintf (struct hist_entry *he,struct perf_hpp *hpp, -int nr_sort_key,struct hists *hists, +struct hists *hists,FILE *fp) {const char *sep = symbol_conf.field_sep;
@@ -453,7 +453,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,if (!sep) ret =scnprintf (hpp->buf, hpp->size,"%*s" , - (nr_sort_key -1 ) *HIERARCHY_INDENT ,"" ); + (hists->nr_hpp_node -2 ) *HIERARCHY_INDENT ,"" );advance_hpp (hpp, ret); printed +=fprintf (fp,"%s" , buf);
@@ -504,12 +504,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,if (size ==0 || size > bfsz) size = hpp.size = bfsz; -if (symbol_conf.report_hierarchy) { -int nr_sort = hists->nr_sort_keys; - -return hist_entry__hierarchy_fprintf (he, &hpp, nr_sort, - hists, fp); - } +if (symbol_conf.report_hierarchy) +return hist_entry__hierarchy_fprintf (he, &hpp, hists, fp);hist_entry__snprintf (he, &hpp);
@@ -521,29 +517,29 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,return ret; } -static int print_hierarchy_indent (const char *sep,int nr_sort, +static int print_hierarchy_indent (const char *sep,int indent,const char *line,FILE *fp) { -if (sep !=NULL || nr_sort <1 ) +if (sep !=NULL || indent <2 )return 0 ; -return fprintf (fp,"%-.*s" , (nr_sort -1 ) *HIERARCHY_INDENT , line); +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) {bool first = true; -int nr_sort; +int indent;int depth;unsigned width =0 ;unsigned header_width =0 ;struct perf_hpp_fmt *fmt; - nr_sort = hists->nr_sort_keys; + indent = hists->nr_hpp_node;/* preserve max indent depth for column headers */ -print_hierarchy_indent (sep, nr_sort, spaces, fp); +print_hierarchy_indent (sep, indent, spaces, fp);hists__for_each_format (hists, fmt) {if (perf_hpp__is_sort_entry (fmt) ||perf_hpp__is_dynamic_entry (fmt))
@@ -582,7 +578,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,fprintf (fp,"\n# " );/* preserve max indent depth for initial dots */ -print_hierarchy_indent (sep, nr_sort, dots, fp); +print_hierarchy_indent (sep, indent, dots, fp); first = true;hists__for_each_format (hists, fmt) {
tools/perf/util/hist.h +1 -0
@@ -80,6 +80,7 @@ struct hists {struct perf_hpp_list *hpp_list;struct list_head hpp_formats;int nr_sort_keys; +int nr_hpp_node; };struct hist_entry_iter ;
Signature · 1 line
2.7.2
Namhyung Kim
maintainerPATCH v2 6/8 perf report: Use hierarchy hpp list on stdio
In reply to Namhyung KimNow hpp formats are linked using perf_hpp_list_node when hierarchy is enabled. Use this info to print entries with multiple sort keys in a single hierarchy properly.
For example, the below example shows using 4 sort keys with 2 levels.
$ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
--percent-limit 1 -i perf.data.sched
...
# Overhead prev_pid+prev_comm / next_pid+next_comm
# ........... .......................................
#
22.36% 0 swapper/0
9.48% 17773 transmission-gt
5.25% 109 kworker/0:1H
1.53% 6524 Xephyr
21.39% 17773 transmission-gt
9.52% 0 swapper/0
9.04% 0 swapper/2
1.78% 0 swapper/3--- tools/perf/ui/stdio/hist.c | 103 +++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 46 deletions(-)
tools/perf/ui/stdio/hist.c +57 -46
@@ -417,6 +417,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he, {const char *sep = symbol_conf.field_sep;struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;char *buf = hpp->buf;size_t size = hpp->size;int ret, printed =0 ;
@@ -428,10 +429,10 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he, ret =scnprintf (hpp->buf, hpp->size,"%*s" , he->depth *HIERARCHY_INDENT ,"" );advance_hpp (hpp, ret); -hists__for_each_format (he->hists, fmt) { -if (perf_hpp__is_sort_entry (fmt) ||perf_hpp__is_dynamic_entry (fmt)) -break ; - +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) {/* * If there's no field_sep, we still need * to display initial ' '.
@@ -529,50 +530,49 @@ static int print_hierarchy_indent(const char *sep, int indent,static int print_hierarchy_header (struct hists *hists,struct perf_hpp *hpp,const char *sep,FILE *fp) { -bool first = true; +bool first_node, first_col;int indent;int depth;unsigned width =0 ;unsigned header_width =0 ;struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node; indent = hists->nr_hpp_node;/* preserve max indent depth for column headers */ print_hierarchy_indent (sep, indent, spaces, fp); -hists__for_each_format (hists, fmt) { -if (perf_hpp__is_sort_entry (fmt) ||perf_hpp__is_dynamic_entry (fmt)) -break ; - -if (!first) -fprintf (fp,"%s" , sep ?:" " ); -else - first = false; +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { fmt->header (fmt, hpp,hists_to_evsel (hists)); -fprintf (fp,"%s" , hpp->buf); +fprintf (fp,"%s%s" , hpp->buf, sep ?:" " ); }/* combine sort headers with ' / ' */ - first = true; -hists__for_each_format (hists, fmt) { -if (!perf_hpp__is_sort_entry (fmt) && !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) -continue ; - -if (!first) + first_node = true; +list_for_each_entry_continue (fmt_node, &hists->hpp_formats, list) { +if (!first_node) header_width +=fprintf (fp," / " ); -else { -fprintf (fp,"%s" , sep ?:" " ); - first = false; - } + first_node = false; - fmt->header (fmt, hpp,hists_to_evsel (hists)); -rtrim (hpp->buf); + first_col = true; +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { +if (perf_hpp__should_skip (fmt, hists)) +continue ; + +if (!first_col) + header_width +=fprintf (fp,"+" ); + first_col = false; + + fmt->header (fmt, hpp,hists_to_evsel (hists)); +rtrim (hpp->buf); - header_width +=fprintf (fp,"%s" ,ltrim (hpp->buf)); + header_width +=fprintf (fp,"%s" ,ltrim (hpp->buf)); + } }fprintf (fp,"\n# " );
@@ -580,29 +580,35 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,/* preserve max indent depth for initial dots */ print_hierarchy_indent (sep, indent, dots, fp); - first = true; -hists__for_each_format (hists, fmt) { -if (perf_hpp__is_sort_entry (fmt) ||perf_hpp__is_dynamic_entry (fmt)) -break ; +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); -if (!first) -fprintf (fp,"%s" , sep ?:" " ); -else - first = false; + first_col = true; +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { +if (!first_col) +fprintf (fp,"%s" , sep ?:".." ); + first_col = false; width = fmt->width (fmt, hpp,hists_to_evsel (hists));fprintf (fp,"%.*s" , width, dots); } depth =0 ; -hists__for_each_format (hists, fmt) { -if (!perf_hpp__is_sort_entry (fmt) && !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) -continue ; +list_for_each_entry_continue (fmt_node, &hists->hpp_formats, list) { + first_col = true; + width = depth *HIERARCHY_INDENT ; - width = fmt->width (fmt, hpp,hists_to_evsel (hists)); - width += depth *HIERARCHY_INDENT ; +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { +if (perf_hpp__should_skip (fmt, hists)) +continue ; + +if (!first_col) + width++;/* for '+' sign between column header */ + first_col = false; + + width += fmt->width (fmt, hpp,hists_to_evsel (hists)); + }if (width > header_width) header_width = width;
@@ -621,6 +627,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows, int max_cols, float min_pcnt, FILE *fp) { struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;struct rb_node *nd;size_t ret =0 ;unsigned int width;
@@ -650,6 +657,10 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,fprintf (fp,"# " );if (symbol_conf.report_hierarchy) { +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); + } nr_rows +=print_hierarchy_header (hists, &dummy_hpp, sep, fp); goto print_entries; }
@@ -734,9 +745,9 @@ print_entries:* display "no entry >= x.xx%" message. */ if (!h->leaf && !hist_entry__has_hierarchy_children (h, min_pcnt)) { -int nr_sort = hists->nr_sort_keys; +int depth = hists->nr_hpp_node + h->depth +1 ; -print_hierarchy_indent (sep, nr_sort + h->depth +1 , spaces, fp); +print_hierarchy_indent (sep, depth, spaces, fp);fprintf (fp,"%*sno entry >= %.2f%%\n" , indent,"" , min_pcnt);if (max_rows && ++nr_rows >= max_rows)
Signature · 1 line
2.7.2
Namhyung Kim
maintainerPATCH v2 7/8 perf hists browser: Use hierarchy hpp list
In reply to Namhyung KimNow hpp formats are linked using perf_hpp_list_node when hierarchy is enabled. Like in stdio, use this info to print entries with multiple sort keys in a single hierarchy properly.
--- tools/perf/ui/browsers/hists.c | 81 +++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 36 deletions(-)
tools/perf/ui/browsers/hists.c +45 -36
@@ -1289,6 +1289,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,off_t row_offset = entry->row_offset;bool first = true;struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;struct hpp_arg arg = { .b = &browser->b, .current_entry = current_entry,
@@ -1320,7 +1321,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,ui_browser__write_nstring (&browser->b,"" , level *HIERARCHY_INDENT ); width -= level *HIERARCHY_INDENT ; -hists__for_each_format (entry->hists, fmt) { +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&entry->hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) {char s[2048 ];struct perf_hpp hpp = { .buf = s,
@@ -1332,10 +1336,6 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser, column++ < browser->b.horiz_scroll)continue ; -if (perf_hpp__is_sort_entry (fmt) || -perf_hpp__is_dynamic_entry (fmt)) -break ; -if (current_entry && browser->b.navkeypressed) {ui_browser__set_color (&browser->b,HE_COLORSET_SELECTED );
@@ -1444,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,int column =0 ;int ret;struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;int indent = browser->hists->nr_hpp_node -2 ;if (current_entry) {
@@ -1461,15 +1462,14 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,ui_browser__write_nstring (&browser->b,"" , level *HIERARCHY_INDENT ); width -= level *HIERARCHY_INDENT ; -hists__for_each_format (browser->hists, fmt) { +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&browser->hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) {if (perf_hpp__should_skip (fmt, browser->hists) || column++ < browser->b.horiz_scroll)continue ; -if (perf_hpp__is_sort_entry (fmt) || -perf_hpp__is_dynamic_entry (fmt)) -break ; - ret = fmt->width (fmt,NULL ,hists_to_evsel (browser->hists));if (first) {
@@ -1551,22 +1551,23 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows .size = size, };struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;size_t ret =0 ;int column =0 ;int indent = hists->nr_hpp_node -2 ; -bool first = true; +bool first_node, first_col; ret =scnprintf (buf, size," " );if (advance_hpp_check (&dummy_hpp, ret))return ret; -hists__for_each_format (hists, fmt) { +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) {if (column++ < browser->b.horiz_scroll)continue ; -if (perf_hpp__is_sort_entry (fmt) ||perf_hpp__is_dynamic_entry (fmt)) -break ; - ret = fmt->header (fmt, &dummy_hpp,hists_to_evsel (hists));if (advance_hpp_check (&dummy_hpp, ret))break ;
@@ -1581,34 +1582,42 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *browsif (advance_hpp_check (&dummy_hpp, ret))return ret; -hists__for_each_format (hists, fmt) { -char *start; - -if (!perf_hpp__is_sort_entry (fmt) && !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) -continue ; - -if (first) { - first = false; - }else { + first_node = true; +list_for_each_entry_continue (fmt_node, &hists->hpp_formats, list) { +if (!first_node) { ret =scnprintf (dummy_hpp.buf, dummy_hpp.size," / " );if (advance_hpp_check (&dummy_hpp, ret))break ; } + first_node = false; - ret = fmt->header (fmt, &dummy_hpp,hists_to_evsel (hists)); - dummy_hpp.buf[ret] ='\0' ; -rtrim (dummy_hpp.buf); + first_col = true; +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { +char *start; - start =ltrim (dummy_hpp.buf); - ret =strlen (start); +if (perf_hpp__should_skip (fmt, hists)) +continue ; -if (start != dummy_hpp.buf) -memmove (dummy_hpp.buf, start, ret +1 ); +if (!first_col) { + ret =scnprintf (dummy_hpp.buf, dummy_hpp.size,"+" ); +if (advance_hpp_check (&dummy_hpp, ret)) +break ; + } + first_col = false; -if (advance_hpp_check (&dummy_hpp, ret)) -break ; + ret = fmt->header (fmt, &dummy_hpp,hists_to_evsel (hists)); + dummy_hpp.buf[ret] ='\0' ; +rtrim (dummy_hpp.buf); + + start =ltrim (dummy_hpp.buf); + ret =strlen (start); + +if (start != dummy_hpp.buf) +memmove (dummy_hpp.buf, start, ret +1 ); + +if (advance_hpp_check (&dummy_hpp, ret)) +break ; + } }return ret;
@@ -1676,7 +1685,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)break ;if (h->has_no_entry) { -hist_browser__show_no_entry (hb, row, h->depth); +hist_browser__show_no_entry (hb, row, h->depth +1 ); row++; } }else {
Signature · 1 line
2.7.2
Namhyung Kim
maintainerPATCH v2 8/8 perf report: Use hierarchy hpp list on gtk
In reply to Namhyung KimNow hpp formats are linked using perf_hpp_list_node when hierarchy is enabled. Like in stdio, use this info to print entries with multiple sort keys in a single hierarchy properly.
--- tools/perf/ui/gtk/hists.c | 55 ++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 22 deletions(-)
tools/perf/ui/gtk/hists.c +33 -22
@@ -407,7 +407,9 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,struct rb_node *node;struct hist_entry *he;struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;u64 total =hists__total_period (hists); +int size;for (node =rb_first (root); node; node =rb_next (node)) {GtkTreeIter iter;
@@ -425,11 +427,11 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,gtk_tree_store_append (store, &iter, parent); col_idx =0 ; -hists__for_each_format (hists, fmt) { -if (perf_hpp__is_sort_entry (fmt) || -perf_hpp__is_dynamic_entry (fmt)) -break ; +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) {if (fmt->color) fmt->color (fmt, hpp, he);else
@@ -439,6 +441,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists, } bf = hpp->buf; + size = hpp->size;perf_hpp_list__for_each_format (he->hpp_list, fmt) {int ret;
@@ -451,9 +454,12 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,advance_hpp (hpp, ret +2 ); } -gtk_tree_store_set (store, &iter, col_idx,rtrim (bf),-1 ); +gtk_tree_store_set (store, &iter, col_idx,ltrim (rtrim (bf)),-1 );if (!he->leaf) { + hpp->buf = bf; + hpp->size = size; +perf_gtk__add_hierarchy_entries (hists, &he->hroot_out, store, &iter, hpp, min_pcnt);
@@ -486,6 +492,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists, float min_pcnt) {struct perf_hpp_fmt *fmt; +struct perf_hpp_list_node *fmt_node;GType col_types[MAX_COLUMNS ];GtkCellRenderer *renderer;GtkTreeStore *store;
@@ -494,7 +501,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,int nr_cols =0 ;char s[512 ];char buf[512 ]; -bool first = true; +bool first_node, first_col;struct perf_hpp hpp = { .buf = s, .size =sizeof (s),
@@ -514,11 +521,11 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists, renderer =gtk_cell_renderer_text_new (); col_idx =0 ; -hists__for_each_format (hists, fmt) { -if (perf_hpp__is_sort_entry (fmt) || -perf_hpp__is_dynamic_entry (fmt)) -break ; +/* the first hpp_list_node is for overhead columns */ + fmt_node =list_first_entry (&hists->hpp_formats, + struct perf_hpp_list_node, list); +perf_hpp_list__for_each_format (&fmt_node->hpp, fmt) { gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view), -1, fmt->name, renderer, "markup",
@@ -527,20 +534,24 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,/* construct merged column header since sort keys share single column */ buf[0 ] ='\0' ; -hists__for_each_format (hists ,fmt) { -if (!perf_hpp__is_sort_entry (fmt) && - !perf_hpp__is_dynamic_entry (fmt)) -continue ; -if (perf_hpp__should_skip (fmt, hists)) -continue ; - -if (first) - first = false; -else + first_node = true; +list_for_each_entry_continue (fmt_node, &hists->hpp_formats, list) { +if (!first_node)strcat (buf," / " ); + first_node = false; - fmt->header (fmt, &hpp,hists_to_evsel (hists)); -strcat (buf,rtrim (hpp.buf)); + first_col = true; +perf_hpp_list__for_each_format (&fmt_node->hpp ,fmt) { +if (perf_hpp__should_skip (fmt, hists)) +continue ; + +if (!first_col) +strcat (buf,"+" ); + first_col = false; + + fmt->header (fmt, &hpp,hists_to_evsel (hists)); +strcat (buf,ltrim (rtrim (hpp.buf))); + } } gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
Signature · 1 line
2.7.2