linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf annotate-data: Support --skip-empty option
@ 2024-08-07  6:17 Namhyung Kim
  2024-08-09 21:33 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Namhyung Kim @ 2024-08-07  6:17 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

The --skip-empty option is to hide dummy events in a group.  Like other
output mode like perf report and perf annotate, the data-type profiling
output should support the option.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate-data.c | 30 ++++++++++++++----
 tools/perf/util/annotate-data.c        | 44 +++++++++++++-------------
 2 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
index 8d6bf08d371d..c3db80a7589a 100644
--- a/tools/perf/ui/browsers/annotate-data.c
+++ b/tools/perf/ui/browsers/annotate-data.c
@@ -46,15 +46,18 @@ static int get_member_overhead(struct annotated_data_type *adt,
 	struct annotated_member *member = entry->data;
 	int i, k;
 
-	for (i = 0; i < member->size; i++) {
+	for (i = 0, k = 0; i < member->size; i++) {
 		struct type_hist *h;
 		struct evsel *evsel;
 		int offset = member->offset + i;
 
 		for_each_group_evsel(evsel, leader) {
+			if (symbol_conf.skip_empty &&
+			    evsel__hists(evsel)->stats.nr_samples == 0)
+				continue;
+
 			h = adt->histograms[evsel->core.idx];
-			k = evsel__group_idx(evsel);
-			update_hist_entry(&entry->hists[k], &h->addr[offset]);
+			update_hist_entry(&entry->hists[k++], &h->addr[offset]);
 		}
 	}
 	return 0;
@@ -203,6 +206,7 @@ static void browser__write(struct ui_browser *uib, void *entry, int row)
 	struct annotated_data_type *adt = he->mem_type;
 	struct evsel *leader = hists_to_evsel(he->hists);
 	struct evsel *evsel;
+	int idx = 0;
 
 	if (member == NULL) {
 		bool current = ui_browser__is_current_entry(uib, row);
@@ -219,9 +223,12 @@ static void browser__write(struct ui_browser *uib, void *entry, int row)
 	/* print the number */
 	for_each_group_evsel(evsel, leader) {
 		struct type_hist *h = adt->histograms[evsel->core.idx];
-		int idx = evsel__group_idx(evsel);
 
-		browser__write_overhead(uib, h, &be->hists[idx], row);
+		if (symbol_conf.skip_empty &&
+		    evsel__hists(evsel)->stats.nr_samples == 0)
+			continue;
+
+		browser__write_overhead(uib, h, &be->hists[idx++], row);
 	}
 
 	/* print type info */
@@ -300,8 +307,17 @@ int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
 
 	ui_helpline__push("Press ESC to exit");
 
-	if (evsel__is_group_event(evsel))
-		browser.nr_events = evsel->core.nr_members;
+	if (evsel__is_group_event(evsel)) {
+		struct evsel *pos;
+		int nr = 0;
+
+		for_each_group_evsel(pos, evsel) {
+			if (!symbol_conf.skip_empty ||
+			    evsel__hists(pos)->stats.nr_samples)
+				nr++;
+		}
+		browser.nr_events = nr;
+	}
 
 	ret = annotated_data_browser__collect_entries(&browser);
 	if (ret == 0)
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 734acdd8c4b7..be3b84a82271 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1492,10 +1492,15 @@ static void print_annotated_data_header(struct hist_entry *he, struct evsel *evs
 		struct evsel *pos;
 		int i = 0;
 
-		for_each_group_evsel(pos, evsel)
-			printf(" event[%d] = %s\n", i++, pos->name);
+		nr_members = 0;
+		for_each_group_evsel(pos, evsel) {
+			if (symbol_conf.skip_empty &&
+			    evsel__hists(pos)->stats.nr_samples == 0)
+				continue;
 
-		nr_members = evsel->core.nr_members;
+			printf(" event[%d] = %s\n", i++, pos->name);
+			nr_members++;
+		}
 	}
 
 	if (symbol_conf.show_total_period) {
@@ -1530,31 +1535,26 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
 {
 	struct annotated_member *child;
 	struct type_hist *h = mem_type->histograms[evsel->core.idx];
-	int i, nr_events = 1, samples = 0;
+	int i, nr_events = 0, samples = 0;
 	u64 period = 0;
 	int width = symbol_conf.show_total_period ? 11 : 7;
+	struct evsel *pos;
 
-	for (i = 0; i < member->size; i++) {
-		samples += h->addr[member->offset + i].nr_samples;
-		period += h->addr[member->offset + i].period;
-	}
-	print_annotated_data_value(h, period, samples);
-
-	if (evsel__is_group_event(evsel)) {
-		struct evsel *pos;
+	for_each_group_evsel(pos, evsel) {
+		h = mem_type->histograms[pos->core.idx];
 
-		for_each_group_member(pos, evsel) {
-			h = mem_type->histograms[pos->core.idx];
+		if (symbol_conf.skip_empty &&
+		    evsel__hists(pos)->stats.nr_samples == 0)
+			continue;
 
-			samples = 0;
-			period = 0;
-			for (i = 0; i < member->size; i++) {
-				samples += h->addr[member->offset + i].nr_samples;
-				period += h->addr[member->offset + i].period;
-			}
-			print_annotated_data_value(h, period, samples);
+		samples = 0;
+		period = 0;
+		for (i = 0; i < member->size; i++) {
+			samples += h->addr[member->offset + i].nr_samples;
+			period += h->addr[member->offset + i].period;
 		}
-		nr_events = evsel->core.nr_members;
+		print_annotated_data_value(h, period, samples);
+		nr_events++;
 	}
 
 	printf(" %10d %10d  %*s%s\t%s",
-- 
2.46.0.rc2.264.g509ed76dc8-goog


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

* Re: [PATCH] perf annotate-data: Support --skip-empty option
  2024-08-07  6:17 [PATCH] perf annotate-data: Support --skip-empty option Namhyung Kim
@ 2024-08-09 21:33 ` Arnaldo Carvalho de Melo
  2024-08-10  4:15   ` Namhyung Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-08-09 21:33 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

On Tue, Aug 06, 2024 at 11:17:13PM -0700, Namhyung Kim wrote:
> The --skip-empty option is to hide dummy events in a group.  Like other
> output mode like perf report and perf annotate, the data-type profiling
> output should support the option.

Thanks, tested and applied.

    Committer testing:
    
    With dummy:
    
      root@number:~# perf annotate --stdio --group --data-type --skip-empty | head -24
      Annotate type: 'pthread_mutex_t' in /usr/lib64/libc.so.6 (50 samples):
       event[0] = cpu_atom/mem-loads,ldlat=30/P
       event[1] = cpu_atom/mem-stores/P
       event[2] = dummy:u
      ============================================================================
                       Percent     offset       size  field
        100.00  100.00    0.00          0         40  pthread_mutex_t        {
        100.00  100.00    0.00          0         40      struct __pthread_mutex_s  __data {
         45.21   84.54    0.00          0          4          int   __lock;
          0.00    0.00    0.00          4          4          unsigned int  __count;
          0.00    1.83    0.00          8          4          int   __owner;
          5.19   10.65    0.00         12          4          unsigned int  __nusers;
         49.61    2.97    0.00         16          4          int   __kind;
          0.00    0.00    0.00         20          2          short int     __spins;
          0.00    0.00    0.00         22          2          short int     __elision;
          0.00    0.00    0.00         24         16          __pthread_list_t      __list {
          0.00    0.00    0.00         24          8              struct __pthread_internal_list*   __prev;
          0.00    0.00    0.00         32          8              struct __pthread_internal_list*   __next;
                                                              };
                                                          };
          0.00    0.00    0.00          0          0      char[]    __size;
         45.21   84.54    0.00          0          8      long int  __align;
                                                    };
    Skipping it:
    
      root@number:~# perf annotate --stdio --group --data-type --skip-empty | head -24
      Annotate type: 'pthread_mutex_t' in /usr/lib64/libc.so.6 (50 samples):
       event[0] = cpu_atom/mem-loads,ldlat=30/P
       event[1] = cpu_atom/mem-stores/P
      ============================================================================
               Percent     offset       size  field
        100.00  100.00          0         40  pthread_mutex_t        {
        100.00  100.00          0         40      struct __pthread_mutex_s  __data {
         45.21   84.54          0          4          int   __lock;
          0.00    0.00          4          4          unsigned int  __count;
          0.00    1.83          8          4          int   __owner;
          5.19   10.65         12          4          unsigned int  __nusers;
         49.61    2.97         16          4          int   __kind;
          0.00    0.00         20          2          short int     __spins;
          0.00    0.00         22          2          short int     __elision;
          0.00    0.00         24         16          __pthread_list_t      __list {
          0.00    0.00         24          8              struct __pthread_internal_list*   __prev;
          0.00    0.00         32          8              struct __pthread_internal_list*   __next;
                                                      };
                                                  };
          0.00    0.00          0          0      char[]    __size;
         45.21   84.54          0          8      long int  __align;
                                              };
    
      Annotate type: 'pthread_mutexattr_t' in /usr/lib64/libc.so.6 (1 samples):
      root@number:~#

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/annotate-data.c | 30 ++++++++++++++----
>  tools/perf/util/annotate-data.c        | 44 +++++++++++++-------------
>  2 files changed, 45 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
> index 8d6bf08d371d..c3db80a7589a 100644
> --- a/tools/perf/ui/browsers/annotate-data.c
> +++ b/tools/perf/ui/browsers/annotate-data.c
> @@ -46,15 +46,18 @@ static int get_member_overhead(struct annotated_data_type *adt,
>  	struct annotated_member *member = entry->data;
>  	int i, k;
>  
> -	for (i = 0; i < member->size; i++) {
> +	for (i = 0, k = 0; i < member->size; i++) {
>  		struct type_hist *h;
>  		struct evsel *evsel;
>  		int offset = member->offset + i;
>  
>  		for_each_group_evsel(evsel, leader) {
> +			if (symbol_conf.skip_empty &&
> +			    evsel__hists(evsel)->stats.nr_samples == 0)
> +				continue;
> +
>  			h = adt->histograms[evsel->core.idx];
> -			k = evsel__group_idx(evsel);
> -			update_hist_entry(&entry->hists[k], &h->addr[offset]);
> +			update_hist_entry(&entry->hists[k++], &h->addr[offset]);
>  		}
>  	}
>  	return 0;
> @@ -203,6 +206,7 @@ static void browser__write(struct ui_browser *uib, void *entry, int row)
>  	struct annotated_data_type *adt = he->mem_type;
>  	struct evsel *leader = hists_to_evsel(he->hists);
>  	struct evsel *evsel;
> +	int idx = 0;
>  
>  	if (member == NULL) {
>  		bool current = ui_browser__is_current_entry(uib, row);
> @@ -219,9 +223,12 @@ static void browser__write(struct ui_browser *uib, void *entry, int row)
>  	/* print the number */
>  	for_each_group_evsel(evsel, leader) {
>  		struct type_hist *h = adt->histograms[evsel->core.idx];
> -		int idx = evsel__group_idx(evsel);
>  
> -		browser__write_overhead(uib, h, &be->hists[idx], row);
> +		if (symbol_conf.skip_empty &&
> +		    evsel__hists(evsel)->stats.nr_samples == 0)
> +			continue;
> +
> +		browser__write_overhead(uib, h, &be->hists[idx++], row);
>  	}
>  
>  	/* print type info */
> @@ -300,8 +307,17 @@ int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
>  
>  	ui_helpline__push("Press ESC to exit");
>  
> -	if (evsel__is_group_event(evsel))
> -		browser.nr_events = evsel->core.nr_members;
> +	if (evsel__is_group_event(evsel)) {
> +		struct evsel *pos;
> +		int nr = 0;
> +
> +		for_each_group_evsel(pos, evsel) {
> +			if (!symbol_conf.skip_empty ||
> +			    evsel__hists(pos)->stats.nr_samples)
> +				nr++;
> +		}
> +		browser.nr_events = nr;
> +	}
>  
>  	ret = annotated_data_browser__collect_entries(&browser);
>  	if (ret == 0)
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 734acdd8c4b7..be3b84a82271 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1492,10 +1492,15 @@ static void print_annotated_data_header(struct hist_entry *he, struct evsel *evs
>  		struct evsel *pos;
>  		int i = 0;
>  
> -		for_each_group_evsel(pos, evsel)
> -			printf(" event[%d] = %s\n", i++, pos->name);
> +		nr_members = 0;
> +		for_each_group_evsel(pos, evsel) {
> +			if (symbol_conf.skip_empty &&
> +			    evsel__hists(pos)->stats.nr_samples == 0)
> +				continue;
>  
> -		nr_members = evsel->core.nr_members;
> +			printf(" event[%d] = %s\n", i++, pos->name);
> +			nr_members++;
> +		}
>  	}
>  
>  	if (symbol_conf.show_total_period) {
> @@ -1530,31 +1535,26 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
>  {
>  	struct annotated_member *child;
>  	struct type_hist *h = mem_type->histograms[evsel->core.idx];
> -	int i, nr_events = 1, samples = 0;
> +	int i, nr_events = 0, samples = 0;
>  	u64 period = 0;
>  	int width = symbol_conf.show_total_period ? 11 : 7;
> +	struct evsel *pos;
>  
> -	for (i = 0; i < member->size; i++) {
> -		samples += h->addr[member->offset + i].nr_samples;
> -		period += h->addr[member->offset + i].period;
> -	}
> -	print_annotated_data_value(h, period, samples);
> -
> -	if (evsel__is_group_event(evsel)) {
> -		struct evsel *pos;
> +	for_each_group_evsel(pos, evsel) {
> +		h = mem_type->histograms[pos->core.idx];
>  
> -		for_each_group_member(pos, evsel) {
> -			h = mem_type->histograms[pos->core.idx];
> +		if (symbol_conf.skip_empty &&
> +		    evsel__hists(pos)->stats.nr_samples == 0)
> +			continue;
>  
> -			samples = 0;
> -			period = 0;
> -			for (i = 0; i < member->size; i++) {
> -				samples += h->addr[member->offset + i].nr_samples;
> -				period += h->addr[member->offset + i].period;
> -			}
> -			print_annotated_data_value(h, period, samples);
> +		samples = 0;
> +		period = 0;
> +		for (i = 0; i < member->size; i++) {
> +			samples += h->addr[member->offset + i].nr_samples;
> +			period += h->addr[member->offset + i].period;
>  		}
> -		nr_events = evsel->core.nr_members;
> +		print_annotated_data_value(h, period, samples);
> +		nr_events++;
>  	}
>  
>  	printf(" %10d %10d  %*s%s\t%s",
> -- 
> 2.46.0.rc2.264.g509ed76dc8-goog

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

* Re: [PATCH] perf annotate-data: Support --skip-empty option
  2024-08-09 21:33 ` Arnaldo Carvalho de Melo
@ 2024-08-10  4:15   ` Namhyung Kim
  0 siblings, 0 replies; 3+ messages in thread
From: Namhyung Kim @ 2024-08-10  4:15 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

On Fri, Aug 09, 2024 at 06:33:09PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Aug 06, 2024 at 11:17:13PM -0700, Namhyung Kim wrote:
> > The --skip-empty option is to hide dummy events in a group.  Like other
> > output mode like perf report and perf annotate, the data-type profiling
> > output should support the option.
> 
> Thanks, tested and applied.

Thanks, but I found a bug when a member has multiple histogram entries.
Could you please fold this?  Or I can send a format patch if you want.

Thanks,
Namhyung


diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
index c3db80a7589a..a937b55da736 100644
--- a/tools/perf/ui/browsers/annotate-data.c
+++ b/tools/perf/ui/browsers/annotate-data.c
@@ -46,11 +46,12 @@ static int get_member_overhead(struct annotated_data_type *adt,
 	struct annotated_member *member = entry->data;
 	int i, k;
 
-	for (i = 0, k = 0; i < member->size; i++) {
+	for (i = 0; i < member->size; i++) {
 		struct type_hist *h;
 		struct evsel *evsel;
 		int offset = member->offset + i;
 
+		k = 0;
 		for_each_group_evsel(evsel, leader) {
 			if (symbol_conf.skip_empty &&
 			    evsel__hists(evsel)->stats.nr_samples == 0)

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

end of thread, other threads:[~2024-08-10  4:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07  6:17 [PATCH] perf annotate-data: Support --skip-empty option Namhyung Kim
2024-08-09 21:33 ` Arnaldo Carvalho de Melo
2024-08-10  4:15   ` Namhyung Kim

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).