[PATCH] perf stat: Create '--add-default' option to append default list

[PATCH] perf stat: Create '--add-default' option to append default list

[Jin Yao]

The event default list includes the most common events which are widely
used by users. But with -e option, the current perf only counts the events
assigned by -e option. Users may want to collect some extra events with
the default list. For this case, users have to manually add all the events
from the default list. It's inconvenient. Also, users may don't know how to
get the default list.

It's better to add a new option to append default list to the -e events.
The new option is '--add-default'.

Before:

root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a -- sleep 1

 Performance counter stats for 'system wide':

              2.05 Joules power/energy-pkg/

       1.000857974 seconds time elapsed

After:

root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a --add-default -- sleep 1

 Performance counter stats for 'system wide':

              2.10 Joules power/energy-pkg/         #    0.000 K/sec
          8,009.89 msec   cpu-clock                 #    7.995 CPUs utilized
               140        context-switches          #    0.017 K/sec
                 9        cpu-migrations            #    0.001 K/sec
                66        page-faults               #    0.008 K/sec
        10,671,929        cycles                    #    0.001 GHz
         4,736,880        instructions              #    0.44  insn per cycle
           942,951        branches                  #    0.118 M/sec
            76,096        branch-misses             #    8.07% of all branches

       1.001809960 seconds time elapsed

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 tools/perf/Documentation/perf-stat.txt | 5 +++++
 tools/perf/builtin-stat.c              | 4 +++-
 tools/perf/util/stat.h                 | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 5d4a673d7621..75a83c2e4dc5 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -438,6 +438,11 @@ convenient for post processing.
 --summary::
 Print summary for interval mode (-I).
 
+--add-default::
+The default event list includes the most common events which are widely
+used by users. But with -e option, the perf only counts the events assigned
+by -e option. This options appends the default event list to the -e events.
+
 EXAMPLES
 --------
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 89c32692f40c..6ac7b946f9a7 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1173,6 +1173,8 @@ static struct option stat_options[] = {
 		       "print summary for interval mode"),
 	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
 			"don't print output (useful with record)"),
+	OPT_BOOLEAN(0, "add-default", &stat_config.add_default,
+		       "add default events"),
 #ifdef HAVE_LIBPFM
 	OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
 		"libpfm4 event selector. use 'perf list' to list available events",
@@ -1755,7 +1757,7 @@ static int add_default_attributes(void)
 		free(str);
 	}
 
-	if (!evsel_list->core.nr_entries) {
+	if (!evsel_list->core.nr_entries || stat_config.add_default) {
 		if (target__has_cpu(&target))
 			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
 
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 9979b4b100f2..6ccc6936348c 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -123,6 +123,7 @@ struct perf_stat_config {
 	bool			 metric_no_merge;
 	bool			 stop_read_counter;
 	bool			 quiet;
+	bool			 add_default;
 	FILE			*output;
 	unsigned int		 interval;
 	unsigned int		 timeout;
-- 
2.17.1

Re: [PATCH] perf stat: Create '--add-default' option to append default list

[Arnaldo Carvalho de Melo]

Em Tue, Dec 22, 2020 at 09:11:31AM +0800, Jin Yao escreveu:
> The event default list includes the most common events which are widely
> used by users. But with -e option, the current perf only counts the events
> assigned by -e option. Users may want to collect some extra events with
> the default list. For this case, users have to manually add all the events
> from the default list. It's inconvenient. Also, users may don't know how to
> get the default list.
> 
> It's better to add a new option to append default list to the -e events.
> The new option is '--add-default'.
> 
> Before:
> 
> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a -- sleep 1
> 
>  Performance counter stats for 'system wide':
> 
>               2.05 Joules power/energy-pkg/
> 
>        1.000857974 seconds time elapsed
> 
> After:
> 
> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a --add-default -- sleep 1

I thought about:

    perf stat -e +power/energy-pkg/ -a -- sleep 1

Which would have its counterpart:

    perf stat -e -cycles -0a --sleep 1

To remove an event from the defaults, perhaps to deal with some specific
hardware where the default or what is in -d, -dd, -ddd, etc can't all be
counted. I.e. - and + would remove or add from whaver list was there at
that point.

- Arnaldo
 
>  Performance counter stats for 'system wide':
> 
>               2.10 Joules power/energy-pkg/         #    0.000 K/sec
>           8,009.89 msec   cpu-clock                 #    7.995 CPUs utilized
>                140        context-switches          #    0.017 K/sec
>                  9        cpu-migrations            #    0.001 K/sec
>                 66        page-faults               #    0.008 K/sec
>         10,671,929        cycles                    #    0.001 GHz
>          4,736,880        instructions              #    0.44  insn per cycle
>            942,951        branches                  #    0.118 M/sec
>             76,096        branch-misses             #    8.07% of all branches
> 
>        1.001809960 seconds time elapsed
> 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/Documentation/perf-stat.txt | 5 +++++
>  tools/perf/builtin-stat.c              | 4 +++-
>  tools/perf/util/stat.h                 | 1 +
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 5d4a673d7621..75a83c2e4dc5 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -438,6 +438,11 @@ convenient for post processing.
>  --summary::
>  Print summary for interval mode (-I).
>  
> +--add-default::
> +The default event list includes the most common events which are widely
> +used by users. But with -e option, the perf only counts the events assigned
> +by -e option. This options appends the default event list to the -e events.
> +
>  EXAMPLES
>  --------
>  
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 89c32692f40c..6ac7b946f9a7 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1173,6 +1173,8 @@ static struct option stat_options[] = {
>  		       "print summary for interval mode"),
>  	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
>  			"don't print output (useful with record)"),
> +	OPT_BOOLEAN(0, "add-default", &stat_config.add_default,
> +		       "add default events"),
>  #ifdef HAVE_LIBPFM
>  	OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
>  		"libpfm4 event selector. use 'perf list' to list available events",
> @@ -1755,7 +1757,7 @@ static int add_default_attributes(void)
>  		free(str);
>  	}
>  
> -	if (!evsel_list->core.nr_entries) {
> +	if (!evsel_list->core.nr_entries || stat_config.add_default) {
>  		if (target__has_cpu(&target))
>  			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
>  
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 9979b4b100f2..6ccc6936348c 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -123,6 +123,7 @@ struct perf_stat_config {
>  	bool			 metric_no_merge;
>  	bool			 stop_read_counter;
>  	bool			 quiet;
> +	bool			 add_default;
>  	FILE			*output;
>  	unsigned int		 interval;
>  	unsigned int		 timeout;
> -- 
> 2.17.1
> 

-- 

- Arnaldo

Re: [PATCH] perf stat: Create '--add-default' option to append default list

[Jin, Yao]

Hi Arnaldo,

On 12/23/2020 12:15 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Dec 22, 2020 at 09:11:31AM +0800, Jin Yao escreveu:
>> The event default list includes the most common events which are widely
>> used by users. But with -e option, the current perf only counts the events
>> assigned by -e option. Users may want to collect some extra events with
>> the default list. For this case, users have to manually add all the events
>> from the default list. It's inconvenient. Also, users may don't know how to
>> get the default list.
>>
>> It's better to add a new option to append default list to the -e events.
>> The new option is '--add-default'.
>>
>> Before:
>>
>> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a -- sleep 1
>>
>>   Performance counter stats for 'system wide':
>>
>>                2.05 Joules power/energy-pkg/
>>
>>         1.000857974 seconds time elapsed
>>
>> After:
>>
>> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a --add-default -- sleep 1
> 
> I thought about:
> 
>      perf stat -e +power/energy-pkg/ -a -- sleep 1
> 

I was surprised to see that '+<event>' syntax had been supported.

root@kbl-ppc:~# ./perf stat -e +power/energy-pkg/ -a -- sleep 1

  Performance counter stats for 'system wide':

               1.99 Joules +power/energy-pkg/

        1.000877852 seconds time elapsed

root@kbl-ppc:~# ./perf stat -e +power/energy-pkg/,+cycles -a -- sleep 1

  Performance counter stats for 'system wide':

               2.00 Joules +power/energy-pkg/
         13,780,620        +cycles

        1.001639147 seconds time elapsed

Are there any scripts or usages need the prefix '+' before event? I don't know. But if we append the 
'+<event>' to the default event list, will break something potentially?

> Which would have its counterpart:
> 
>      perf stat -e -cycles -0a --sleep 1
> 
> To remove an event from the defaults, perhaps to deal with some specific
> hardware where the default or what is in -d, -dd, -ddd, etc can't all be
> counted. I.e. - and + would remove or add from whaver list was there at
> that point.
> 
> - Arnaldo
>   

Yes, + and - are more flexible solution. Just for above question, will '+<event>' break existing 
usage? And for '-', I don't know if user can remember clearly for what the events are in default list.

Thanks
Jin Yao

>>   Performance counter stats for 'system wide':
>>
>>                2.10 Joules power/energy-pkg/         #    0.000 K/sec
>>            8,009.89 msec   cpu-clock                 #    7.995 CPUs utilized
>>                 140        context-switches          #    0.017 K/sec
>>                   9        cpu-migrations            #    0.001 K/sec
>>                  66        page-faults               #    0.008 K/sec
>>          10,671,929        cycles                    #    0.001 GHz
>>           4,736,880        instructions              #    0.44  insn per cycle
>>             942,951        branches                  #    0.118 M/sec
>>              76,096        branch-misses             #    8.07% of all branches
>>
>>         1.001809960 seconds time elapsed
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>>   tools/perf/Documentation/perf-stat.txt | 5 +++++
>>   tools/perf/builtin-stat.c              | 4 +++-
>>   tools/perf/util/stat.h                 | 1 +
>>   3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
>> index 5d4a673d7621..75a83c2e4dc5 100644
>> --- a/tools/perf/Documentation/perf-stat.txt
>> +++ b/tools/perf/Documentation/perf-stat.txt
>> @@ -438,6 +438,11 @@ convenient for post processing.
>>   --summary::
>>   Print summary for interval mode (-I).
>>   
>> +--add-default::
>> +The default event list includes the most common events which are widely
>> +used by users. But with -e option, the perf only counts the events assigned
>> +by -e option. This options appends the default event list to the -e events.
>> +
>>   EXAMPLES
>>   --------
>>   
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 89c32692f40c..6ac7b946f9a7 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1173,6 +1173,8 @@ static struct option stat_options[] = {
>>   		       "print summary for interval mode"),
>>   	OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
>>   			"don't print output (useful with record)"),
>> +	OPT_BOOLEAN(0, "add-default", &stat_config.add_default,
>> +		       "add default events"),
>>   #ifdef HAVE_LIBPFM
>>   	OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
>>   		"libpfm4 event selector. use 'perf list' to list available events",
>> @@ -1755,7 +1757,7 @@ static int add_default_attributes(void)
>>   		free(str);
>>   	}
>>   
>> -	if (!evsel_list->core.nr_entries) {
>> +	if (!evsel_list->core.nr_entries || stat_config.add_default) {
>>   		if (target__has_cpu(&target))
>>   			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
>>   
>> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
>> index 9979b4b100f2..6ccc6936348c 100644
>> --- a/tools/perf/util/stat.h
>> +++ b/tools/perf/util/stat.h
>> @@ -123,6 +123,7 @@ struct perf_stat_config {
>>   	bool			 metric_no_merge;
>>   	bool			 stop_read_counter;
>>   	bool			 quiet;
>> +	bool			 add_default;
>>   	FILE			*output;
>>   	unsigned int		 interval;
>>   	unsigned int		 timeout;
>> -- 
>> 2.17.1
>>
> 

Re: [PATCH] perf stat: Create '--add-default' option to append default list

[Jin, Yao]

On 12/23/2020 8:56 AM, Jin, Yao wrote:
> Hi Arnaldo,
> 
> On 12/23/2020 12:15 AM, Arnaldo Carvalho de Melo wrote:
>> Em Tue, Dec 22, 2020 at 09:11:31AM +0800, Jin Yao escreveu:
>>> The event default list includes the most common events which are widely
>>> used by users. But with -e option, the current perf only counts the events
>>> assigned by -e option. Users may want to collect some extra events with
>>> the default list. For this case, users have to manually add all the events
>>> from the default list. It's inconvenient. Also, users may don't know how to
>>> get the default list.
>>>
>>> It's better to add a new option to append default list to the -e events.
>>> The new option is '--add-default'.
>>>
>>> Before:
>>>
>>> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a -- sleep 1
>>>
>>>   Performance counter stats for 'system wide':
>>>
>>>                2.05 Joules power/energy-pkg/
>>>
>>>         1.000857974 seconds time elapsed
>>>
>>> After:
>>>
>>> root@kbl-ppc:~# ./perf stat -e power/energy-pkg/ -a --add-default -- sleep 1
>>
>> I thought about:
>>
>>      perf stat -e +power/energy-pkg/ -a -- sleep 1
>>
> 
> I was surprised to see that '+<event>' syntax had been supported.
> 
> root@kbl-ppc:~# ./perf stat -e +power/energy-pkg/ -a -- sleep 1
> 
>   Performance counter stats for 'system wide':
> 
>                1.99 Joules +power/energy-pkg/
> 
>         1.000877852 seconds time elapsed
> 
> root@kbl-ppc:~# ./perf stat -e +power/energy-pkg/,+cycles -a -- sleep 1
> 
>   Performance counter stats for 'system wide':
> 
>                2.00 Joules +power/energy-pkg/
>          13,780,620        +cycles
> 
>         1.001639147 seconds time elapsed
> 
> Are there any scripts or usages need the prefix '+' before event? I don't know. But if we append the 
> '+<event>' to the default event list, will break something potentially?
> 
>> Which would have its counterpart:
>>
>>      perf stat -e -cycles -0a --sleep 1
>>
>> To remove an event from the defaults, perhaps to deal with some specific
>> hardware where the default or what is in -d, -dd, -ddd, etc can't all be
>> counted. I.e. - and + would remove or add from whaver list was there at
>> that point.
>>
>> - Arnaldo
> 
> Yes, + and - are more flexible solution. Just for above question, will '+<event>' break existing 
> usage? And for '-', I don't know if user can remember clearly for what the events are in default list.
> 

For '-', another difficulty is it may conflict with the hardware cache event.

Say we remove the "-                      { return '-'; }" from parse-events.l, such as:

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 9db5097317f4..145653d1ce16 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -387,7 +387,6 @@ r{num_raw_hex}              { return raw(yyscanner); }
  {name}                 { return pmu_str_check(yyscanner, _parse_state); }
  {name_tag}             { return str(yyscanner, PE_NAME); }
  "/"                    { BEGIN(config); return '/'; }
--                      { return '-'; }
  ,                      { BEGIN(event); return ','; }
  :                      { return ':'; }
  "{"                    { BEGIN(event); return '{'; }

The syntax of '-<event>' is supported.

root@kbl-ppc:~# ./perf stat -e -cycles -a -- sleep 1

  Performance counter stats for 'system wide':

         14,008,859      -cycles

        1.001471494 seconds time elapsed

But the parsing of hardware cache event would be failed. :(

root@kbl-ppc:~# ./perf stat -e LLC-stores -a -- sleep 1
event syntax error: 'LLC-stores'
                         \___ parser error

That complicates things. :(

Thanks
Jin Yao

> Thanks
> Jin Yao
> 
>>>   Performance counter stats for 'system wide':
>>>
>>>                2.10 Joules power/energy-pkg/         #    0.000 K/sec
>>>            8,009.89 msec   cpu-clock                 #    7.995 CPUs utilized
>>>                 140        context-switches          #    0.017 K/sec
>>>                   9        cpu-migrations            #    0.001 K/sec
>>>                  66        page-faults               #    0.008 K/sec
>>>          10,671,929        cycles                    #    0.001 GHz
>>>           4,736,880        instructions              #    0.44  insn per cycle
>>>             942,951        branches                  #    0.118 M/sec
>>>              76,096        branch-misses             #    8.07% of all branches
>>>
>>>         1.001809960 seconds time elapsed
>>>
>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>> ---
>>>   tools/perf/Documentation/perf-stat.txt | 5 +++++
>>>   tools/perf/builtin-stat.c              | 4 +++-
>>>   tools/perf/util/stat.h                 | 1 +
>>>   3 files changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
>>> index 5d4a673d7621..75a83c2e4dc5 100644
>>> --- a/tools/perf/Documentation/perf-stat.txt
>>> +++ b/tools/perf/Documentation/perf-stat.txt
>>> @@ -438,6 +438,11 @@ convenient for post processing.
>>>   --summary::
>>>   Print summary for interval mode (-I).
>>> +--add-default::
>>> +The default event list includes the most common events which are widely
>>> +used by users. But with -e option, the perf only counts the events assigned
>>> +by -e option. This options appends the default event list to the -e events.
>>> +
>>>   EXAMPLES
>>>   --------
>>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>>> index 89c32692f40c..6ac7b946f9a7 100644
>>> --- a/tools/perf/builtin-stat.c
>>> +++ b/tools/perf/builtin-stat.c
>>> @@ -1173,6 +1173,8 @@ static struct option stat_options[] = {
>>>                  "print summary for interval mode"),
>>>       OPT_BOOLEAN(0, "quiet", &stat_config.quiet,
>>>               "don't print output (useful with record)"),
>>> +    OPT_BOOLEAN(0, "add-default", &stat_config.add_default,
>>> +               "add default events"),
>>>   #ifdef HAVE_LIBPFM
>>>       OPT_CALLBACK(0, "pfm-events", &evsel_list, "event",
>>>           "libpfm4 event selector. use 'perf list' to list available events",
>>> @@ -1755,7 +1757,7 @@ static int add_default_attributes(void)
>>>           free(str);
>>>       }
>>> -    if (!evsel_list->core.nr_entries) {
>>> +    if (!evsel_list->core.nr_entries || stat_config.add_default) {
>>>           if (target__has_cpu(&target))
>>>               default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
>>> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
>>> index 9979b4b100f2..6ccc6936348c 100644
>>> --- a/tools/perf/util/stat.h
>>> +++ b/tools/perf/util/stat.h
>>> @@ -123,6 +123,7 @@ struct perf_stat_config {
>>>       bool             metric_no_merge;
>>>       bool             stop_read_counter;
>>>       bool             quiet;
>>> +    bool             add_default;
>>>       FILE            *output;
>>>       unsigned int         interval;
>>>       unsigned int         timeout;
>>> -- 
>>> 2.17.1
>>>
>>

Re: [PATCH] perf stat: Create '--add-default' option to append default list

[Andi Kleen]

On Tue, Dec 22, 2020 at 09:11:31AM +0800, Jin Yao wrote:
> The event default list includes the most common events which are widely
> used by users. But with -e option, the current perf only counts the events
> assigned by -e option. Users may want to collect some extra events with
> the default list. For this case, users have to manually add all the events
> from the default list. It's inconvenient. Also, users may don't know how to
> get the default list.
> 
> It's better to add a new option to append default list to the -e events.
> The new option is '--add-default'.

A more concise syntax would be -e +event

The + would apply to the whole event list if there are multiple.

and maybe -event too to remove something from the default list.

-Andi

Re: [PATCH] perf stat: Create '--add-default' option to append default list

[Andi Kleen]

> A more concise syntax would be -e +event
> 
> The + would apply to the whole event list if there are multiple.
> 
> and maybe -event too to remove something from the default list.

Sorry that was an old email. Please ignore.

-Andi