LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] perf annotate: Support multiple events without group
@ 2018-05-10 13:59 Jin Yao
  2018-05-17  4:53 ` Jin, Yao
  2018-05-17 16:27 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 5+ messages in thread
From: Jin Yao @ 2018-05-10 13:59 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

See example,

perf record -e cycles,branches ./div
perf annotate main --stdio or
perf annotate main --stdio2 or
perf annotate main

The "perf annotate" should show both cycles and branches on the
left side, but actually it only shows one event cycles.

It works with events group like:
perf record -e "{cycles,branches}" ./div

It should work too even without group.

This patch uses 'symbol_conf.nr_events > 1' to check if it's the
multiple events case.

With this patch,

perf record -e cycles,branches ./div
perf annotate main --stdio2

  ......
                           for (i = 0; i < 2000000000; i++) {
                                   flag = compute_flag();
  4.85   5.83  38:   xor    %eax,%eax
  0.01   0.01      → callq  compute_flag

                                   count++;
  4.44   2.27        mov    count,%edx
  0.00   0.00        add    $0x1,%edx
  ......

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/util/annotate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 5d74a30..ca8d4b1 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1054,6 +1054,8 @@ annotation_line__new(struct annotate_args *args, size_t privsize)
 
 	if (perf_evsel__is_group_event(evsel))
 		nr = evsel->nr_members;
+	else if (symbol_conf.nr_events > nr)
+		nr = symbol_conf.nr_events;
 
 	size += sizeof(al->samples[0]) * nr;
 
@@ -1326,6 +1328,8 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 
 		if (perf_evsel__is_group_event(evsel))
 			width *= evsel->nr_members;
+		else if (symbol_conf.nr_events > 1)
+			width *= symbol_conf.nr_events;
 
 		if (!*al->line)
 			printf(" %*s:\n", width, " ");
@@ -1967,6 +1971,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
 
 	if (perf_evsel__is_group_event(evsel))
 		width *= evsel->nr_members;
+	else if (symbol_conf.nr_events > 1)
+		width *= symbol_conf.nr_events;
 
 	graph_dotted_len = printf(" %-*.*s|	Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n",
 				  width, width, symbol_conf.show_total_period ? "Period" :
@@ -2578,6 +2584,8 @@ int symbol__annotate2(struct symbol *sym, struct map *map, struct perf_evsel *ev
 
 	if (perf_evsel__is_group_event(evsel))
 		nr_pcnt = evsel->nr_members;
+	else if (symbol_conf.nr_events > nr_pcnt)
+		nr_pcnt = symbol_conf.nr_events;
 
 	err = symbol__annotate(sym, map, evsel, 0, parch);
 	if (err)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf annotate: Support multiple events without group
  2018-05-10 13:59 [PATCH] perf annotate: Support multiple events without group Jin Yao
@ 2018-05-17  4:53 ` Jin, Yao
  2018-05-17 16:27 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 5+ messages in thread
From: Jin, Yao @ 2018-05-17  4:53 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin

Hi,

Any comments for this fix?

Thanks
Jin Yao

On 5/10/2018 9:59 PM, Jin Yao wrote:
> See example,
> 
> perf record -e cycles,branches ./div
> perf annotate main --stdio or
> perf annotate main --stdio2 or
> perf annotate main
> 
> The "perf annotate" should show both cycles and branches on the
> left side, but actually it only shows one event cycles.
> 
> It works with events group like:
> perf record -e "{cycles,branches}" ./div
> 
> It should work too even without group.
> 
> This patch uses 'symbol_conf.nr_events > 1' to check if it's the
> multiple events case.
> 
> With this patch,
> 
> perf record -e cycles,branches ./div
> perf annotate main --stdio2
> 
>    ......
>                             for (i = 0; i < 2000000000; i++) {
>                                     flag = compute_flag();
>    4.85   5.83  38:   xor    %eax,%eax
>    0.01   0.01      → callq  compute_flag
> 
>                                     count++;
>    4.44   2.27        mov    count,%edx
>    0.00   0.00        add    $0x1,%edx
>    ......
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>   tools/perf/util/annotate.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 5d74a30..ca8d4b1 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1054,6 +1054,8 @@ annotation_line__new(struct annotate_args *args, size_t privsize)
>   
>   	if (perf_evsel__is_group_event(evsel))
>   		nr = evsel->nr_members;
> +	else if (symbol_conf.nr_events > nr)
> +		nr = symbol_conf.nr_events;
>   
>   	size += sizeof(al->samples[0]) * nr;
>   
> @@ -1326,6 +1328,8 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>   
>   		if (perf_evsel__is_group_event(evsel))
>   			width *= evsel->nr_members;
> +		else if (symbol_conf.nr_events > 1)
> +			width *= symbol_conf.nr_events;
>   
>   		if (!*al->line)
>   			printf(" %*s:\n", width, " ");
> @@ -1967,6 +1971,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
>   
>   	if (perf_evsel__is_group_event(evsel))
>   		width *= evsel->nr_members;
> +	else if (symbol_conf.nr_events > 1)
> +		width *= symbol_conf.nr_events;
>   
>   	graph_dotted_len = printf(" %-*.*s|	Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n",
>   				  width, width, symbol_conf.show_total_period ? "Period" :
> @@ -2578,6 +2584,8 @@ int symbol__annotate2(struct symbol *sym, struct map *map, struct perf_evsel *ev
>   
>   	if (perf_evsel__is_group_event(evsel))
>   		nr_pcnt = evsel->nr_members;
> +	else if (symbol_conf.nr_events > nr_pcnt)
> +		nr_pcnt = symbol_conf.nr_events;
>   
>   	err = symbol__annotate(sym, map, evsel, 0, parch);
>   	if (err)
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf annotate: Support multiple events without group
  2018-05-10 13:59 [PATCH] perf annotate: Support multiple events without group Jin Yao
  2018-05-17  4:53 ` Jin, Yao
@ 2018-05-17 16:27 ` Arnaldo Carvalho de Melo
  2018-05-17 20:27   ` Jiri Olsa
  1 sibling, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-05-17 16:27 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Em Thu, May 10, 2018 at 09:59:22PM +0800, Jin Yao escreveu:
> See example,
> 
> perf record -e cycles,branches ./div
> perf annotate main --stdio or
> perf annotate main --stdio2 or
> perf annotate main
> 
> The "perf annotate" should show both cycles and branches on the
> left side, but actually it only shows one event cycles.
> 
> It works with events group like:
> perf record -e "{cycles,branches}" ./div
> 
> It should work too even without group.

Humm, I think that this should be done the way it is for perf report,
i.e. you select the group output by using --group, no?

Jiri, isn't that how it is done in 'perf report' for non-explicit
groups?

- Arnaldo
 
> This patch uses 'symbol_conf.nr_events > 1' to check if it's the
> multiple events case.
> 
> With this patch,
> 
> perf record -e cycles,branches ./div
> perf annotate main --stdio2
> 
>   ......
>                            for (i = 0; i < 2000000000; i++) {
>                                    flag = compute_flag();
>   4.85   5.83  38:   xor    %eax,%eax
>   0.01   0.01      → callq  compute_flag
> 
>                                    count++;
>   4.44   2.27        mov    count,%edx
>   0.00   0.00        add    $0x1,%edx
>   ......
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/util/annotate.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 5d74a30..ca8d4b1 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1054,6 +1054,8 @@ annotation_line__new(struct annotate_args *args, size_t privsize)
>  
>  	if (perf_evsel__is_group_event(evsel))
>  		nr = evsel->nr_members;
> +	else if (symbol_conf.nr_events > nr)
> +		nr = symbol_conf.nr_events;
>  
>  	size += sizeof(al->samples[0]) * nr;
>  
> @@ -1326,6 +1328,8 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
>  
>  		if (perf_evsel__is_group_event(evsel))
>  			width *= evsel->nr_members;
> +		else if (symbol_conf.nr_events > 1)
> +			width *= symbol_conf.nr_events;
>  
>  		if (!*al->line)
>  			printf(" %*s:\n", width, " ");
> @@ -1967,6 +1971,8 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
>  
>  	if (perf_evsel__is_group_event(evsel))
>  		width *= evsel->nr_members;
> +	else if (symbol_conf.nr_events > 1)
> +		width *= symbol_conf.nr_events;
>  
>  	graph_dotted_len = printf(" %-*.*s|	Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n",
>  				  width, width, symbol_conf.show_total_period ? "Period" :
> @@ -2578,6 +2584,8 @@ int symbol__annotate2(struct symbol *sym, struct map *map, struct perf_evsel *ev
>  
>  	if (perf_evsel__is_group_event(evsel))
>  		nr_pcnt = evsel->nr_members;
> +	else if (symbol_conf.nr_events > nr_pcnt)
> +		nr_pcnt = symbol_conf.nr_events;
>  
>  	err = symbol__annotate(sym, map, evsel, 0, parch);
>  	if (err)
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf annotate: Support multiple events without group
  2018-05-17 16:27 ` Arnaldo Carvalho de Melo
@ 2018-05-17 20:27   ` Jiri Olsa
  2018-05-18  0:28     ` Jin, Yao
  0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2018-05-17 20:27 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jin Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	ak, kan.liang, yao.jin

On Thu, May 17, 2018 at 01:27:50PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 10, 2018 at 09:59:22PM +0800, Jin Yao escreveu:
> > See example,
> > 
> > perf record -e cycles,branches ./div
> > perf annotate main --stdio or
> > perf annotate main --stdio2 or
> > perf annotate main
> > 
> > The "perf annotate" should show both cycles and branches on the
> > left side, but actually it only shows one event cycles.
> > 
> > It works with events group like:
> > perf record -e "{cycles,branches}" ./div
> > 
> > It should work too even without group.
> 
> Humm, I think that this should be done the way it is for perf report,
> i.e. you select the group output by using --group, no?
> 
> Jiri, isn't that how it is done in 'perf report' for non-explicit
> groups?

yep, if there's no event group, the --group will enable the group
output in report.. sounds ok to follow this also in annotate

'perf report' TUI offers list of events to choose from
and the --stdio version prints report for each event
annotate could do the same

jirka

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf annotate: Support multiple events without group
  2018-05-17 20:27   ` Jiri Olsa
@ 2018-05-18  0:28     ` Jin, Yao
  0 siblings, 0 replies; 5+ messages in thread
From: Jin, Yao @ 2018-05-18  0:28 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin



On 5/18/2018 4:27 AM, Jiri Olsa wrote:
> On Thu, May 17, 2018 at 01:27:50PM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Thu, May 10, 2018 at 09:59:22PM +0800, Jin Yao escreveu:
>>> See example,
>>>
>>> perf record -e cycles,branches ./div
>>> perf annotate main --stdio or
>>> perf annotate main --stdio2 or
>>> perf annotate main
>>>
>>> The "perf annotate" should show both cycles and branches on the
>>> left side, but actually it only shows one event cycles.
>>>
>>> It works with events group like:
>>> perf record -e "{cycles,branches}" ./div
>>>
>>> It should work too even without group.
>>
>> Humm, I think that this should be done the way it is for perf report,
>> i.e. you select the group output by using --group, no?
>>
>> Jiri, isn't that how it is done in 'perf report' for non-explicit
>> groups?
> 
> yep, if there's no event group, the --group will enable the group
> output in report.. sounds ok to follow this also in annotate
> 
> 'perf report' TUI offers list of events to choose from
> and the --stdio version prints report for each event
> annotate could do the same
> 
> jirka
> 

Hi Jiri, Arnaldo,

OK, thanks, I will look at --group and see how it works.

Yes, using a unified interface (--group) for non-explicit groups is a 
good choice.

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-18  0:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-10 13:59 [PATCH] perf annotate: Support multiple events without group Jin Yao
2018-05-17  4:53 ` Jin, Yao
2018-05-17 16:27 ` Arnaldo Carvalho de Melo
2018-05-17 20:27   ` Jiri Olsa
2018-05-18  0:28     ` Jin, Yao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).