2014-11-21 16:12:58

by Liang, Kan

[permalink] [raw]
Subject: [PATCH V5 1/1] perf tool:perf diff support for different binaries

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.

Here is a simple examples (Only change f2 and f3 functions location.)
There is no change in behavior, but the old perf diff display the wrong
differential profile.

example_v1.c

noinline void f3(void)
{
volatile int i;
for (i = 0; i < 10000;) {

if(i%2)
i++;
else
i++;
}
}

noinline void f2(void)
{
volatile int a = 100, b, c;
for (b = 0; b < 10000; b++)
c = a * b;

}

noinline void f1(void)
{
f2();
f3();
}

int main()
{
int i;
for (i = 0; i < 100000; i++)
f1();
}

example_v2.c

noinline void f2(void)
{
volatile int a = 100, b, c;
for (b = 0; b < 10000; b++)
c = a * b;
}

noinline void f3(void)
{
volatile int i;
for (i = 0; i < 10000;) {
if(i%2)
i++;
else
i++;
}
}

noinline void f1(void)
{
f2();
f3();
}

int main()
{
int i;
for (i = 0; i < 100000; i++)
f1();
}

[lk@localhost perf_diff]$ gcc example_v1.c -o example
[lk@localhost perf_diff]$ perf record -o example_v1.data ./example
[ perf record: Woken up 4 times to write data ]
[ perf record: Captured and wrote 0.813 MB example_v1.data (~35522
samples) ]

[lk@localhost perf_diff]$ gcc example_v2.c -o example
[lk@localhost perf_diff]$ perf record -o example_v2.data ./example
[ perf record: Woken up 4 times to write data ]
[ perf record: Captured and wrote 0.824 MB example_v2.data (~36015
samples) ]

Old perf diff result.
[lk@localhost perf_diff]$ perf diff example_v1.data example_v2.data
Event 'cycles'

Baseline Delta Shared Object Symbol
........ ....... ................ ...............................

[kernel.vmlinux] [k] __perf_event_task_sched_out
0.00% [kernel.vmlinux] [k] apic_timer_interrupt
[kernel.vmlinux] [k] idle_cpu
[kernel.vmlinux] [k] intel_pstate_timer_func
[kernel.vmlinux] [k] native_read_msr_safe
0.00% [kernel.vmlinux] [k] native_read_tsc
0.00% [kernel.vmlinux] [k] native_write_msr_safe
[kernel.vmlinux] [k] ntp_tick_length
0.00% [kernel.vmlinux] [k] rb_erase
0.00% [kernel.vmlinux] [k] tick_sched_timer
0.00% [kernel.vmlinux] [k] unmap_single_vma
0.00% [kernel.vmlinux] [k] update_wall_time
0.00% example [.] f1
46.24% example [.] f2
53.71% -7.55% example [.] f3
+53.81% example [.] f3
0.02% example [.] main

New perf diff result.
[lk@localhost perf_diff]$ perf diff example_v1.data example_v2.data
Event 'cycles'

Baseline Delta Shared Object Symbol
........ ....... ................ ...............................

[kernel.vmlinux] [k] __perf_event_task_sched_out
0.00% [kernel.vmlinux] [k] apic_timer_interrupt
[kernel.vmlinux] [k] idle_cpu
[kernel.vmlinux] [k] intel_pstate_timer_func
[kernel.vmlinux] [k] native_read_msr_safe
0.00% [kernel.vmlinux] [k] native_read_tsc
0.00% [kernel.vmlinux] [k] native_write_msr_safe
[kernel.vmlinux] [k] ntp_tick_length
0.00% [kernel.vmlinux] [k] rb_erase
0.00% [kernel.vmlinux] [k] tick_sched_timer
0.00% [kernel.vmlinux] [k] unmap_single_vma
0.00% [kernel.vmlinux] [k] update_wall_time
0.00% example [.] f1
46.24% -0.08% example [.] f2
53.71% +0.11% example [.] f3
0.02% example [.] main

Signed-off-by: Kan Liang <[email protected]>
Acked-by: Namhyung Kim <[email protected]>

---
The patch is seperated from "[PATCH V4 0/3] perf tool: perf diff sort changes" patch set.
The first patch of the patch set has been merged.
The third patch of the patch set for symoff will be sent out by another thread.

Changes since V4:
- Seperate from old patch set
- Add an example in changelog
- Update man page

tools/perf/Documentation/perf-diff.txt | 5 +++++
tools/perf/util/sort.c | 9 +++++++++
2 files changed, 14 insertions(+)

diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index e463caa..cf2be66 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -20,6 +20,11 @@ If no parameters are passed it will assume perf.data.old and perf.data.
The differential profile is displayed only for events matching both
specified perf.data files.

+If no parameters are passed the samples will be sorted by dso and symbol.
+As the perf.data files could come from different binaries, the symbols addresses
+could vary. So perf diff is based on the comparison of the files and
+symbols name.
+
OPTIONS
-------
-D::
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 82a5596..91a1c6b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -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, but the name of symbol.
+ */
+ if (sort__mode == SORT_MODE__DIFF)
+ sd->entry->se_collapse = sort__sym_sort;
+
} else if (sd->entry == &sort_dso) {
sort__has_dso = 1;
}
--
1.8.3.2


2014-12-01 20:17:46

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH V5 1/1] perf tool:perf diff support for different binaries

On Fri, Nov 21, 2014 at 10:55:48AM -0500, [email protected] wrote:

SNIP

> 0.00% [kernel.vmlinux] [k] native_read_tsc
> 0.00% [kernel.vmlinux] [k] native_write_msr_safe
> [kernel.vmlinux] [k] ntp_tick_length
> 0.00% [kernel.vmlinux] [k] rb_erase
> 0.00% [kernel.vmlinux] [k] tick_sched_timer
> 0.00% [kernel.vmlinux] [k] unmap_single_vma
> 0.00% [kernel.vmlinux] [k] update_wall_time
> 0.00% example [.] f1
> 46.24% -0.08% example [.] f2
> 53.71% +0.11% example [.] f3
> 0.02% example [.] main
>
> Signed-off-by: Kan Liang <[email protected]>
> Acked-by: Namhyung Kim <[email protected]>

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

2014-12-01 22:35:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH V5 1/1] perf tool:perf diff support for different binaries

Em Fri, Nov 21, 2014 at 10:55:48AM -0500, [email protected] escreveu:
> 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.
>
> Here is a simple examples (Only change f2 and f3 functions location.)
> There is no change in behavior, but the old perf diff display the wrong
> differential profile.

[acme@zoo linux]$ am /wb/1.patch
Applying: perf tool:perf diff support for different binaries
/home/acme/git/linux/.git/rebase-apply/patch:23: trailing whitespace.
If no parameters are passed the samples will be sorted by dso and
symbol.
/home/acme/git/linux/.git/rebase-apply/patch:26: trailing whitespace.
symbols name.
warning: 2 lines add whitespace errors.

[1]+ Stopped am /wb/1.patch
[acme@zoo linux]$

Fixing this up this time.

Please consider:

chmod +x .git/hooks/*

- Arnaldo

2014-12-02 15:51:20

by Liang, Kan

[permalink] [raw]
Subject: RE: [PATCH V5 1/1] perf tool:perf diff support for different binaries



>
> Em Fri, Nov 21, 2014 at 10:55:48AM -0500, [email protected] escreveu:
> > 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.
> >
> > Here is a simple examples (Only change f2 and f3 functions location.)
> > There is no change in behavior, but the old perf diff display the
> > wrong differential profile.
>
> [acme@zoo linux]$ am /wb/1.patch
> Applying: perf tool:perf diff support for different binaries
> /home/acme/git/linux/.git/rebase-apply/patch:23: trailing whitespace.
> If no parameters are passed the samples will be sorted by dso and symbol.
> /home/acme/git/linux/.git/rebase-apply/patch:26: trailing whitespace.
> symbols name.
> warning: 2 lines add whitespace errors.
>

Sorry for the inconvenience, I will repost a clean version then.

Thanks,
Kan

> [1]+ Stopped am /wb/1.patch
> [acme@zoo linux]$
>
> Fixing this up this time.
>
> Please consider:
>
> chmod +x .git/hooks/*
>
> - Arnaldo