All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.