Re: [PATCH v3 3/9] perf annotate: Fix wrong --show-total-period option showing number of samples
From: Taeung Song
Date: Mon Jul 24 2017 - 17:29:00 EST
On 07/25/2017 02:37 AM, Arnaldo Carvalho de Melo wrote:
Em Mon, Jul 24, 2017 at 10:51:58AM +0900, Taeung Song escreveu:
Hi Arnaldo,
Sorry, I'm too late.
On 07/21/2017 08:24 PM, Arnaldo Carvalho de Melo wrote:
Em Fri, Jul 21, 2017 at 06:41:29PM +0900, Taeung Song escreveu:
On 07/21/2017 04:19 AM, Arnaldo Carvalho de Melo wrote:
Em Thu, Jul 20, 2017 at 06:36:55AM +0900, Taeung Song escreveu:
Currently the --show-total-period option of perf-annotate
is different from perf-report's.
For example, perf-report ordinarily shows period and number of samples.
# Overhead Samples Period Command Shared Object Symbol
9.83% 102 84813704 test test [.] knapsack
But this is not what this patch does, it is still doing too many things,
it should first add sample to those function signatures, leaving the
"meat" to a followup patch, where we will not be distracted with
infrastructure needed to do what you describe in the changelog.
I'm doing it here this time, please look at the result, later.
ok, I'm waiting for it.
And if you give me some sketchy code, that's fine.
If you do, I'll remake this patch based on the result. :)
Take a look at the acme/tmp_perf/core, that is where I got yesterday.
- Arnaldo
I fetched all branch of your repository, but it don't seem to be pushed.
$ git remote get-url acme
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
$ git branch -a | grep acme | grep tmp | grep core
remotes/acme/tmp.perf/core
remotes/acme/tmp_perf.core
$ git show tmp.perf/core | head -3
commit 4827c52cd001e208704ab733a389c96ae2e70e5b
Author: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Date: Fri Jul 21 12:25:57 2017 -0700
The one above, look further down, from 896bccd3cb8d to 0321d0281cbb,
there are some more missing, but the ones below should be ready for
sending to Ingo, which I plan to do today.
I didn't see them..
Thank you for adjusted patchset.
I checked them a while ago.
Would you add Reported-by: Namhyung Kim
to the commit 0321d0281cbbb404ea73f9e1869ec8db42e8ddfd ?
And we can handle other patches later,
but how about additionally fixing the header
on the annotate stdio browser ?
(If you don't want to handle additional patches anymore today,
I'll send them with next patchset.)
"Percent" -> "Event Count"
Because I think it would be better to show the proper first column.
Moreover there is the below case that is not aligned due to big period
values.
perf annotate --stdio -i milian.data --show-total-period
Percent | Source code & Disassembly of test for cycles:ppp (1442
samples)
-------------------------------------------------------------------------------
:
:
:
: Disassembly of section .text:
...
0 : 40089d: pxor %xmm1,%xmm1
27288350 : 4008a1: cvtsi2sd %rsi,%xmm1
0 : 4008a6: pxor %xmm5,%xmm5
So, I made a patch like below:
commit 6b96b9947e83474bd6e6fd09f93c390536bb435b
Author: Taeung Song <treeze.taeung@xxxxxxxxx>
Date: Tue Jul 25 06:17:59 2017 +0900
perf annotate: Show the proper header when using --show-total-period
Currently a first column is only "Percent",
so fix it to show correct column name based on given options.
(e.g. if using --show-total-period, show "Event count" as a first
column)
Reported-by: Milian Wolff <milian.wolff@xxxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 004072f..0224595 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1142,7 +1142,7 @@ static int disasm_line__print(struct disasm_line
*dl, struct symbol *sym, u64 st
color = get_percent_color(percent);
if (symbol_conf.show_total_period)
- color_fprintf(stdout, color, " %7" PRIu64,
+ color_fprintf(stdout, color, " %11" PRIu64,
sample.period);
else
color_fprintf(stdout, color, " %7.2f",
percent);
@@ -1173,6 +1173,10 @@ static int disasm_line__print(struct disasm_line
*dl, struct symbol *sym, u64 st
if (perf_evsel__is_group_event(evsel))
width *= evsel->nr_members;
+ if (symbol_conf.show_total_period)
+ width += perf_evsel__is_group_event(evsel) ?
+ 4 * evsel->nr_members : 4;
+
if (!*dl->line)
printf(" %*s:\n", width, " ");
else
@@ -1823,8 +1827,14 @@ int symbol__annotate_printf(struct symbol *sym,
struct map *map,
if (perf_evsel__is_group_event(evsel))
width *= evsel->nr_members;
+ if (symbol_conf.show_total_period)
+ width += perf_evsel__is_group_event(evsel) ?
+ 4 * evsel->nr_members : 4;
+
graph_dotted_len = printf(" %-*.*s| Source code &
Disassembly of %s for %s (%" PRIu64 " samples)\n",
- width, width, "Percent", d_filename, evsel_name,
h->nr_samples);
+ width, width,
+ symbol_conf.show_total_period ?
"Event count" : "Percent",
+ d_filename, evsel_name, h->nr_samples);
printf("%-*.*s----\n",
graph_dotted_len, graph_dotted_len, graph_dotted_line);
Thanks,
Taeung
4827c52cd001 perf jevents: Make build fail on JSON parse error
768750b250b4 perf top: Support lookup of symbols in other mount namespaces.
bd09c1d5f473 perf stat: Use group read for event groups
b9830d226aae perf evsel: Add read_counter() method
e9c58e8b74b8 perf evsel: Add read_size method
b1f952a065d6 perf evsel: Add verbose output for sys_perf_event_open fallback
1321cb6fb1ac perf jvmti: Fix linker error when libelf config is disabled
27cf8ae9ac36 perf annotate: Process tracing data in pipe mode
76dd07b982e0 perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile
f246e8b7f34e perf cgroup: Fix refcount usage
60cdc09e7d77 perf report: Fix kernel symbol adjustment for s390x
0321d0281cbb perf annotate stdio: Fix --show-total-period
ecd5f9959d2c perf annotate: Do not overwrite sample->period
461c17f00f40 perf annotate: Store the sample period in each histogram bucket
bab89f6aed7e perf hists: Pass perf_sample to __symbol__inc_addr_samples()
8158683da3d3 perf annotate: Rename 'sum' to 'nr_samples' in struct sym_hist
896bccd3cb8d perf annotate: Introduce struct sym_hist_entry
$ git show tmp_perf.core | head -3
commit 3331778eb08a50cec959da933c040bd7fbdde396
Author: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Date: Thu Jul 31 09:00:56 2014 +0300
Thanks,
Taeung