perf tool: perf diff sort changes
v2 cover perf tool: perf diff sort changes
From: Kan Liang <[email protected]>
Current perf diff has some issues. E.g. the default sort key is symbol, but it doesn't work well. It sorts as address. Furthermore, the old perf diff can only work on perf.data from the same binary.
This patch set fixes the symbol issue and extends it to compare the perf.data from different binaries and different kernels. It's useful for debugging the regression issue.
Current perf diff can do address compare, the patch set also keep this feature. A new sort key "addr" is introduced. It can let the user sort differential profile by the address. It should be useful for debugging the scaling issue, if the user think function level diff is too high granularity.
Here is an example.
v1_1_6perf.data is perf record result of version one tchain on 1.6 kernel. v2_1_8perf.data is perf record result of version two tchain on 1.8 kernel. They are from different binaries.
v1_1_8_1perf.data and v1_1_8_2perf.data are perf record result of version one tchain on 1.8 kernel. I run perf record twice.
They are from same binary.
Old perf diff with default sort key "dso,symbol" for different binary ./perf diff -s dso,symbol v1_1_6perf.data v2_1_8perf.data
Event 'cycles'
Baseline Delta Shared Object Symbol ........ ....... ................. .........................
0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] notifier_call_chain
0.01% [kernel.kallsyms] [k] perf_event_task_tick
0.01% [kernel.kallsyms] [k] run_posix_cpu_timers
0.01% [kernel.kallsyms] [k] run_timer_softirq
0.01% [kernel.kallsyms] [k] trigger_load_balance
0.01% [kernel.kallsyms] [k] update_vsyscall
[kernel.vmlinux] [k] __run_hrtimer
[kernel.vmlinux] [k] apic_timer_interrupt
[kernel.vmlinux] [k] enqueue_task
[kernel.vmlinux] [k] hrtimer_interrupt
[kernel.vmlinux] [k] native_write_msr_safe
[kernel.vmlinux] [k] trigger_load_balance
[kernel.vmlinux] [k] update_wall_time
0.05% [unknown] [.] 0x0000000000400540
0.04% [unknown] [.] 0x0000000000400541
0.03% [unknown] [.] 0x000000000040054b
0.04% [unknown] [.] 0x0000000000400552
33.55% [unknown] [.] 0x0000000000400554
1.22% [unknown] [.] 0x000000000040055a
8.00% [unknown] [.] 0x000000000040055e
0.02% [unknown] [.] 0x0000000000400562
8.41% [unknown] [.] 0x0000000000400564
48.13% [unknown] [.] 0x000000000040056b
0.16% [unknown] [.] 0x0000000000400570
0.17% [unknown] [.] 0x0000000000400571
+0.45% [unknown] [.] 0x0000000000400580
+0.29% [unknown] [.] 0x0000000000400581
0.01% [unknown] [.] 0x0000000000400583
0.01% [unknown] [.] 0x0000000000400588
+0.22% [unknown] [.] 0x000000000040058b
0.01% +13.35% [unknown] [.] 0x000000000040058d
0.06% [unknown] [.] 0x0000000000400591
+0.78% [unknown] [.] 0x0000000000400593
0.04% [unknown] [.] 0x0000000000400595
0.01% +6.18% [unknown] [.] 0x0000000000400597
+1.47% [unknown] [.] 0x000000000040059b
+6.46% [unknown] [.] 0x000000000040059d
+1.28% [unknown] [.] 0x00000000004005a1
+66.38% [unknown] [.] 0x00000000004005a5
+1.34% [unknown] [.] 0x00000000004005a7
+1.15% [unknown] [.] 0x00000000004005a8
+0.05% [unknown] [.] 0x00000000004005ba
+0.03% [unknown] [.] 0x00000000004005bf
+0.02% [unknown] [.] 0x00000000004005c4
+0.05% [unknown] [.] 0x00000000004005c9
+0.03% [unknown] [.] 0x00000000004005ce
+0.27% [unknown] [.] 0x00000000004005d2
+0.15% [unknown] [.] 0x00000000004005d6
[unknown] [.] 0x00000000004005d8
[unknown] [.] 0x00000000004005d9New perf diff with default sort key "dso,symbol" for different binary ./perf diff -s dso,symbol v1_1_6perf.data v2_1_8perf.data
Event 'cycles'
Baseline Delta Shared Object Symbol ........ ....... ................. .........................
0.01% [kernel.kallsyms] [k] native_write_msr_safe
0.01% [kernel.kallsyms] [k] notifier_call_chain
0.01% [kernel.kallsyms] [k] perf_event_task_tick
0.01% [kernel.kallsyms] [k] run_posix_cpu_timers
0.01% [kernel.kallsyms] [k] run_timer_softirq
0.01% [kernel.kallsyms] [k] trigger_load_balance
0.01% [kernel.kallsyms] [k] update_vsyscall
[kernel.vmlinux] [k] __run_hrtimer
[kernel.vmlinux] [k] apic_timer_interrupt
[kernel.vmlinux] [k] enqueue_task
[kernel.vmlinux] [k] hrtimer_interrupt
[kernel.vmlinux] [k] native_write_msr_safe
[kernel.vmlinux] [k] trigger_load_balance
[kernel.vmlinux] [k] update_wall_time
0.14% +0.47% tchain [.] f2
99.82% -0.47% tchain [.] f3New perf diff with sort key "symbol,addr" ffor same binary perf diff -s symbol,addr v1_1_8_1perf.data v1_1_8_2perf.data
Event 'cycles'
Baseline Delta Symbol Addr ........ ....... ............................... ...............................
0.00% [k] __update_cpu_load [k] 0xffffffff810b9b8c
[k] _raw_spin_lock_irqsave [k] 0xffffffff81759364
[k] _raw_spin_unlock_irqrestore [k] 0xffffffff817594ba
0.00% [k] apic_timer_interrupt [k] 0xffffffff8175a9f0
[k] apic_timer_interrupt [k] 0xffffffff8175aa58
[k] check_preempt_curr [k] 0xffffffff810b461c
0.00% [.] f1 [.] 0x00000000004005fa
0.03% +0.03% [.] f2 [.] 0x00000000004005ba
0.03% +0.01% [.] f2 [.] 0x00000000004005bf
0.05% [.] f2 [.] 0x00000000004005c4
0.04% -0.02% [.] f2 [.] 0x00000000004005c9
0.03% [.] f2 [.] 0x00000000004005ce
0.17% +0.11% [.] f2 [.] 0x00000000004005d2
0.15% [.] f2 [.] 0x00000000004005d6
[.] f2 [.] 0x00000000004005d9
0.37% +0.02% [.] f3 [.] 0x0000000000400580
0.18% +0.03% [.] f3 [.] 0x0000000000400581
0.01% [.] f3 [.] 0x0000000000400584
0.18% +0.04% [.] f3 [.] 0x000000000040058b
12.31% +0.11% [.] f3 [.] 0x000000000040058d
0.03% [.] f3 [.] 0x0000000000400590
0.80% -0.12% [.] f3 [.] 0x0000000000400593
6.66% +0.09% [.] f3 [.] 0x0000000000400597
1.81% -0.36% [.] f3 [.] 0x000000000040059b
6.35% +0.24% [.] f3 [.] 0x000000000040059d
1.42% -0.22% [.] f3 [.] 0x00000000004005a1
66.83% +0.22% [.] f3 [.] 0x00000000004005a5
1.29% -0.12% [.] f3 [.] 0x00000000004005a7
1.22% -0.04% [.] f3 [.] 0x00000000004005a8
0.00% [k] load_balance [k] 0xffffffff810be7f6
0.00% [k] native_apic_mem_write [k] 0xffffffff81077920
0.00% [k] native_write_msr_safe [k] 0xffffffff8107d7ba
0.00% [k] native_write_msr_safe [k] 0xffffffff8107d7bd
0.00% [k] rb_erase [k] 0xffffffff813346b1
[k] resched_curr [k] 0xffffffff810b4095
[k] restore_args [k] 0xffffffff8175a744
0.00% [k] run_timer_softirq [k] 0xffffffff810e306f
[k] select_task_rq_fair [k] 0xffffffff810bbab9
0.00% [k] select_task_rq_fair [k] 0xffffffff810bbc89
[k] task_tick_fair [k] 0xffffffff810bd286
0.00% [k] task_tick_fair [k] 0xffffffff810bd40e
[k] task_waking_fair [k] 0xffffffff810baf97
0.00% [k] try_to_wake_up [k] 0xffffffff810b75e8
[k] update_cfs_rq_blocked_load [k] 0xffffffff810bb129
0.00% [k] update_cpu_load_active [k] 0xffffffff810ba08b
[k] update_group_capacity [k] 0xffffffff810bdde0
[k] update_wall_time [k] 0xffffffff810eac46Changes from V1:
- mmap2 as default of tool's definition
- Using se_collapse to match symbol name.
- Sort key "addr" is introduced to compare address from same binary.
Kan Liang (3):
perf tool: Fix perf diff symble sort issue perf tool:perf diff support for different binaries perf tool: Add sort key addr for perf diff
tools/perf/Documentation/perf-diff.txt | 7 +++++-- tools/perf/builtin-diff.c | 3 ++- tools/perf/util/hist.c | 5 +++-- tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 38 ++++++++++++++++++++++++++++++++++ tools/perf/util/sort.h | 2 ++ 6 files changed, 51 insertions(+), 5 deletions(-)
Signature · 1 line
1.8.3.2
PATCH v2 1/3 perf tool: Fix perf diff symble sort issue
From: Kan Liang <[email protected]>
This patch fixes a bug for perf diff.
Without mmap2, perf diff fails to find the symbol name. The default symbol sort key doesn't work well.
Signed-off-by Kan Liang <[email protected]>
--- tools/perf/builtin-diff.c | 1 + 1 file changed, 1 insertion(+)
tools/perf/builtin-diff.c +1 -0
@@ -357,6 +357,7 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,static struct perf_tool tool = { .sample = diff__process_sample_event, .mmap = perf_event__process_mmap, + .mmap2 = perf_event__process_mmap2, .comm = perf_event__process_comm, .exit = perf_event__process_exit, .fork = perf_event__process_fork,
Signature · 1 line
1.8.3.2
Re: [PATCH V2 1/3] perf tool: Fix perf diff symble sort issue
Hi kan,
Quoted 7 lines · kan liang
From: Kan Liang <[email protected]>
This patch fixes a bug for perf diff.
Without mmap2, perf diff fails to find the symbol name. The default symbol sort key doesn't work well.Signed-off-by Kan Liang <[email protected]>
Acked-by Namhyung Kim <[email protected]>
Thanks,
Namhyung
Quoted 16 lines
--- tools/perf/builtin-diff.c | 1 + 1 file changed, 1 insertion(+)tools/perf/builtin-diff.c +1
-0@@ -357,6 +357,7 @@ static int diff__process_sample_event(struct perf_tool *tool __maybe_unused,static struct perf_tool tool = { .sample = diff__process_sample_event, .mmap = perf_event__process_mmap, + .mmap2 = perf_event__process_mmap2, .comm = perf_event__process_comm, .exit = perf_event__process_exit, .fork = perf_event__process_fork,
PATCH v2 2/3 perf tool:perf diff support for different binaries
In reply to [email protected]From: Kan Liang <[email protected]>
Currently, the perf diff only works with same binaries. That's because it compares the symbol start address. It doesn't work if the perf.data comes from different binaries. This patch matches the function names.
Signed-off-by Kan Liang <[email protected]>
--- tools/perf/util/sort.c | 9 +++++++++ 1 file changed, 9 insertions(+)
tools/perf/util/sort.c +9 -0
@@ -1430,6 +1430,15 @@ int sort_dimension__add(const char *tok) sort__has_parent =1 ; }else if (sd->entry == &sort_sym) { sort__has_sym =1 ; +/* +* perf diff displays the performance difference amongst +* two or more perf.data files. Those files could come from +* different binaries. So we should not compare their ips. +* The name of symble should be the key we do compare. +*/ +if (sort__mode ==SORT_MODE__DIFF ) + sd->entry->se_collapse = sort__sym_sort; + }else if (sd->entry == &sort_dso) { sort__has_dso =1 ; }
Signature · 1 line
1.8.3.2
Re: [PATCH V2 2/3] perf tool:perf diff support for different binaries
Quoted 7 lines · kan liang
From: Kan Liang <[email protected]>
Currently, the perf diff only works with same binaries. That's because it compares the symbol start address. It doesn't work if the perf.data comes from different binaries. This patch matches the function names.
Signed-off-by Kan Liang <[email protected]>
Acked-by Namhyung Kim <[email protected]>
Thanks,
Namhyung
Quoted 24 lines
--- tools/perf/util/sort.c | 9 +++++++++ 1 file changed, 9 insertions(+)tools/perf/util/sort.c +9
-0@@ -1430,6 +1430,15 @@ int sort_dimension__add(const char *tok) sort__has_parent =1 ; }else if (sd->entry == &sort_sym) { sort__has_sym =1 ; +/* +* perf diff displays the performance difference amongst +* two or more perf.data files. Those files could come from +* different binaries. So we should not compare their ips. +* The name of symble should be the key we do compare. +*/ +if (sort__mode ==SORT_MODE__DIFF ) + sd->entry->se_collapse = sort__sym_sort; + }else if (sd->entry == &sort_dso) { sort__has_dso =1 ; }
PATCH v2 3/3 perf tool: Add sort key addr for perf diff
In reply to [email protected]From: Kan Liang <[email protected]>
Sometime, especially debugging scaling issue, the function level diff may be high granularity. The user may want to do deeper diff analysis for some cache or lock issue. The "addr" key can let the user sort differential profile by ips. This feature should only work when the perf.data comes from same binary.
Signed-off-by Kan Liang <[email protected]>
--- tools/perf/Documentation/perf-diff.txt | 7 +++++-- tools/perf/builtin-diff.c | 2 +- tools/perf/util/hist.c | 5 +++-- tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 29 +++++++++++++++++++++++++++++ tools/perf/util/sort.h | 2 ++ 6 files changed, 41 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-diff.txt +5 -2
@@ -50,8 +50,11 @@ OPTIONS -s:: --sort=:: - Sort by key(s): pid, comm, dso, symbol, cpu, parent, srcline. - Please see description of --sort in the perf-report man page. + Sort by key(s): pid, comm, dso, symbol, cpu, parent, srcline, addr. + + - addr: Address executed at the time of sample (for same binary compare) + + For other keys, please see description of --sort in the perf-report man page. -t:: --field-separator=::
tools/perf/builtin-diff.c +1 -1
@@ -744,7 +744,7 @@ static const struct option options[] = {OPT_STRING ('S' ,"symbols" , &symbol_conf.sym_list_str,"symbol[,symbol...]" ,"only consider these symbols" ),OPT_STRING ('s' ,"sort" , &sort_order,"key[,key2...]" , -"sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..." +"sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, addr, ..." " Please refer the man page for the complete list." ),OPT_STRING ('t' ,"field-separator" , &symbol_conf.field_sep,"separator" , "separator for columns, no spaces will be added between "
tools/perf/util/hist.c +3 -2
@@ -67,13 +67,14 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h) symlen = h->ms.sym->namelen +4 ;if (verbose) symlen +=BITS_PER_LONG /4 +2 +3 ; -hists__new_col_len (hists,HISTC_SYMBOL , symlen); }else { symlen = unresolved_col_width +4 +2 ; -hists__new_col_len (hists,HISTC_SYMBOL , symlen);hists__set_unres_dso_col_len (hists,HISTC_DSO ); } +hists__new_col_len (hists,HISTC_SYMBOL , symlen); +hists__new_col_len (hists,HISTC_ADDR , symlen); + len =thread__comm_len (h->thread);if (hists__new_col_len (hists,HISTC_COMM , len))hists__set_col_len (hists,HISTC_THREAD , len +6 );
tools/perf/util/hist.h +1 -0
@@ -24,6 +24,7 @@ enum hist_filter {enum hist_column {HISTC_SYMBOL , +HISTC_ADDR ,HISTC_DSO ,HISTC_THREAD ,HISTC_COMM ,
tools/perf/util/sort.c +29 -0
@@ -280,6 +280,34 @@ struct sort_entry sort_sym = { .se_width_idx = HISTC_SYMBOL, }; +static int64_t +sort__addr_cmp (struct hist_entry *left,struct hist_entry *right) +{ +return _sort__addr_cmp (left->ip, right->ip); +} + +static int hist_entry__addr_snprintf (struct hist_entry *he,char *bf, +size_t size,unsigned int width) +{ +struct map *map = he->ms.map; +u64 ip; + +if (map) + ip = map->unmap_ip (map, he->ip); +else + ip = he->ip; + +return _hist_entry__sym_snprintf (NULL ,NULL , ip, + he->level, bf, size, width); +} + +struct sort_entry sort_addr = { + .se_header ="Addr" , + .se_cmp = sort__addr_cmp, + .se_snprintf = hist_entry__addr_snprintf, + .se_width_idx =HISTC_ADDR , +}; +/* --sort srcline */ static int64_t
@@ -1170,6 +1198,7 @@ static struct sort_dimension common_sort_dimensions[] = {DIM (SORT_COMM ,"comm" , sort_comm),DIM (SORT_DSO ,"dso" , sort_dso),DIM (SORT_SYM ,"symbol" , sort_sym), +DIM (SORT_ADDR ,"addr" , sort_addr),DIM (SORT_PARENT ,"parent" , sort_parent),DIM (SORT_CPU ,"cpu" , sort_cpu), DIM(SORT_SRCLINE, "srcline", sort_srcline),
tools/perf/util/sort.h +2 -0
@@ -38,6 +38,7 @@ extern enum sort_mode sort__mode;extern struct sort_entry sort_comm;extern struct sort_entry sort_dso;extern struct sort_entry sort_sym; +extern struct sort_entry sort_addr;extern struct sort_entry sort_parent;extern struct sort_entry sort_dso_from;extern struct sort_entry sort_dso_to;
@@ -161,6 +162,7 @@ enum sort_type {SORT_COMM ,SORT_DSO ,SORT_SYM , +SORT_ADDR ,SORT_PARENT ,SORT_CPU , SORT_SRCLINE,
Signature · 1 line
1.8.3.2
Namhyung Kim
maintainerRe: [PATCH V2 3/3] perf tool: Add sort key addr for perf diff
Quoted 7 lines · kan liang
From: Kan Liang <[email protected]>
Sometime, especially debugging scaling issue, the function level diff may be high granularity. The user may want to do deeper diff analysis for some cache or lock issue. The "addr" key can let the user sort differential profile by ips. This feature should only work when the perf.data comes from same binary.
I'd like to suggest you to add "symoff" sort key rather than "addr" key. I think it's better since changes in one function would not affect to others. It'd look like below..
$ perf report -s comm,dso,symoff
# Overhead Command Shared Object Symbol+Offset
# ........ .............. ................... ...............................
#
25.96% swapper [kernel.kallsyms] [k] intel_idle+0x3c
8.51% kworker/9:0 [kernel.kallsyms] [k] smp_call_function_many+0x1d
6.46% swapper [kernel.kallsyms] [k] idle_enter_fair+0xa0
6.23% Xorg [kernel.kallsyms] [k] add_wait_queue+0x09
6.02% synergys libc-2.17.so [.] malloc+0x21What do you think?
Thanks,
Namhyung
Quoted 142 lines
Signed-off-by Kan Liang <[email protected]>--- tools/perf/Documentation/perf-diff.txt | 7 +++++-- tools/perf/builtin-diff.c | 2 +- tools/perf/util/hist.c | 5 +++-- tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 29 +++++++++++++++++++++++++++++ tools/perf/util/sort.h | 2 ++ 6 files changed, 41 insertions(+), 5 deletions(-)tools/perf/Documentation/perf-diff.txt +5
-2@@ -50,8 +50,11 @@ OPTIONS -s:: --sort=:: - Sort by key(s): pid, comm, dso, symbol, cpu, parent, srcline. - Please see description of --sort in the perf-report man page. + Sort by key(s): pid, comm, dso, symbol, cpu, parent, srcline, addr. + + - addr: Address executed at the time of sample (for same binary compare) + + For other keys, please see description of --sort in the perf-report man page. -t:: --field-separator=::tools/perf/builtin-diff.c +1
-1@@ -744,7 +744,7 @@ static const struct option options[] = {OPT_STRING ('S' ,"symbols" , &symbol_conf.sym_list_str,"symbol[,symbol...]" ,"only consider these symbols" ),OPT_STRING ('s' ,"sort" , &sort_order,"key[,key2...]" , -"sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, ..." +"sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, addr, ..." " Please refer the man page for the complete list." ),OPT_STRING ('t' ,"field-separator" , &symbol_conf.field_sep,"separator" , "separator for columns, no spaces will be added between "tools/perf/util/hist.c +3
-2@@ -67,13 +67,14 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h) symlen = h->ms.sym->namelen +4 ;if (verbose) symlen +=BITS_PER_LONG /4 +2 +3 ; -hists__new_col_len (hists,HISTC_SYMBOL , symlen); }else { symlen = unresolved_col_width +4 +2 ; -hists__new_col_len (hists,HISTC_SYMBOL , symlen);hists__set_unres_dso_col_len (hists,HISTC_DSO ); } +hists__new_col_len (hists,HISTC_SYMBOL , symlen); +hists__new_col_len (hists,HISTC_ADDR , symlen); + len =thread__comm_len (h->thread);if (hists__new_col_len (hists,HISTC_COMM , len))hists__set_col_len (hists,HISTC_THREAD , len +6 );tools/perf/util/hist.h +1
-0@@ -24,6 +24,7 @@ enum hist_filter {enum hist_column {HISTC_SYMBOL , +HISTC_ADDR ,HISTC_DSO ,HISTC_THREAD ,HISTC_COMM ,tools/perf/util/sort.c +29
-0@@ -280,6 +280,34 @@ struct sort_entry sort_sym = { .se_width_idx = HISTC_SYMBOL, }; +static int64_t +sort__addr_cmp (struct hist_entry *left,struct hist_entry *right) +{ +return _sort__addr_cmp (left->ip, right->ip); +} + +static int hist_entry__addr_snprintf (struct hist_entry *he,char *bf, +size_t size,unsigned int width) +{ +struct map *map = he->ms.map; +u64 ip; + +if (map) + ip = map->unmap_ip (map, he->ip); +else + ip = he->ip; + +return _hist_entry__sym_snprintf (NULL ,NULL , ip, + he->level, bf, size, width); +} + +struct sort_entry sort_addr = { + .se_header ="Addr" , + .se_cmp = sort__addr_cmp, + .se_snprintf = hist_entry__addr_snprintf, + .se_width_idx =HISTC_ADDR , +}; +/* --sort srcline */ static int64_t@@ -1170,6 +1198,7 @@ static struct sort_dimension common_sort_dimensions[] = {DIM (SORT_COMM ,"comm" , sort_comm),DIM (SORT_DSO ,"dso" , sort_dso),DIM (SORT_SYM ,"symbol" , sort_sym), +DIM (SORT_ADDR ,"addr" , sort_addr),DIM (SORT_PARENT ,"parent" , sort_parent),DIM (SORT_CPU ,"cpu" , sort_cpu), DIM(SORT_SRCLINE, "srcline", sort_srcline),tools/perf/util/sort.h +2
-0@@ -38,6 +38,7 @@ extern enum sort_mode sort__mode;extern struct sort_entry sort_comm;extern struct sort_entry sort_dso;extern struct sort_entry sort_sym; +extern struct sort_entry sort_addr;extern struct sort_entry sort_parent;extern struct sort_entry sort_dso_from;extern struct sort_entry sort_dso_to;@@ -161,6 +162,7 @@ enum sort_type {SORT_COMM ,SORT_DSO ,SORT_SYM , +SORT_ADDR ,SORT_PARENT ,SORT_CPU , SORT_SRCLINE,