linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2)
@ 2024-04-11  3:32 Namhyung Kim
  2024-04-11  3:32 ` [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary Namhyung Kim
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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

Hello,

This is to support interactive TUI browser for type annotation.

v2 changes:
 * fix build errors when libslang2 or libdw is missing  (Arnaldo)
 * update commit messages with examples  (Arnaldo)
 * skip updating sample histogram for stack canary  (Arnaldo)
 * add Reviewed-by from Ian
 
Like the normal (code) annotation, it should be able to display the data type
annotation.  Now `perf annotate --data-type` will show the result in TUI by
default if it's enabled.  Also `perf report -s type` can show the same output
using a menu item.

It's still in a very early stage and supports the basic functionalities only.
I'll work on more features like in the normal annotation browser later.

The code is also available at 'perf/annotate-data-tui-v2' branch at

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (7):
  perf annotate-data: Skip sample histogram for stack canary
  perf annotate: Show progress of sample processing
  perf annotate-data: Add hist_entry__annotate_data_tty()
  perf annotate-data: Add hist_entry__annotate_data_tui()
  perf annotate-data: Support event group display in TUI
  perf report: Add a menu item to annotate data type in TUI
  perf report: Do not collect sample histogram unnecessarily

 tools/perf/builtin-annotate.c          | 149 ++++--------
 tools/perf/builtin-report.c            |   7 +-
 tools/perf/ui/browsers/Build           |   1 +
 tools/perf/ui/browsers/annotate-data.c | 312 +++++++++++++++++++++++++
 tools/perf/ui/browsers/hists.c         |  31 +++
 tools/perf/util/annotate-data.c        | 113 +++++++++
 tools/perf/util/annotate-data.h        |  22 ++
 tools/perf/util/annotate.c             |  12 +-
 8 files changed, 534 insertions(+), 113 deletions(-)
 create mode 100644 tools/perf/ui/browsers/annotate-data.c


base-commit: 9c3e9af74326978ba6f4432bb038e6c80f4f56fd
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 2/7] perf annotate: Show progress of sample processing Namhyung Kim
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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

It's a pseudo data type and has no field.

Fixes: b3c95109c131 ("perf annotate-data: Add stack canary type")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 11da27801d88..ec79c120a7d2 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2399,8 +2399,9 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
 		mem_type = find_data_type(&dloc);
 
 		if (mem_type == NULL && is_stack_canary(arch, op_loc)) {
-			mem_type = &canary_type;
-			dloc.type_offset = 0;
+			istat->good++;
+			he->mem_type_off = 0;
+			return &canary_type;
 		}
 
 		if (mem_type)
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 2/7] perf annotate: Show progress of sample processing
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
  2024-04-11  3:32 ` [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 3/7] perf annotate-data: Add hist_entry__annotate_data_tty() Namhyung Kim
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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

Like 'perf report', it can take a while to process samples.

Show a progress window to inform users how that it is not stuck.

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 16e1581207c9..332e1ddcacbd 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -37,6 +37,7 @@
 #include "util/map_symbol.h"
 #include "util/branch.h"
 #include "util/util.h"
+#include "ui/progress.h"
 
 #include <dlfcn.h>
 #include <errno.h>
@@ -665,13 +666,23 @@ static int __cmd_annotate(struct perf_annotate *ann)
 	evlist__for_each_entry(session->evlist, pos) {
 		struct hists *hists = evsel__hists(pos);
 		u32 nr_samples = hists->stats.nr_samples;
+		struct ui_progress prog;
 
 		if (nr_samples > 0) {
 			total_nr_samples += nr_samples;
-			hists__collapse_resort(hists, NULL);
+
+			ui_progress__init(&prog, nr_samples,
+					  "Merging related events...");
+			hists__collapse_resort(hists, &prog);
+			ui_progress__finish();
+
 			/* Don't sort callchain */
 			evsel__reset_sample_bit(pos, CALLCHAIN);
-			evsel__output_resort(pos, NULL);
+
+			ui_progress__init(&prog, nr_samples,
+					  "Sorting events for output...");
+			evsel__output_resort(pos, &prog);
+			ui_progress__finish();
 
 			/*
 			 * An event group needs to display other events too.
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 3/7] perf annotate-data: Add hist_entry__annotate_data_tty()
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
  2024-04-11  3:32 ` [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary Namhyung Kim
  2024-04-11  3:32 ` [PATCH 2/7] perf annotate: Show progress of sample processing Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 4/7] perf annotate-data: Add hist_entry__annotate_data_tui() Namhyung Kim
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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

And move the related code into util/annotate-data.c file.

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c   | 106 +-----------------------------
 tools/perf/util/annotate-data.c | 112 ++++++++++++++++++++++++++++++++
 tools/perf/util/annotate-data.h |   9 +++
 3 files changed, 122 insertions(+), 105 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 332e1ddcacbd..0812664faa54 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -329,108 +329,6 @@ static int hist_entry__tty_annotate(struct hist_entry *he,
 	return symbol__tty_annotate2(&he->ms, evsel);
 }
 
-static void print_annotated_data_header(struct hist_entry *he, struct evsel *evsel)
-{
-	struct dso *dso = map__dso(he->ms.map);
-	int nr_members = 1;
-	int nr_samples = he->stat.nr_events;
-	int width = 7;
-	const char *val_hdr = "Percent";
-
-	if (evsel__is_group_event(evsel)) {
-		struct hist_entry *pair;
-
-		list_for_each_entry(pair, &he->pairs.head, pairs.node)
-			nr_samples += pair->stat.nr_events;
-	}
-
-	printf("Annotate type: '%s' in %s (%d samples):\n",
-	       he->mem_type->self.type_name, dso->name, nr_samples);
-
-	if (evsel__is_group_event(evsel)) {
-		struct evsel *pos;
-		int i = 0;
-
-		for_each_group_evsel(pos, evsel)
-			printf(" event[%d] = %s\n", i++, pos->name);
-
-		nr_members = evsel->core.nr_members;
-	}
-
-	if (symbol_conf.show_total_period) {
-		width = 11;
-		val_hdr = "Period";
-	} else if (symbol_conf.show_nr_samples) {
-		width = 7;
-		val_hdr = "Samples";
-	}
-
-	printf("============================================================================\n");
-	printf("%*s %10s %10s  %s\n", (width + 1) * nr_members, val_hdr,
-	       "offset", "size", "field");
-}
-
-static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_samples)
-{
-	double percent = h->period ? (100.0 * period / h->period) : 0;
-	const char *color = get_percent_color(percent);
-
-	if (symbol_conf.show_total_period)
-		color_fprintf(stdout, color, " %11" PRIu64, period);
-	else if (symbol_conf.show_nr_samples)
-		color_fprintf(stdout, color, " %7d", nr_samples);
-	else
-		color_fprintf(stdout, color, " %7.2f", percent);
-}
-
-static void print_annotated_data_type(struct annotated_data_type *mem_type,
-				      struct annotated_member *member,
-				      struct evsel *evsel, int indent)
-{
-	struct annotated_member *child;
-	struct type_hist *h = mem_type->histograms[evsel->core.idx];
-	int i, nr_events = 1, samples = 0;
-	u64 period = 0;
-	int width = symbol_conf.show_total_period ? 11 : 7;
-
-	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_member(pos, evsel) {
-			h = mem_type->histograms[pos->core.idx];
-
-			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);
-		}
-		nr_events = evsel->core.nr_members;
-	}
-
-	printf(" %10d %10d  %*s%s\t%s",
-	       member->offset, member->size, indent, "", member->type_name,
-	       member->var_name ?: "");
-
-	if (!list_empty(&member->children))
-		printf(" {\n");
-
-	list_for_each_entry(child, &member->children, node)
-		print_annotated_data_type(mem_type, child, evsel, indent + 4);
-
-	if (!list_empty(&member->children))
-		printf("%*s}", (width + 1) * nr_events + 24 + indent, "");
-	printf(";\n");
-}
-
 static void print_annotate_data_stat(struct annotated_data_stat *s)
 {
 #define PRINT_STAT(fld) if (s->fld) printf("%10d : %s\n", s->fld, #fld)
@@ -571,9 +469,7 @@ static void hists__find_annotations(struct hists *hists,
 					goto find_next;
 			}
 
-			print_annotated_data_header(he, evsel);
-			print_annotated_data_type(he->mem_type, &he->mem_type->self, evsel, 0);
-			printf("\n");
+			hist_entry__annotate_data_tty(he, evsel);
 			goto find_next;
 		}
 
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index b69a1cd1577a..b150137a92dc 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -19,6 +19,7 @@
 #include "evlist.h"
 #include "map.h"
 #include "map_symbol.h"
+#include "sort.h"
 #include "strbuf.h"
 #include "symbol.h"
 #include "symbol_conf.h"
@@ -1710,3 +1711,114 @@ int annotated_data_type__update_samples(struct annotated_data_type *adt,
 	h->addr[offset].period += period;
 	return 0;
 }
+
+static void print_annotated_data_header(struct hist_entry *he, struct evsel *evsel)
+{
+	struct dso *dso = map__dso(he->ms.map);
+	int nr_members = 1;
+	int nr_samples = he->stat.nr_events;
+	int width = 7;
+	const char *val_hdr = "Percent";
+
+	if (evsel__is_group_event(evsel)) {
+		struct hist_entry *pair;
+
+		list_for_each_entry(pair, &he->pairs.head, pairs.node)
+			nr_samples += pair->stat.nr_events;
+	}
+
+	printf("Annotate type: '%s' in %s (%d samples):\n",
+	       he->mem_type->self.type_name, dso->name, nr_samples);
+
+	if (evsel__is_group_event(evsel)) {
+		struct evsel *pos;
+		int i = 0;
+
+		for_each_group_evsel(pos, evsel)
+			printf(" event[%d] = %s\n", i++, pos->name);
+
+		nr_members = evsel->core.nr_members;
+	}
+
+	if (symbol_conf.show_total_period) {
+		width = 11;
+		val_hdr = "Period";
+	} else if (symbol_conf.show_nr_samples) {
+		width = 7;
+		val_hdr = "Samples";
+	}
+
+	printf("============================================================================\n");
+	printf("%*s %10s %10s  %s\n", (width + 1) * nr_members, val_hdr,
+	       "offset", "size", "field");
+}
+
+static void print_annotated_data_value(struct type_hist *h, u64 period, int nr_samples)
+{
+	double percent = h->period ? (100.0 * period / h->period) : 0;
+	const char *color = get_percent_color(percent);
+
+	if (symbol_conf.show_total_period)
+		color_fprintf(stdout, color, " %11" PRIu64, period);
+	else if (symbol_conf.show_nr_samples)
+		color_fprintf(stdout, color, " %7d", nr_samples);
+	else
+		color_fprintf(stdout, color, " %7.2f", percent);
+}
+
+static void print_annotated_data_type(struct annotated_data_type *mem_type,
+				      struct annotated_member *member,
+				      struct evsel *evsel, int indent)
+{
+	struct annotated_member *child;
+	struct type_hist *h = mem_type->histograms[evsel->core.idx];
+	int i, nr_events = 1, samples = 0;
+	u64 period = 0;
+	int width = symbol_conf.show_total_period ? 11 : 7;
+
+	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_member(pos, evsel) {
+			h = mem_type->histograms[pos->core.idx];
+
+			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);
+		}
+		nr_events = evsel->core.nr_members;
+	}
+
+	printf(" %10d %10d  %*s%s\t%s",
+	       member->offset, member->size, indent, "", member->type_name,
+	       member->var_name ?: "");
+
+	if (!list_empty(&member->children))
+		printf(" {\n");
+
+	list_for_each_entry(child, &member->children, node)
+		print_annotated_data_type(mem_type, child, evsel, indent + 4);
+
+	if (!list_empty(&member->children))
+		printf("%*s}", (width + 1) * nr_events + 24 + indent, "");
+	printf(";\n");
+}
+
+int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel)
+{
+	print_annotated_data_header(he, evsel);
+	print_annotated_data_type(he->mem_type, &he->mem_type->self, evsel, 0);
+	printf("\n");
+
+	return 0;
+}
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index fe1e53d6e8c7..01489db267d4 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -10,6 +10,7 @@
 struct annotated_op_loc;
 struct debuginfo;
 struct evsel;
+struct hist_entry;
 struct map_symbol;
 struct thread;
 
@@ -156,6 +157,8 @@ void annotated_data_type__tree_delete(struct rb_root *root);
 /* Release all global variable information in the tree */
 void global_var_type__tree_delete(struct rb_root *root);
 
+int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel);
+
 #else /* HAVE_DWARF_SUPPORT */
 
 static inline struct annotated_data_type *
@@ -182,6 +185,12 @@ static inline void global_var_type__tree_delete(struct rb_root *root __maybe_unu
 {
 }
 
+static inline int hist_entry__annotate_data_tty(struct hist_entry *he __maybe_unused,
+						struct evsel *evsel __maybe_unused)
+{
+	return -1;
+}
+
 #endif /* HAVE_DWARF_SUPPORT */
 
 #endif /* _PERF_ANNOTATE_DATA_H */
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 4/7] perf annotate-data: Add hist_entry__annotate_data_tui()
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
                   ` (2 preceding siblings ...)
  2024-04-11  3:32 ` [PATCH 3/7] perf annotate-data: Add hist_entry__annotate_data_tty() Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 5/7] perf annotate-data: Support event group display in TUI Namhyung Kim
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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, Arnaldo Carvalho de Melo

Support data type profiling output on TUI.

Testing from Arnaldo:

First make sure that the debug information for your workload binaries
in embedded in them by building it with '-g' or install the debuginfo
packages, since our workload is 'find':

  root@number:~# type find
  find is hashed (/usr/bin/find)
  root@number:~# rpm -qf /usr/bin/find
  findutils-4.9.0-5.fc39.x86_64
  root@number:~# dnf debuginfo-install findutils
  <SNIP>
  root@number:~#

Then collect some data:

  root@number:~# echo 1 > /proc/sys/vm/drop_caches
  root@number:~# perf mem record find / > /dev/null
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.331 MB perf.data (3982 samples) ]
  root@number:~#

Finally do data-type annotation with the following command, that will
default, as 'perf report' to the --tui mode, with lines colored to
highlight the hotspots, etc.

  root@number:~# perf annotate --data-type
  Annotate type: 'struct predicate' (58 samples)
      Percent     Offset       Size  Field
       100.00          0        312  struct predicate {
         0.00          0          8      PRED_FUNC        pred_func;
         0.00          8          8      char*    p_name;
         0.00         16          4      enum predicate_type      p_type;
         0.00         20          4      enum predicate_precedence        p_prec;
         0.00         24          1      _Bool    side_effects;
         0.00         25          1      _Bool    no_default_print;
         0.00         26          1      _Bool    need_stat;
         0.00         27          1      _Bool    need_type;
         0.00         28          1      _Bool    need_inum;
         0.00         32          4      enum EvaluationCost      p_cost;
         0.00         36          4      float    est_success_rate;
         0.00         40          1      _Bool    literal_control_chars;
         0.00         41          1      _Bool    artificial;
         0.00         48          8      char*    arg_text;
  <SNIP>

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c          |  30 ++-
 tools/perf/ui/browsers/Build           |   1 +
 tools/perf/ui/browsers/annotate-data.c | 282 +++++++++++++++++++++++++
 tools/perf/util/annotate-data.c        |   3 +-
 tools/perf/util/annotate-data.h        |  13 ++
 5 files changed, 324 insertions(+), 5 deletions(-)
 create mode 100644 tools/perf/ui/browsers/annotate-data.c

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 0812664faa54..6f7104f06c42 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -469,8 +469,32 @@ static void hists__find_annotations(struct hists *hists,
 					goto find_next;
 			}
 
-			hist_entry__annotate_data_tty(he, evsel);
-			goto find_next;
+			if (use_browser == 1)
+				key = hist_entry__annotate_data_tui(he, evsel, NULL);
+			else
+				key = hist_entry__annotate_data_tty(he, evsel);
+
+			switch (key) {
+			case -1:
+				if (!ann->skip_missing)
+					return;
+				/* fall through */
+			case K_RIGHT:
+			case '>':
+				next = rb_next(nd);
+				break;
+			case K_LEFT:
+			case '<':
+				next = rb_prev(nd);
+				break;
+			default:
+				return;
+			}
+
+			if (next != NULL)
+				nd = next;
+
+			continue;
 		}
 
 		if (use_browser == 2) {
@@ -873,9 +897,7 @@ int cmd_annotate(int argc, const char **argv)
 		use_browser = 2;
 #endif
 
-	/* FIXME: only support stdio for now */
 	if (annotate.data_type) {
-		use_browser = 0;
 		annotate_opts.annotate_src = false;
 		symbol_conf.annotate_data_member = true;
 		symbol_conf.annotate_data_sample = true;
diff --git a/tools/perf/ui/browsers/Build b/tools/perf/ui/browsers/Build
index 7a1d5ddaf688..2608b5da3167 100644
--- a/tools/perf/ui/browsers/Build
+++ b/tools/perf/ui/browsers/Build
@@ -1,4 +1,5 @@
 perf-y += annotate.o
+perf-y += annotate-data.o
 perf-y += hists.o
 perf-y += map.o
 perf-y += scripts.o
diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
new file mode 100644
index 000000000000..fefacaaf16db
--- /dev/null
+++ b/tools/perf/ui/browsers/annotate-data.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <inttypes.h>
+#include <string.h>
+#include <sys/ttydefaults.h>
+
+#include "ui/browser.h"
+#include "ui/helpline.h"
+#include "ui/keysyms.h"
+#include "ui/ui.h"
+#include "util/annotate.h"
+#include "util/annotate-data.h"
+#include "util/evsel.h"
+#include "util/sort.h"
+
+struct annotated_data_browser {
+	struct ui_browser b;
+	struct list_head entries;
+};
+
+struct browser_entry {
+	struct list_head node;
+	struct annotated_member *data;
+	struct type_hist_entry hists;
+	int indent;
+};
+
+static void update_hist_entry(struct type_hist_entry *dst,
+			      struct type_hist_entry *src)
+{
+	dst->nr_samples += src->nr_samples;
+	dst->period += src->period;
+}
+
+static int get_member_overhead(struct annotated_data_type *adt,
+			       struct browser_entry *entry,
+			       struct evsel *evsel)
+{
+	struct annotated_member *member = entry->data;
+	int i;
+
+	for (i = 0; i < member->size; i++) {
+		struct type_hist *h;
+		int offset = member->offset + i;
+
+		h = adt->histograms[evsel->core.idx];
+		update_hist_entry(&entry->hists, &h->addr[offset]);
+	}
+	return 0;
+}
+
+static int add_child_entries(struct annotated_data_browser *browser,
+			     struct annotated_data_type *adt,
+			     struct annotated_member *member,
+			     struct evsel *evsel, int indent)
+{
+	struct annotated_member *pos;
+	struct browser_entry *entry;
+	int nr_entries = 0;
+
+	entry = zalloc(sizeof(*entry));
+	if (entry == NULL)
+		return -1;
+
+	entry->data = member;
+	entry->indent = indent;
+	if (get_member_overhead(adt, entry, evsel) < 0) {
+		free(entry);
+		return -1;
+	}
+
+	list_add_tail(&entry->node, &browser->entries);
+	nr_entries++;
+
+	list_for_each_entry(pos, &member->children, node) {
+		int nr = add_child_entries(browser, adt, pos, evsel, indent + 1);
+
+		if (nr < 0)
+			return nr;
+
+		nr_entries += nr;
+	}
+
+	/* add an entry for the closing bracket ("}") */
+	if (!list_empty(&member->children)) {
+		entry = zalloc(sizeof(*entry));
+		if (entry == NULL)
+			return -1;
+
+		entry->indent = indent;
+		list_add_tail(&entry->node, &browser->entries);
+		nr_entries++;
+	}
+
+	return nr_entries;
+}
+
+static int annotated_data_browser__collect_entries(struct annotated_data_browser *browser)
+{
+	struct hist_entry *he = browser->b.priv;
+	struct annotated_data_type *adt = he->mem_type;
+	struct evsel *evsel = hists_to_evsel(he->hists);
+
+	INIT_LIST_HEAD(&browser->entries);
+	browser->b.entries = &browser->entries;
+	browser->b.nr_entries = add_child_entries(browser, adt, &adt->self,
+						  evsel, /*indent=*/0);
+	return 0;
+}
+
+static void annotated_data_browser__delete_entries(struct annotated_data_browser *browser)
+{
+	struct browser_entry *pos, *tmp;
+
+	list_for_each_entry_safe(pos, tmp, &browser->entries, node) {
+		list_del_init(&pos->node);
+		free(pos);
+	}
+}
+
+static unsigned int browser__refresh(struct ui_browser *uib)
+{
+	return ui_browser__list_head_refresh(uib);
+}
+
+static int browser__show(struct ui_browser *uib)
+{
+	struct hist_entry *he = uib->priv;
+	struct annotated_data_type *adt = he->mem_type;
+	const char *help = "Press 'h' for help on key bindings";
+	char title[256];
+
+	snprintf(title, sizeof(title), "Annotate type: '%s' (%d samples)",
+		 adt->self.type_name, he->stat.nr_events);
+
+	if (ui_browser__show(uib, title, help) < 0)
+		return -1;
+
+	/* second line header */
+	ui_browser__gotorc_title(uib, 0, 0);
+	ui_browser__set_color(uib, HE_COLORSET_ROOT);
+
+	if (symbol_conf.show_total_period)
+		strcpy(title, "Period");
+	else if (symbol_conf.show_nr_samples)
+		strcpy(title, "Samples");
+	else
+		strcpy(title, "Percent");
+
+	ui_browser__printf(uib, " %10s %10s %10s  %s",
+			   title, "Offset", "Size", "Field");
+	ui_browser__write_nstring(uib, "", uib->width);
+	return 0;
+}
+
+static void browser__write_overhead(struct ui_browser *uib,
+				    struct type_hist *total,
+				    struct type_hist_entry *hist, int row)
+{
+	u64 period = hist->period;
+	double percent = total->period ? (100.0 * period / total->period) : 0;
+	bool current = ui_browser__is_current_entry(uib, row);
+	int nr_samples = 0;
+
+	ui_browser__set_percent_color(uib, percent, current);
+
+	if (symbol_conf.show_total_period)
+		ui_browser__printf(uib, " %10" PRIu64, period);
+	else if (symbol_conf.show_nr_samples)
+		ui_browser__printf(uib, " %10d", nr_samples);
+	else
+		ui_browser__printf(uib, " %10.2f", percent);
+
+	ui_browser__set_percent_color(uib, 0, current);
+}
+
+static void browser__write(struct ui_browser *uib, void *entry, int row)
+{
+	struct browser_entry *be = entry;
+	struct annotated_member *member = be->data;
+	struct hist_entry *he = uib->priv;
+	struct annotated_data_type *adt = he->mem_type;
+	struct evsel *evsel = hists_to_evsel(he->hists);
+
+	if (member == NULL) {
+		bool current = ui_browser__is_current_entry(uib, row);
+
+		/* print the closing bracket */
+		ui_browser__set_percent_color(uib, 0, current);
+		ui_browser__write_nstring(uib, "", 11);
+		ui_browser__printf(uib, " %10s %10s  %*s};",
+				   "", "", be->indent * 4, "");
+		ui_browser__write_nstring(uib, "", uib->width);
+		return;
+	}
+
+	/* print the number */
+	browser__write_overhead(uib, adt->histograms[evsel->core.idx],
+				&be->hists, row);
+
+	/* print type info */
+	if (be->indent == 0 && !member->var_name) {
+		ui_browser__printf(uib, " %10d %10d  %s%s",
+				   member->offset, member->size,
+				   member->type_name,
+				   list_empty(&member->children) ? ";" : " {");
+	} else {
+		ui_browser__printf(uib, " %10d %10d  %*s%s\t%s%s",
+				   member->offset, member->size,
+				   be->indent * 4, "", member->type_name,
+				   member->var_name ?: "",
+				   list_empty(&member->children) ? ";" : " {");
+	}
+	/* fill the rest */
+	ui_browser__write_nstring(uib, "", uib->width);
+}
+
+static int annotated_data_browser__run(struct annotated_data_browser *browser,
+				       struct evsel *evsel __maybe_unused,
+				       struct hist_browser_timer *hbt)
+{
+	int delay_secs = hbt ? hbt->refresh : 0;
+	int key;
+
+	if (browser__show(&browser->b) < 0)
+		return -1;
+
+	while (1) {
+		key = ui_browser__run(&browser->b, delay_secs);
+
+		switch (key) {
+		case K_TIMER:
+			if (hbt)
+				hbt->timer(hbt->arg);
+			continue;
+		case K_F1:
+		case 'h':
+			ui_browser__help_window(&browser->b,
+		"UP/DOWN/PGUP\n"
+		"PGDN/SPACE    Navigate\n"
+		"</>           Move to prev/next symbol\n"
+		"q/ESC/CTRL+C  Exit\n\n");
+			continue;
+		case K_LEFT:
+		case '<':
+		case '>':
+		case K_ESC:
+		case 'q':
+		case CTRL('c'):
+			goto out;
+		default:
+			continue;
+		}
+	}
+out:
+	ui_browser__hide(&browser->b);
+	return key;
+}
+
+int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
+				  struct hist_browser_timer *hbt)
+{
+	struct annotated_data_browser browser = {
+		.b = {
+			.refresh = browser__refresh,
+			.seek	 = ui_browser__list_head_seek,
+			.write	 = browser__write,
+			.priv	 = he,
+			.extra_title_lines = 1,
+		},
+	};
+	int ret;
+
+	ui_helpline__push("Press ESC to exit");
+
+	ret = annotated_data_browser__collect_entries(&browser);
+	if (ret == 0)
+		ret = annotated_data_browser__run(&browser, evsel, hbt);
+
+	annotated_data_browser__delete_entries(&browser);
+
+	return ret;
+}
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index b150137a92dc..1cd857400038 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -1820,5 +1820,6 @@ int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel)
 	print_annotated_data_type(he->mem_type, &he->mem_type->self, evsel, 0);
 	printf("\n");
 
-	return 0;
+	/* move to the next entry */
+	return '>';
 }
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 01489db267d4..0a57d9f5ee78 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -10,6 +10,7 @@
 struct annotated_op_loc;
 struct debuginfo;
 struct evsel;
+struct hist_browser_timer;
 struct hist_entry;
 struct map_symbol;
 struct thread;
@@ -193,4 +194,16 @@ static inline int hist_entry__annotate_data_tty(struct hist_entry *he __maybe_un
 
 #endif /* HAVE_DWARF_SUPPORT */
 
+#ifdef HAVE_SLANG_SUPPORT
+int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
+				  struct hist_browser_timer *hbt);
+#else
+static inline int hist_entry__annotate_data_tui(struct hist_entry *he __maybe_unused,
+						struct evsel *evsel __maybe_unused,
+						struct hist_browser_timer *hbt __maybe_unused)
+{
+	return -1;
+}
+#endif /* HAVE_SLANG_SUPPORT */
+
 #endif /* _PERF_ANNOTATE_DATA_H */
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 5/7] perf annotate-data: Support event group display in TUI
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
                   ` (3 preceding siblings ...)
  2024-04-11  3:32 ` [PATCH 4/7] perf annotate-data: Add hist_entry__annotate_data_tui() Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 6/7] perf report: Add a menu item to annotate data type " Namhyung Kim
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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, Arnaldo Carvalho de Melo

Like in stdio, it should print all events in a group together.

Committer notes:

Collect it:

  root@number:~# perf record -a -e '{cpu_core/mem-loads,ldlat=30/P,cpu_core/mem-stores/P}'
  ^C[ perf record: Woken up 8 times to write data ]
  [ perf record: Captured and wrote 4.980 MB perf.data (55825 samples) ]
  root@number:~#

Then do it in stdio:

  root@number:~# perf annotate --stdio --data-type

  Annotate type: 'union ' in /usr/lib64/libc.so.6 (1131 samples):
   event[0] = cpu_core/mem-loads,ldlat=30/P
   event[1] = cpu_core/mem-stores/P
  ============================================================================
           Percent     offset       size  field
    100.00  100.00          0         40  union    {
    100.00  100.00          0         40      struct __pthread_mutex_s    __data {
     48.61   23.46          0          4          int     __lock;
      0.00    0.48          4          4          unsigned int    __count;
      6.38   41.32          8          4          int     __owner;
      8.74   34.02         12          4          unsigned int    __nusers;
     35.66    0.26         16          4          int     __kind;
      0.61    0.45         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;
     48.61   23.94          0          8      long int    __align;
                                          };

Now with TUI before this patch:

  root@number:~# perf annotate --tui --data-type
  Annotate type: 'union ' (790 samples)
      Percent     Offset       Size  Field
       100.00          0         40  union  {
       100.00          0         40      struct __pthread_mutex_s __data {
        48.61          0          4          int  __lock;
         0.00          4          4          unsigned int __count;
         6.38          8          4          int  __owner;
         8.74         12          4          unsigned int __nusers;
        35.66         16          4          int  __kind;
         0.61         20          2          short int    __spins;
         0.00         22          2          short int    __elision;
         0.00         24         16          __pthread_list_t     __list {
         0.00         24          8              struct __pthread_internal_list*  __prev;
         0.00         32          8              struct __pthread_internal_list*  __next;

         0.00          0          0      char*    __size;
        48.61          0          8      long int __align;
                                     };

And now after this patch:

Annotate type: 'union ' (790 samples)
               Percent     Offset       Size  Field
     100.00     100.00          0         40  union  {
     100.00     100.00          0         40      struct __pthread_mutex_s      __data {
      48.61      23.46          0          4          int       __lock;
       0.00       0.48          4          4          unsigned int      __count;
       6.38      41.32          8          4          int       __owner;
       8.74      34.02         12          4          unsigned int      __nusers;
      35.66       0.26         16          4          int       __kind;
       0.61       0.45         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;
      48.61      23.94          0          8      long int      __align;
                                              };

On a followup patch the --tui output should have this that is present in
--stdio:

  And the --stdio has all the missing info in TUI:

    Annotate type: 'union ' in /usr/lib64/libc.so.6 (1131 samples):
     event[0] = cpu_core/mem-loads,ldlat=30/P
     event[1] = cpu_core/mem-stores/P

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate-data.c | 50 ++++++++++++++++++++------
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate-data.c b/tools/perf/ui/browsers/annotate-data.c
index fefacaaf16db..a4a0f042f201 100644
--- a/tools/perf/ui/browsers/annotate-data.c
+++ b/tools/perf/ui/browsers/annotate-data.c
@@ -10,20 +10,27 @@
 #include "util/annotate.h"
 #include "util/annotate-data.h"
 #include "util/evsel.h"
+#include "util/evlist.h"
 #include "util/sort.h"
 
 struct annotated_data_browser {
 	struct ui_browser b;
 	struct list_head entries;
+	int nr_events;
 };
 
 struct browser_entry {
 	struct list_head node;
 	struct annotated_member *data;
-	struct type_hist_entry hists;
+	struct type_hist_entry *hists;
 	int indent;
 };
 
+static struct annotated_data_browser *get_browser(struct ui_browser *uib)
+{
+	return container_of(uib, struct annotated_data_browser, b);
+}
+
 static void update_hist_entry(struct type_hist_entry *dst,
 			      struct type_hist_entry *src)
 {
@@ -33,17 +40,21 @@ static void update_hist_entry(struct type_hist_entry *dst,
 
 static int get_member_overhead(struct annotated_data_type *adt,
 			       struct browser_entry *entry,
-			       struct evsel *evsel)
+			       struct evsel *leader)
 {
 	struct annotated_member *member = entry->data;
-	int i;
+	int i, k;
 
 	for (i = 0; i < member->size; i++) {
 		struct type_hist *h;
+		struct evsel *evsel;
 		int offset = member->offset + i;
 
-		h = adt->histograms[evsel->core.idx];
-		update_hist_entry(&entry->hists, &h->addr[offset]);
+		for_each_group_evsel(evsel, leader) {
+			h = adt->histograms[evsel->core.idx];
+			k = evsel__group_idx(evsel);
+			update_hist_entry(&entry->hists[k], &h->addr[offset]);
+		}
 	}
 	return 0;
 }
@@ -61,6 +72,12 @@ static int add_child_entries(struct annotated_data_browser *browser,
 	if (entry == NULL)
 		return -1;
 
+	entry->hists = calloc(browser->nr_events, sizeof(*entry->hists));
+	if (entry->hists == NULL) {
+		free(entry);
+		return -1;
+	}
+
 	entry->data = member;
 	entry->indent = indent;
 	if (get_member_overhead(adt, entry, evsel) < 0) {
@@ -113,6 +130,7 @@ static void annotated_data_browser__delete_entries(struct annotated_data_browser
 
 	list_for_each_entry_safe(pos, tmp, &browser->entries, node) {
 		list_del_init(&pos->node);
+		free(pos->hists);
 		free(pos);
 	}
 }
@@ -126,6 +144,7 @@ static int browser__show(struct ui_browser *uib)
 {
 	struct hist_entry *he = uib->priv;
 	struct annotated_data_type *adt = he->mem_type;
+	struct annotated_data_browser *browser = get_browser(uib);
 	const char *help = "Press 'h' for help on key bindings";
 	char title[256];
 
@@ -146,7 +165,8 @@ static int browser__show(struct ui_browser *uib)
 	else
 		strcpy(title, "Percent");
 
-	ui_browser__printf(uib, " %10s %10s %10s  %s",
+	ui_browser__printf(uib, "%*s %10s %10s %10s  %s",
+			   11 * (browser->nr_events - 1), "",
 			   title, "Offset", "Size", "Field");
 	ui_browser__write_nstring(uib, "", uib->width);
 	return 0;
@@ -175,18 +195,20 @@ static void browser__write_overhead(struct ui_browser *uib,
 
 static void browser__write(struct ui_browser *uib, void *entry, int row)
 {
+	struct annotated_data_browser *browser = get_browser(uib);
 	struct browser_entry *be = entry;
 	struct annotated_member *member = be->data;
 	struct hist_entry *he = uib->priv;
 	struct annotated_data_type *adt = he->mem_type;
-	struct evsel *evsel = hists_to_evsel(he->hists);
+	struct evsel *leader = hists_to_evsel(he->hists);
+	struct evsel *evsel;
 
 	if (member == NULL) {
 		bool current = ui_browser__is_current_entry(uib, row);
 
 		/* print the closing bracket */
 		ui_browser__set_percent_color(uib, 0, current);
-		ui_browser__write_nstring(uib, "", 11);
+		ui_browser__write_nstring(uib, "", 11 * browser->nr_events);
 		ui_browser__printf(uib, " %10s %10s  %*s};",
 				   "", "", be->indent * 4, "");
 		ui_browser__write_nstring(uib, "", uib->width);
@@ -194,8 +216,12 @@ static void browser__write(struct ui_browser *uib, void *entry, int row)
 	}
 
 	/* print the number */
-	browser__write_overhead(uib, adt->histograms[evsel->core.idx],
-				&be->hists, row);
+	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);
+	}
 
 	/* print type info */
 	if (be->indent == 0 && !member->var_name) {
@@ -267,11 +293,15 @@ int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
 			.priv	 = he,
 			.extra_title_lines = 1,
 		},
+		.nr_events = 1,
 	};
 	int ret;
 
 	ui_helpline__push("Press ESC to exit");
 
+	if (evsel__is_group_event(evsel))
+		browser.nr_events = evsel->core.nr_members;
+
 	ret = annotated_data_browser__collect_entries(&browser);
 	if (ret == 0)
 		ret = annotated_data_browser__run(&browser, evsel, hbt);
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 6/7] perf report: Add a menu item to annotate data type in TUI
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
                   ` (4 preceding siblings ...)
  2024-04-11  3:32 ` [PATCH 5/7] perf annotate-data: Support event group display in TUI Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11  3:32 ` [PATCH 7/7] perf report: Do not collect sample histogram unnecessarily Namhyung Kim
  2024-04-11 11:02 ` [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Arnaldo Carvalho de Melo
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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

When the hist entry has the type info, it should be able to display the
annotation browser for the type like in `perf annotate --data-type`.

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c    |  5 +++++
 tools/perf/ui/browsers/hists.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index dcd93ee5fc24..aaa6427a1224 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1694,6 +1694,11 @@ int cmd_report(int argc, const char **argv)
 	else
 		use_browser = 0;
 
+	if (report.data_type && use_browser == 1) {
+		symbol_conf.annotate_data_member = true;
+		symbol_conf.annotate_data_sample = true;
+	}
+
 	if (sort_order && strstr(sort_order, "ipc")) {
 		parse_options_usage(report_usage, options, "s", 1);
 		goto error;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 0c02b3a8e121..71b32591d61a 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -38,6 +38,7 @@
 #include "../ui.h"
 #include "map.h"
 #include "annotate.h"
+#include "annotate-data.h"
 #include "srcline.h"
 #include "string2.h"
 #include "units.h"
@@ -2505,6 +2506,32 @@ add_annotate_opt(struct hist_browser *browser __maybe_unused,
 	return 1;
 }
 
+static int
+do_annotate_type(struct hist_browser *browser, struct popup_action *act)
+{
+	struct hist_entry *he = browser->he_selection;
+
+	hist_entry__annotate_data_tui(he, act->evsel, browser->hbt);
+	ui_browser__handle_resize(&browser->b);
+	return 0;
+}
+
+static int
+add_annotate_type_opt(struct hist_browser *browser,
+		      struct popup_action *act, char **optstr,
+		      struct hist_entry *he)
+{
+	if (he == NULL || he->mem_type == NULL || he->mem_type->histograms == NULL)
+		return 0;
+
+	if (asprintf(optstr, "Annotate type %s", he->mem_type->self.type_name) < 0)
+		return 0;
+
+	act->evsel = hists_to_evsel(browser->hists);
+	act->fn = do_annotate_type;
+	return 1;
+}
+
 static int
 do_zoom_thread(struct hist_browser *browser, struct popup_action *act)
 {
@@ -3307,6 +3334,10 @@ static int evsel__hists_browse(struct evsel *evsel, int nr_events, const char *h
 						       browser->he_selection->ip);
 		}
 skip_annotation:
+		nr_options += add_annotate_type_opt(browser,
+						    &actions[nr_options],
+						    &options[nr_options],
+						    browser->he_selection);
 		nr_options += add_thread_opt(browser, &actions[nr_options],
 					     &options[nr_options], thread);
 		nr_options += add_dso_opt(browser, &actions[nr_options],
-- 
2.44.0.478.gd926399ef9-goog


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

* [PATCH 7/7] perf report: Do not collect sample histogram unnecessarily
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
                   ` (5 preceding siblings ...)
  2024-04-11  3:32 ` [PATCH 6/7] perf report: Add a menu item to annotate data type " Namhyung Kim
@ 2024-04-11  3:32 ` Namhyung Kim
  2024-04-11 11:02 ` [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Arnaldo Carvalho de Melo
  7 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2024-04-11  3:32 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 data type profiling alone doesn't need the sample histogram for
functions.  It only needs the histogram for the types.

Let's remove the condition in the report_callback to check if data type
profiling is selected and make sure the annotation has the 'struct
annotated_source' instantiated before calling symbol__disassemble().

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c | 2 +-
 tools/perf/util/annotate.c  | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index aaa6427a1224..dafba6e030ef 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -172,7 +172,7 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
 	struct mem_info *mi;
 	struct branch_info *bi;
 
-	if (!ui__has_annotation() && !rep->symbol_ipc && !rep->data_type)
+	if (!ui__has_annotation() && !rep->symbol_ipc)
 		return 0;
 
 	if (sort__mode == SORT_MODE__BRANCH) {
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ec79c120a7d2..7595c8fbc2c5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -908,6 +908,13 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
 
 	args.arch = arch;
 	args.ms = *ms;
+
+	if (notes->src == NULL) {
+		notes->src = annotated_source__new();
+		if (notes->src == NULL)
+			return -1;
+	}
+
 	if (annotate_opts.full_addr)
 		notes->src->start = map__objdump_2mem(ms->map, ms->sym->start);
 	else
-- 
2.44.0.478.gd926399ef9-goog


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

* Re: [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2)
  2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
                   ` (6 preceding siblings ...)
  2024-04-11  3:32 ` [PATCH 7/7] perf report: Do not collect sample histogram unnecessarily Namhyung Kim
@ 2024-04-11 11:02 ` Arnaldo Carvalho de Melo
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-04-11 11:02 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Kan Liang, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

On Wed, Apr 10, 2024 at 08:32:49PM -0700, Namhyung Kim wrote:
> Hello,
> 
> This is to support interactive TUI browser for type annotation.
> 
> v2 changes:
>  * fix build errors when libslang2 or libdw is missing  (Arnaldo)
>  * update commit messages with examples  (Arnaldo)
>  * skip updating sample histogram for stack canary  (Arnaldo)
>  * add Reviewed-by from Ian

Thanks, applied.

- Arnaldo
  
> Like the normal (code) annotation, it should be able to display the data type
> annotation.  Now `perf annotate --data-type` will show the result in TUI by
> default if it's enabled.  Also `perf report -s type` can show the same output
> using a menu item.
> 
> It's still in a very early stage and supports the basic functionalities only.
> I'll work on more features like in the normal annotation browser later.
> 
> The code is also available at 'perf/annotate-data-tui-v2' branch at
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (7):
>   perf annotate-data: Skip sample histogram for stack canary
>   perf annotate: Show progress of sample processing
>   perf annotate-data: Add hist_entry__annotate_data_tty()
>   perf annotate-data: Add hist_entry__annotate_data_tui()
>   perf annotate-data: Support event group display in TUI
>   perf report: Add a menu item to annotate data type in TUI
>   perf report: Do not collect sample histogram unnecessarily
> 
>  tools/perf/builtin-annotate.c          | 149 ++++--------
>  tools/perf/builtin-report.c            |   7 +-
>  tools/perf/ui/browsers/Build           |   1 +
>  tools/perf/ui/browsers/annotate-data.c | 312 +++++++++++++++++++++++++
>  tools/perf/ui/browsers/hists.c         |  31 +++
>  tools/perf/util/annotate-data.c        | 113 +++++++++
>  tools/perf/util/annotate-data.h        |  22 ++
>  tools/perf/util/annotate.c             |  12 +-
>  8 files changed, 534 insertions(+), 113 deletions(-)
>  create mode 100644 tools/perf/ui/browsers/annotate-data.c
> 
> 
> base-commit: 9c3e9af74326978ba6f4432bb038e6c80f4f56fd
> -- 
> 2.44.0.478.gd926399ef9-goog

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

end of thread, other threads:[~2024-04-11 11:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-11  3:32 [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Namhyung Kim
2024-04-11  3:32 ` [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary Namhyung Kim
2024-04-11  3:32 ` [PATCH 2/7] perf annotate: Show progress of sample processing Namhyung Kim
2024-04-11  3:32 ` [PATCH 3/7] perf annotate-data: Add hist_entry__annotate_data_tty() Namhyung Kim
2024-04-11  3:32 ` [PATCH 4/7] perf annotate-data: Add hist_entry__annotate_data_tui() Namhyung Kim
2024-04-11  3:32 ` [PATCH 5/7] perf annotate-data: Support event group display in TUI Namhyung Kim
2024-04-11  3:32 ` [PATCH 6/7] perf report: Add a menu item to annotate data type " Namhyung Kim
2024-04-11  3:32 ` [PATCH 7/7] perf report: Do not collect sample histogram unnecessarily Namhyung Kim
2024-04-11 11:02 ` [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2) Arnaldo Carvalho de Melo

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