Re: [PATCH 2/4] tools/perf: Add dynamic headers for perf report columns

2021-03-15 Thread Athira Rajeev



> On 12-Mar-2021, at 6:27 PM, Jiri Olsa  wrote:
> 
> On Tue, Mar 09, 2021 at 09:03:58AM -0500, Athira Rajeev wrote:
>> Currently the header string for different columns in perf report
>> is fixed. Some fields of perf sample could have different meaning
>> for different architectures than the meaning conveyed by the header
>> string. An example is the new field 'var2_w' of perf_sample_weight
>> structure. This is presently captured as 'Local INSTR Latency' in
>> perf mem report. But this could be used to denote a different latency
>> cycle in another architecture.
>> 
>> Introduce a weak function arch_perf_header_entry__add() to set
>> the arch specific header string for the fields which can contain dynamic
>> header. If the architecture do not have this function, fall back to the
>> default header string value.
>> 
>> Signed-off-by: Athira Rajeev 
>> ---
>> tools/perf/util/event.h |  1 +
>> tools/perf/util/sort.c  | 19 ++-
>> 2 files changed, 19 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
>> index f603edbbbc6f..89b149e2e70a 100644
>> --- a/tools/perf/util/event.h
>> +++ b/tools/perf/util/event.h
>> @@ -427,5 +427,6 @@ void  cpu_map_data__synthesize(struct 
>> perf_record_cpu_map_data *data, struct per
>> 
>> void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 
>> *array, u64 type);
>> void arch_perf_synthesize_sample_weight(const struct perf_sample *data, 
>> __u64 *array, u64 type);
>> +const char *arch_perf_header_entry__add(const char *se_header);
>> 
>> #endif /* __PERF_RECORD_H */
>> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> index 0d5ad42812b9..741a6df29fa0 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -25,6 +25,7 @@
>> #include 
>> #include "mem-events.h"
>> #include "annotate.h"
>> +#include "event.h"
>> #include "time-utils.h"
>> #include "cgroup.h"
>> #include "machine.h"
>> @@ -45,6 +46,7 @@
>> regex_t  ignore_callees_regex;
>> int  have_ignore_callees = 0;
>> enum sort_mode   sort__mode = SORT_MODE__NORMAL;
>> +const char  *dynamic_headers[] = {"local_ins_lat"};
>> 
>> /*
>>  * Replaces all occurrences of a char used with the:
>> @@ -1816,6 +1818,16 @@ struct sort_dimension {
>>  int taken;
>> };
>> 
>> +const char * __weak arch_perf_header_entry__add(const char *se_header)
> 
> no need for the __add suffix in here
> 
> jirka
> 

Thanks Jiri for the review.

I will include this change in next version.

Thanks
Athira

>> +{
>> +return se_header;
>> +}
>> +
>> +static void sort_dimension_add_dynamic_header(struct sort_dimension *sd)
>> +{
>> +sd->entry->se_header = 
>> arch_perf_header_entry__add(sd->entry->se_header);
>> +}
>> +
>> #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
>> 
>> static struct sort_dimension common_sort_dimensions[] = {
>> @@ -2739,11 +2751,16 @@ int sort_dimension__add(struct perf_hpp_list *list, 
>> const char *tok,
>>  struct evlist *evlist,
>>  int level)
>> {
>> -unsigned int i;
>> +unsigned int i, j;
>> 
>>  for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
>>  struct sort_dimension *sd = _sort_dimensions[i];
>> 
>> +for (j = 0; j < ARRAY_SIZE(dynamic_headers); j++) {
>> +if (!strcmp(dynamic_headers[j], sd->name))
>> +sort_dimension_add_dynamic_header(sd);
>> +}
>> +
>>  if (strncasecmp(tok, sd->name, strlen(tok)))
>>  continue;
>> 
>> -- 
>> 1.8.3.1



Re: [PATCH 2/4] tools/perf: Add dynamic headers for perf report columns

2021-03-12 Thread Jiri Olsa
On Tue, Mar 09, 2021 at 09:03:58AM -0500, Athira Rajeev wrote:
> Currently the header string for different columns in perf report
> is fixed. Some fields of perf sample could have different meaning
> for different architectures than the meaning conveyed by the header
> string. An example is the new field 'var2_w' of perf_sample_weight
> structure. This is presently captured as 'Local INSTR Latency' in
> perf mem report. But this could be used to denote a different latency
> cycle in another architecture.
> 
> Introduce a weak function arch_perf_header_entry__add() to set
> the arch specific header string for the fields which can contain dynamic
> header. If the architecture do not have this function, fall back to the
> default header string value.
> 
> Signed-off-by: Athira Rajeev 
> ---
>  tools/perf/util/event.h |  1 +
>  tools/perf/util/sort.c  | 19 ++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index f603edbbbc6f..89b149e2e70a 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -427,5 +427,6 @@ void  cpu_map_data__synthesize(struct 
> perf_record_cpu_map_data *data, struct per
>  
>  void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 
> *array, u64 type);
>  void arch_perf_synthesize_sample_weight(const struct perf_sample *data, 
> __u64 *array, u64 type);
> +const char *arch_perf_header_entry__add(const char *se_header);
>  
>  #endif /* __PERF_RECORD_H */
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 0d5ad42812b9..741a6df29fa0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include "mem-events.h"
>  #include "annotate.h"
> +#include "event.h"
>  #include "time-utils.h"
>  #include "cgroup.h"
>  #include "machine.h"
> @@ -45,6 +46,7 @@
>  regex_t  ignore_callees_regex;
>  int  have_ignore_callees = 0;
>  enum sort_mode   sort__mode = SORT_MODE__NORMAL;
> +const char   *dynamic_headers[] = {"local_ins_lat"};
>  
>  /*
>   * Replaces all occurrences of a char used with the:
> @@ -1816,6 +1818,16 @@ struct sort_dimension {
>   int taken;
>  };
>  
> +const char * __weak arch_perf_header_entry__add(const char *se_header)

no need for the __add suffix in here

jirka

> +{
> + return se_header;
> +}
> +
> +static void sort_dimension_add_dynamic_header(struct sort_dimension *sd)
> +{
> + sd->entry->se_header = 
> arch_perf_header_entry__add(sd->entry->se_header);
> +}
> +
>  #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
>  
>  static struct sort_dimension common_sort_dimensions[] = {
> @@ -2739,11 +2751,16 @@ int sort_dimension__add(struct perf_hpp_list *list, 
> const char *tok,
>   struct evlist *evlist,
>   int level)
>  {
> - unsigned int i;
> + unsigned int i, j;
>  
>   for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
>   struct sort_dimension *sd = _sort_dimensions[i];
>  
> + for (j = 0; j < ARRAY_SIZE(dynamic_headers); j++) {
> + if (!strcmp(dynamic_headers[j], sd->name))
> + sort_dimension_add_dynamic_header(sd);
> + }
> +
>   if (strncasecmp(tok, sd->name, strlen(tok)))
>   continue;
>  
> -- 
> 1.8.3.1
> 



[PATCH 2/4] tools/perf: Add dynamic headers for perf report columns

2021-03-09 Thread Athira Rajeev
Currently the header string for different columns in perf report
is fixed. Some fields of perf sample could have different meaning
for different architectures than the meaning conveyed by the header
string. An example is the new field 'var2_w' of perf_sample_weight
structure. This is presently captured as 'Local INSTR Latency' in
perf mem report. But this could be used to denote a different latency
cycle in another architecture.

Introduce a weak function arch_perf_header_entry__add() to set
the arch specific header string for the fields which can contain dynamic
header. If the architecture do not have this function, fall back to the
default header string value.

Signed-off-by: Athira Rajeev 
---
 tools/perf/util/event.h |  1 +
 tools/perf/util/sort.c  | 19 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index f603edbbbc6f..89b149e2e70a 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -427,5 +427,6 @@ void  cpu_map_data__synthesize(struct 
perf_record_cpu_map_data *data, struct per
 
 void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 
*array, u64 type);
 void arch_perf_synthesize_sample_weight(const struct perf_sample *data, __u64 
*array, u64 type);
+const char *arch_perf_header_entry__add(const char *se_header);
 
 #endif /* __PERF_RECORD_H */
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 0d5ad42812b9..741a6df29fa0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -25,6 +25,7 @@
 #include 
 #include "mem-events.h"
 #include "annotate.h"
+#include "event.h"
 #include "time-utils.h"
 #include "cgroup.h"
 #include "machine.h"
@@ -45,6 +46,7 @@
 regex_tignore_callees_regex;
 inthave_ignore_callees = 0;
 enum sort_mode sort__mode = SORT_MODE__NORMAL;
+const char *dynamic_headers[] = {"local_ins_lat"};
 
 /*
  * Replaces all occurrences of a char used with the:
@@ -1816,6 +1818,16 @@ struct sort_dimension {
int taken;
 };
 
+const char * __weak arch_perf_header_entry__add(const char *se_header)
+{
+   return se_header;
+}
+
+static void sort_dimension_add_dynamic_header(struct sort_dimension *sd)
+{
+   sd->entry->se_header = 
arch_perf_header_entry__add(sd->entry->se_header);
+}
+
 #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
 
 static struct sort_dimension common_sort_dimensions[] = {
@@ -2739,11 +2751,16 @@ int sort_dimension__add(struct perf_hpp_list *list, 
const char *tok,
struct evlist *evlist,
int level)
 {
-   unsigned int i;
+   unsigned int i, j;
 
for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
struct sort_dimension *sd = _sort_dimensions[i];
 
+   for (j = 0; j < ARRAY_SIZE(dynamic_headers); j++) {
+   if (!strcmp(dynamic_headers[j], sd->name))
+   sort_dimension_add_dynamic_header(sd);
+   }
+
if (strncasecmp(tok, sd->name, strlen(tok)))
continue;
 
-- 
1.8.3.1