[PATCHSET 0/6] perf annotate: Support --code-with-type on TUI (v1)

[PATCHSET 0/6] perf annotate: Support --code-with-type on TUI (v1)

[Namhyung Kim]

Hello,

The --code-with-type option is to show normal annotate output with type
information.  It was supported only on --stdio, but this change adds it
to TUI as well.

Actually the command line option sets the default behavior and users can
change it by pressing 'y' key in the TUI annotate browser.

The code is also available at 'perf/annotate-code-type-tui-v1' branch at
https://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (6):
  perf annotate: Rename to __hist_entry__tui_annotate()
  perf annotate: Remove __annotation_line__write()
  perf annotate: Pass annotation_print_data to annotaiton_line__write()
  perf annotate: Simplify width calculation in annotation_line__write()
  perf annotate: Add --code-with-type support for TUI
  perf annotate: Add 'y' hot key to toggle data type display

 tools/perf/Documentation/perf-annotate.txt |   1 -
 tools/perf/builtin-annotate.c              |   5 -
 tools/perf/ui/browsers/annotate.c          |  43 ++++--
 tools/perf/ui/browsers/hists.c             |   2 +-
 tools/perf/util/annotate.c                 | 149 ++++++++++++++-------
 tools/perf/util/annotate.h                 |  27 ++--
 tools/perf/util/hist.h                     |  12 +-
 7 files changed, 149 insertions(+), 90 deletions(-)

-- 
2.49.0

[PATCH 1/6] perf annotate: Rename to __hist_entry__tui_annotate()

[Namhyung Kim]

There are three different but similar functions for annotation on TUI.
Rename it to __hist_entry__tui_annotate() and make sure it passes 'he'.
It's not used for now but it'll be needed for later use.

Also remove map_symbol__tui_annotate() which was a simple wrapper.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c | 17 +++++++----------
 tools/perf/ui/browsers/hists.c    |  2 +-
 tools/perf/util/annotate.h        | 12 ------------
 tools/perf/util/hist.h            | 12 +++++++-----
 4 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index ab776b1ed2d5b4ba..af3e52ce4da62eb9 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -27,6 +27,7 @@ struct annotate_browser {
 	struct rb_node		   *curr_hot;
 	struct annotation_line	   *selection;
 	struct arch		   *arch;
+	struct hist_entry	   *he;
 	bool			    searching_backwards;
 	char			    search_bf[128];
 };
@@ -483,7 +484,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	target_ms.map = ms->map;
 	target_ms.sym = dl->ops.target.sym;
 	annotation__unlock(notes);
-	symbol__tui_annotate(&target_ms, evsel, hbt);
+	__hist_entry__tui_annotate(browser->he, &target_ms, evsel, hbt);
 	sym_title(ms->sym, ms->map, title, sizeof(title), annotate_opts.percent_type);
 	ui_browser__show_title(&browser->b, title);
 	return true;
@@ -958,12 +959,6 @@ static int annotate_browser__run(struct annotate_browser *browser,
 	return key;
 }
 
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
-			     struct hist_browser_timer *hbt)
-{
-	return symbol__tui_annotate(ms, evsel, hbt);
-}
-
 int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
@@ -972,11 +967,12 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
 	SLang_init_tty(0, 0, 0);
 	SLtty_set_suspend_state(true);
 
-	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
+	return __hist_entry__tui_annotate(he, &he->ms, evsel, hbt);
 }
 
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
-			 struct hist_browser_timer *hbt)
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+			       struct evsel *evsel,
+			       struct hist_browser_timer *hbt)
 {
 	struct symbol *sym = ms->sym;
 	struct annotation *notes = symbol__annotation(sym);
@@ -990,6 +986,7 @@ int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
 			.priv	 = ms,
 			.use_navkeypressed = true,
 		},
+		.he = he,
 	};
 	struct dso *dso;
 	int ret = -1, err;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d26b925e3d7f46af..55455c49faf01891 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2484,8 +2484,8 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
 	else
 		evsel = hists_to_evsel(browser->hists);
 
-	err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
 	he = hist_browser__selected_entry(browser);
+	err = __hist_entry__tui_annotate(he, &act->ms, evsel, browser->hbt);
 	/*
 	 * offer option to annotate the other branch source or target
 	 * (if they exists) when returning from annotate
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index bbb89b32f398b3c9..9cd723b8b987db1a 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -470,18 +470,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
 int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
 int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
 
-#ifdef HAVE_SLANG_SUPPORT
-int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
-			 struct hist_browser_timer *hbt);
-#else
-static inline int symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
-				struct evsel *evsel  __maybe_unused,
-				struct hist_browser_timer *hbt __maybe_unused)
-{
-	return 0;
-}
-#endif
-
 void annotation_options__init(void);
 void annotation_options__exit(void);
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index c64254088fc77246..11ae738772ca4f61 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -712,8 +712,9 @@ struct block_hist {
 #include "../ui/keysyms.h"
 void attr_to_script(char *buf, struct perf_event_attr *attr);
 
-int map_symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
-			     struct hist_browser_timer *hbt);
+int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
+			       struct evsel *evsel,
+			       struct hist_browser_timer *hbt);
 
 int hist_entry__tui_annotate(struct hist_entry *he, struct evsel *evsel,
 			     struct hist_browser_timer *hbt);
@@ -741,9 +742,10 @@ int evlist__tui_browse_hists(struct evlist *evlist __maybe_unused,
 {
 	return 0;
 }
-static inline int map_symbol__tui_annotate(struct map_symbol *ms __maybe_unused,
-					   struct evsel *evsel __maybe_unused,
-					   struct hist_browser_timer *hbt __maybe_unused)
+static inline int __hist_entry__tui_annotate(struct hist_entry *he __maybe_unused,
+					     struct map_symbol *ms __maybe_unused,
+					     struct evsel *evsel __maybe_unused,
+					     struct hist_browser_timer *hbt __maybe_unused)
 {
 	return 0;
 }
-- 
2.49.0

[PATCH 2/6] perf annotate: Remove __annotation_line__write()

[Namhyung Kim]

Get rid of the internal function and convert function arguments into
local variables if they are used more then twice.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 49 ++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 264a212b47df850c..48fac08c4c9502b1 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1934,24 +1934,25 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
 	return -ENOMEM;
 }
 
-static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
-				     bool first_line, bool current_entry, bool change_color, int width,
-				     void *obj, unsigned int percent_type,
-				     int  (*obj__set_color)(void *obj, int color),
-				     void (*obj__set_percent_color)(void *obj, double percent, bool current),
-				     int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current),
-				     void (*obj__printf)(void *obj, const char *fmt, ...),
-				     void (*obj__write_graph)(void *obj, int graph))
-
-{
-	double percent_max = annotation_line__max_percent(al, percent_type);
-	int pcnt_width = annotation__pcnt_width(notes),
-	    cycles_width = annotation__cycles_width(notes);
+void annotation_line__write(struct annotation_line *al, struct annotation *notes,
+			    struct annotation_write_ops *wops)
+{
+	bool current_entry = wops->current_entry;
+	double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
+	int width = wops->width;
+	int pcnt_width = annotation__pcnt_width(notes);
+	int cycles_width = annotation__cycles_width(notes);
 	bool show_title = false;
 	char bf[256];
 	int printed;
-
-	if (first_line && (al->offset == -1 || percent_max == 0.0)) {
+	void *obj = wops->obj;
+	int  (*obj__set_color)(void *obj, int color) = wops->set_color;
+	void (*obj__set_percent_color)(void *obj, double percent, bool current) = wops->set_percent_color;
+	int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current) = wops->set_jumps_percent_color;
+	void (*obj__printf)(void *obj, const char *fmt, ...) = wops->printf;
+	void (*obj__write_graph)(void *obj, int graph) = wops->write_graph;
+
+	if (wops->first_line && (al->offset == -1 || percent_max == 0.0)) {
 		if (notes->branch && al->cycles) {
 			if (al->cycles->ipc == 0.0 && al->cycles->avg == 0)
 				show_title = true;
@@ -1965,7 +1966,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 		for (i = 0; i < al->data_nr; i++) {
 			double percent;
 
-			percent = annotation_data__percent(&al->data[i], percent_type);
+			percent = annotation_data__percent(&al->data[i],
+							   annotate_opts.percent_type);
 
 			obj__set_percent_color(obj, percent, current_entry);
 			if (symbol_conf.show_total_period) {
@@ -2101,10 +2103,10 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 			}
 		}
 
-		if (change_color)
+		if (wops->change_color)
 			color = obj__set_color(obj, HE_COLORSET_ADDR);
 		obj__printf(obj, bf);
-		if (change_color)
+		if (wops->change_color)
 			obj__set_color(obj, color);
 
 		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
@@ -2114,17 +2116,6 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
 
 }
 
-void annotation_line__write(struct annotation_line *al, struct annotation *notes,
-			    struct annotation_write_ops *wops)
-{
-	__annotation_line__write(al, notes, wops->first_line, wops->current_entry,
-				 wops->change_color, wops->width, wops->obj,
-				 annotate_opts.percent_type,
-				 wops->set_color, wops->set_percent_color,
-				 wops->set_jumps_percent_color, wops->printf,
-				 wops->write_graph);
-}
-
 int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
 		      struct arch **parch)
 {
-- 
2.49.0

[PATCH 3/6] perf annotate: Pass annotation_print_data to annotaiton_line__write()

[Namhyung Kim]

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c | 13 ++++++++++--
 tools/perf/util/annotate.c        | 35 +++++++++++++++++--------------
 tools/perf/util/annotate.h        | 15 +++++++++++--
 3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index af3e52ce4da62eb9..0057fb485e85aaaf 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -28,6 +28,8 @@ struct annotate_browser {
 	struct annotation_line	   *selection;
 	struct arch		   *arch;
 	struct hist_entry	   *he;
+	struct debuginfo	   *dbg;
+	struct evsel		   *evsel;
 	bool			    searching_backwards;
 	char			    search_bf[128];
 };
@@ -108,12 +110,18 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 		.printf			 = annotate_browser__printf,
 		.write_graph		 = annotate_browser__write_graph,
 	};
+	struct annotation_print_data apd = {
+		.he = ab->he,
+		.arch = ab->arch,
+		.evsel = ab->evsel,
+		.dbg = ab->dbg,
+	};
 
 	/* The scroll bar isn't being used */
 	if (!browser->navkeypressed)
 		ops.width += 1;
 
-	annotation_line__write(al, notes, &ops);
+	annotation_line__write(al, notes, &ops, &apd);
 
 	if (ops.current_entry)
 		ab->selection = al;
@@ -902,7 +910,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 			continue;
 		}
 		case 'P':
-			map_symbol__annotation_dump(ms, evsel);
+			map_symbol__annotation_dump(ms, evsel, browser->he);
 			continue;
 		case 't':
 			if (symbol_conf.show_total_period) {
@@ -987,6 +995,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 			.use_navkeypressed = true,
 		},
 		.he = he,
+		.evsel = evsel,
 	};
 	struct dso *dso;
 	int ret = -1, err;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 48fac08c4c9502b1..7df726b99541a571 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -765,15 +765,6 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
 			    struct debuginfo *dbg, struct disasm_line *dl,
 			    int *type_offset);
 
-struct annotation_print_data {
-	struct hist_entry *he;
-	struct evsel *evsel;
-	struct arch *arch;
-	struct debuginfo *dbg;
-	u64 start;
-	int addr_fmt_width;
-};
-
 static int
 annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
 		       struct annotation_options *opts, int printed,
@@ -1230,7 +1221,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 	struct annotation_print_data apd = {
 		.he = he,
 		.evsel = evsel,
-		.start = map__rip_2objdump(map, sym->start),
 	};
 	int printed = 2, queue_len = 0;
 	int more = 0;
@@ -1357,7 +1347,8 @@ static void FILE__write_graph(void *fp, int graph)
 	fputs(s, fp);
 }
 
-static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
+static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
+				     struct annotation_print_data *apd)
 {
 	struct annotation *notes = symbol__annotation(sym);
 	struct annotation_write_ops wops = {
@@ -1374,7 +1365,7 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
 	list_for_each_entry(al, &notes->src->source, node) {
 		if (annotation_line__filter(al))
 			continue;
-		annotation_line__write(al, notes, &wops);
+		annotation_line__write(al, notes, &wops, apd);
 		fputc('\n', fp);
 		wops.first_line = false;
 	}
@@ -1382,13 +1373,18 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
 	return 0;
 }
 
-int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
+int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
+				struct hist_entry *he)
 {
 	const char *ev_name = evsel__name(evsel);
 	char buf[1024];
 	char *filename;
 	int err = -1;
 	FILE *fp;
+	struct annotation_print_data apd = {
+		.he = he,
+		.evsel = evsel,
+	};
 
 	if (asprintf(&filename, "%s.annotation", ms->sym->name) < 0)
 		return -1;
@@ -1404,7 +1400,7 @@ int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
 
 	fprintf(fp, "%s() %s\nEvent: %s\n\n",
 		ms->sym->name, dso__long_name(map__dso(ms->map)), ev_name);
-	symbol__annotate_fprintf2(ms->sym, fp);
+	symbol__annotate_fprintf2(ms->sym, fp, &apd);
 
 	fclose(fp);
 	err = 0;
@@ -1655,6 +1651,10 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
 	struct symbol *sym = ms->sym;
 	struct rb_root source_line = RB_ROOT;
 	struct hists *hists = evsel__hists(evsel);
+	struct annotation_print_data apd = {
+		.he = he,
+		.evsel = evsel,
+	};
 	char buf[1024];
 	int err;
 
@@ -1677,7 +1677,7 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
 	hists__scnprintf_title(hists, buf, sizeof(buf));
 	fprintf(stdout, "%s, [percent: %s]\n%s() %s\n",
 		buf, percent_type_str(annotate_opts.percent_type), sym->name, dso__long_name(dso));
-	symbol__annotate_fprintf2(sym, stdout);
+	symbol__annotate_fprintf2(sym, stdout, &apd);
 
 	annotated_source__purge(symbol__annotation(sym)->src);
 
@@ -1935,7 +1935,8 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
 }
 
 void annotation_line__write(struct annotation_line *al, struct annotation *notes,
-			    struct annotation_write_ops *wops)
+			    struct annotation_write_ops *wops,
+			    struct annotation_print_data *apd)
 {
 	bool current_entry = wops->current_entry;
 	double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
@@ -2112,6 +2113,8 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
 
 		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
+
+		(void)apd;
 	}
 
 }
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 9cd723b8b987db1a..bc195bd0429e039d 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -199,8 +199,18 @@ struct annotation_write_ops {
 	void (*write_graph)(void *obj, int graph);
 };
 
+struct annotation_print_data {
+	struct hist_entry *he;
+	struct evsel *evsel;
+	struct arch *arch;
+	struct debuginfo *dbg;
+	u64 start;
+	int addr_fmt_width;
+};
+
 void annotation_line__write(struct annotation_line *al, struct annotation *notes,
-			    struct annotation_write_ops *ops);
+			    struct annotation_write_ops *ops,
+			    struct annotation_print_data *apd);
 
 int __annotation__scnprintf_samples_period(struct annotation *notes,
 					   char *bf, size_t size,
@@ -462,7 +472,8 @@ void symbol__annotate_zero_histogram(struct symbol *sym, struct evsel *evsel);
 void symbol__annotate_decay_histogram(struct symbol *sym, struct evsel *evsel);
 void annotated_source__purge(struct annotated_source *as);
 
-int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel);
+int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
+				struct hist_entry *he);
 
 bool ui__has_annotation(void);
 
-- 
2.49.0

[PATCH 4/6] perf annotate: Simplify width calculation in annotation_line__write()

[Namhyung Kim]

The width is updated after each part is printed.  It can skip the output
processing if the total printed size is bigger than the width.

No function changes intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 7df726b99541a571..94f73c8944cde519 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1991,6 +1991,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 					   symbol_conf.show_nr_samples ? "Samples" : "Percent");
 		}
 	}
+	width -= pcnt_width;
 
 	if (notes->branch) {
 		if (al->cycles && al->cycles->ipc)
@@ -2054,11 +2055,12 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 			obj__printf(obj, "%*s", ANNOTATION__AVG_IPC_WIDTH, bf);
 		}
 	}
+	width -= cycles_width;
 
 	obj__printf(obj, " ");
 
 	if (!*al->line)
-		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width, " ");
+		obj__printf(obj, "%-*s", width, " ");
 	else if (al->offset == -1) {
 		if (al->line_nr && annotate_opts.show_linenr)
 			printed = scnprintf(bf, sizeof(bf), "%-*d ",
@@ -2067,7 +2069,7 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 			printed = scnprintf(bf, sizeof(bf), "%-*s  ",
 					    notes->src->widths.addr, " ");
 		obj__printf(obj, bf);
-		obj__printf(obj, "%-*s", width - printed - pcnt_width - cycles_width + 1, al->line);
+		obj__printf(obj, "%-*s", width - printed + 1, al->line);
 	} else {
 		u64 addr = al->offset;
 		int color = -1;
@@ -2110,9 +2112,11 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 		if (wops->change_color)
 			obj__set_color(obj, color);
 
+		width -= printed + 3;
+
 		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
 
-		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
+		obj__printf(obj, "%-*s", width, bf);
 
 		(void)apd;
 	}
-- 
2.49.0

[PATCH 5/6] perf annotate: Add --code-with-type support for TUI

[Namhyung Kim]

Until now, the --code-with-type option is available only on stdio.
But it was an artifical limitation because of an implemention issue.

Implement the same logic in annotation_line__write() for stdio2/TUI.
Make disasm_line__write() return the number of printed characters so
that it can skip unnecessary operations when the screen is full.

Remove the limitation and update the man page.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-annotate.txt |  1 -
 tools/perf/builtin-annotate.c              |  5 --
 tools/perf/ui/browsers/annotate.c          |  6 +++
 tools/perf/util/annotate.c                 | 61 +++++++++++++++++++---
 4 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index 46090c5b42b4762f..547f1a2680185e3c 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -170,7 +170,6 @@ include::itrace.txt[]
 
 --code-with-type::
 	Show data type info in code annotation (for memory instructions only).
-	Currently it only works with --stdio option.
 
 
 SEE ALSO
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 9833c2c82a2fee46..6debd725392db4a4 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -917,11 +917,6 @@ int cmd_annotate(int argc, const char **argv)
 		symbol_conf.annotate_data_sample = true;
 	} else if (annotate_opts.code_with_type) {
 		symbol_conf.annotate_data_member = true;
-
-		if (!annotate.use_stdio) {
-			pr_err("--code-with-type only works with --stdio.\n");
-			goto out_delete;
-		}
 	}
 
 	setup_browser(true);
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 0057fb485e85aaaf..686fa54e545e275c 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -4,6 +4,7 @@
 #include "../ui.h"
 #include "../../util/annotate.h"
 #include "../../util/debug.h"
+#include "../../util/debuginfo.h"
 #include "../../util/dso.h"
 #include "../../util/hist.h"
 #include "../../util/sort.h"
@@ -1021,6 +1022,9 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 
 	ui_helpline__push("Press ESC to exit");
 
+	if (annotate_opts.code_with_type)
+		browser.dbg = debuginfo__new(dso__long_name(dso));
+
 	browser.b.width = notes->src->widths.max_line_len;
 	browser.b.nr_entries = notes->src->nr_entries;
 	browser.b.entries = &notes->src->source;
@@ -1031,6 +1035,8 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 
 	ret = annotate_browser__run(&browser, evsel, hbt);
 
+	if (annotate_opts.code_with_type)
+		debuginfo__delete(browser.dbg);
 	if(not_annotated)
 		annotated_source__purge(notes->src);
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 94f73c8944cde519..bf480f5b87eedd50 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1362,6 +1362,11 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
 	};
 	struct annotation_line *al;
 
+	if (annotate_opts.code_with_type) {
+		evsel__get_arch(apd->evsel, &apd->arch);
+		apd->dbg = debuginfo__new(dso__long_name(map__dso(apd->he->ms.map)));
+	}
+
 	list_for_each_entry(al, &notes->src->source, node) {
 		if (annotation_line__filter(al))
 			continue;
@@ -1370,6 +1375,9 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
 		wops.first_line = false;
 	}
 
+	if (annotate_opts.code_with_type)
+		debuginfo__delete(apd->dbg);
+
 	return 0;
 }
 
@@ -1742,7 +1750,7 @@ static double annotation_line__max_percent(struct annotation_line *al,
 	return percent_max;
 }
 
-static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
+static int disasm_line__write(struct disasm_line *dl, struct annotation *notes,
 			       void *obj, char *bf, size_t size,
 			       void (*obj__printf)(void *obj, const char *fmt, ...),
 			       void (*obj__write_graph)(void *obj, int graph))
@@ -1770,8 +1778,8 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes,
 		obj__printf(obj, "  ");
 	}
 
-	disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
-			       notes->src->widths.max_ins_name);
+	return disasm_line__scnprintf(dl, bf, size, !annotate_opts.use_offset,
+				      notes->src->widths.max_ins_name);
 }
 
 static void ipc_coverage_string(char *bf, int size, struct annotation *notes)
@@ -2114,11 +2122,52 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
 
 		width -= printed + 3;
 
-		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
+		printed = disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf),
+					     obj__printf, obj__write_graph);
+
+		obj__printf(obj, "%s", bf);
+		width -= printed;
+
+		if (annotate_opts.code_with_type && apd->dbg) {
+			struct annotated_data_type *data_type;
+			int offset = 0;
+
+			data_type = __hist_entry__get_data_type(apd->he, apd->arch,
+								apd->dbg,
+								disasm_line(al),
+								&offset);
+			if (data_type && data_type != NO_TYPE) {
+				char member[256];
+
+				printed = scnprintf(bf, sizeof(bf),
+						    "\t\t# data-type: %s",
+						    data_type->self.type_name);
 
-		obj__printf(obj, "%-*s", width, bf);
+				if (data_type != &stackop_type &&
+				    data_type != &canary_type &&
+				    sizeof(bf) > (size_t)printed) {
+					printed += scnprintf(bf + printed,
+							     sizeof(bf) - printed,
+							     " +%#x", offset);
+				}
+
+				if (annotated_data_type__get_member_name(data_type,
+									 member,
+									 sizeof(member),
+									 offset) &&
+				    sizeof(bf) > (size_t)printed) {
+					printed += scnprintf(bf + printed,
+							     sizeof(bf) - printed,
+							     " (%s)", member);
+				}
 
-		(void)apd;
+				obj__printf(obj, "%-*s", width, bf);
+			} else {
+				obj__printf(obj, "%-*s", width, " ");
+			}
+		} else {
+			obj__printf(obj, "%-*s", width, " ");
+		}
 	}
 
 }
-- 
2.49.0

[PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Namhyung Kim]

Support data type display with a key press so that users can toggle the
output dynamically on TUI.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/annotate.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 686fa54e545e275c..cd1d452035a265d3 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"b             Toggle percent base [period/hits]\n"
 		"B             Branch counter abbr list (Optional)\n"
 		"?             Search string backwards\n"
-		"f             Toggle showing offsets to full address\n");
+		"f             Toggle showing offsets to full address\n"
+		"y             Toggle data type display\n");
 			continue;
 		case 'r':
 			script_browse(NULL, NULL);
@@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		case 'f':
 			annotation__toggle_full_addr(notes, ms);
 			continue;
+		case 'y':
+			annotate_opts.code_with_type ^= 1;
+			if (browser->dbg == NULL)
+				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
+			continue;
 		case K_LEFT:
 		case '<':
 		case '>':
@@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
 
 	ret = annotate_browser__run(&browser, evsel, hbt);
 
-	if (annotate_opts.code_with_type)
-		debuginfo__delete(browser.dbg);
+	debuginfo__delete(browser.dbg);
 	if(not_annotated)
 		annotated_source__purge(notes->src);
 
-- 
2.49.0

Re: [PATCH 2/6] perf annotate: Remove __annotation_line__write()

[Arnaldo Carvalho de Melo]

On Sat, May 31, 2025 at 11:52:58PM -0700, Namhyung Kim wrote:
> Get rid of the internal function and convert function arguments into
> local variables if they are used more then twice.

"more than once"? "twice or more"?

- Arnaldo

> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/annotate.c | 49 ++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 264a212b47df850c..48fac08c4c9502b1 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1934,24 +1934,25 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
>  	return -ENOMEM;
>  }
>  
> -static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
> -				     bool first_line, bool current_entry, bool change_color, int width,
> -				     void *obj, unsigned int percent_type,
> -				     int  (*obj__set_color)(void *obj, int color),
> -				     void (*obj__set_percent_color)(void *obj, double percent, bool current),
> -				     int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current),
> -				     void (*obj__printf)(void *obj, const char *fmt, ...),
> -				     void (*obj__write_graph)(void *obj, int graph))
> -
> -{
> -	double percent_max = annotation_line__max_percent(al, percent_type);
> -	int pcnt_width = annotation__pcnt_width(notes),
> -	    cycles_width = annotation__cycles_width(notes);
> +void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> +			    struct annotation_write_ops *wops)
> +{
> +	bool current_entry = wops->current_entry;
> +	double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
> +	int width = wops->width;
> +	int pcnt_width = annotation__pcnt_width(notes);
> +	int cycles_width = annotation__cycles_width(notes);
>  	bool show_title = false;
>  	char bf[256];
>  	int printed;
> -
> -	if (first_line && (al->offset == -1 || percent_max == 0.0)) {
> +	void *obj = wops->obj;
> +	int  (*obj__set_color)(void *obj, int color) = wops->set_color;
> +	void (*obj__set_percent_color)(void *obj, double percent, bool current) = wops->set_percent_color;
> +	int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current) = wops->set_jumps_percent_color;
> +	void (*obj__printf)(void *obj, const char *fmt, ...) = wops->printf;
> +	void (*obj__write_graph)(void *obj, int graph) = wops->write_graph;
> +
> +	if (wops->first_line && (al->offset == -1 || percent_max == 0.0)) {
>  		if (notes->branch && al->cycles) {
>  			if (al->cycles->ipc == 0.0 && al->cycles->avg == 0)
>  				show_title = true;
> @@ -1965,7 +1966,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
>  		for (i = 0; i < al->data_nr; i++) {
>  			double percent;
>  
> -			percent = annotation_data__percent(&al->data[i], percent_type);
> +			percent = annotation_data__percent(&al->data[i],
> +							   annotate_opts.percent_type);
>  
>  			obj__set_percent_color(obj, percent, current_entry);
>  			if (symbol_conf.show_total_period) {
> @@ -2101,10 +2103,10 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
>  			}
>  		}
>  
> -		if (change_color)
> +		if (wops->change_color)
>  			color = obj__set_color(obj, HE_COLORSET_ADDR);
>  		obj__printf(obj, bf);
> -		if (change_color)
> +		if (wops->change_color)
>  			obj__set_color(obj, color);
>  
>  		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
> @@ -2114,17 +2116,6 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
>  
>  }
>  
> -void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> -			    struct annotation_write_ops *wops)
> -{
> -	__annotation_line__write(al, notes, wops->first_line, wops->current_entry,
> -				 wops->change_color, wops->width, wops->obj,
> -				 annotate_opts.percent_type,
> -				 wops->set_color, wops->set_percent_color,
> -				 wops->set_jumps_percent_color, wops->printf,
> -				 wops->write_graph);
> -}
> -
>  int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
>  		      struct arch **parch)
>  {
> -- 
> 2.49.0

Re: [PATCH 2/6] perf annotate: Remove __annotation_line__write()

[Arnaldo Carvalho de Melo]

On Thu, Jun 05, 2025 at 04:55:26PM -0300, Arnaldo Carvalho de Melo wrote:
> On Sat, May 31, 2025 at 11:52:58PM -0700, Namhyung Kim wrote:
> > Get rid of the internal function and convert function arguments into
> > local variables if they are used more then twice.
> 
> "more than once"? "twice or more"?

Well, its "more than twice", as you did:

-               if (change_color)
+               if (wops->change_color)
                        color = obj__set_color(obj, HE_COLORSET_ADDR);
                obj__printf(obj, bf);
-               if (change_color)
+               if (wops->change_color)
                        obj__set_color(obj, color);

Ok, I wouldn't touch this, i.e. would leave all as local variables that
way the patch would be smaller :-)

So its just 'then once' to 'than once' the fixup.

- Arnaldo

> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/annotate.c | 49 ++++++++++++++++----------------------
> >  1 file changed, 20 insertions(+), 29 deletions(-)
> > 
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 264a212b47df850c..48fac08c4c9502b1 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -1934,24 +1934,25 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
> >  	return -ENOMEM;
> >  }
> >  
> > -static void __annotation_line__write(struct annotation_line *al, struct annotation *notes,
> > -				     bool first_line, bool current_entry, bool change_color, int width,
> > -				     void *obj, unsigned int percent_type,
> > -				     int  (*obj__set_color)(void *obj, int color),
> > -				     void (*obj__set_percent_color)(void *obj, double percent, bool current),
> > -				     int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current),
> > -				     void (*obj__printf)(void *obj, const char *fmt, ...),
> > -				     void (*obj__write_graph)(void *obj, int graph))
> > -
> > -{
> > -	double percent_max = annotation_line__max_percent(al, percent_type);
> > -	int pcnt_width = annotation__pcnt_width(notes),
> > -	    cycles_width = annotation__cycles_width(notes);
> > +void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> > +			    struct annotation_write_ops *wops)
> > +{
> > +	bool current_entry = wops->current_entry;
> > +	double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
> > +	int width = wops->width;
> > +	int pcnt_width = annotation__pcnt_width(notes);
> > +	int cycles_width = annotation__cycles_width(notes);
> >  	bool show_title = false;
> >  	char bf[256];
> >  	int printed;
> > -
> > -	if (first_line && (al->offset == -1 || percent_max == 0.0)) {
> > +	void *obj = wops->obj;
> > +	int  (*obj__set_color)(void *obj, int color) = wops->set_color;
> > +	void (*obj__set_percent_color)(void *obj, double percent, bool current) = wops->set_percent_color;
> > +	int  (*obj__set_jumps_percent_color)(void *obj, int nr, bool current) = wops->set_jumps_percent_color;
> > +	void (*obj__printf)(void *obj, const char *fmt, ...) = wops->printf;
> > +	void (*obj__write_graph)(void *obj, int graph) = wops->write_graph;
> > +
> > +	if (wops->first_line && (al->offset == -1 || percent_max == 0.0)) {
> >  		if (notes->branch && al->cycles) {
> >  			if (al->cycles->ipc == 0.0 && al->cycles->avg == 0)
> >  				show_title = true;
> > @@ -1965,7 +1966,8 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> >  		for (i = 0; i < al->data_nr; i++) {
> >  			double percent;
> >  
> > -			percent = annotation_data__percent(&al->data[i], percent_type);
> > +			percent = annotation_data__percent(&al->data[i],
> > +							   annotate_opts.percent_type);
> >  
> >  			obj__set_percent_color(obj, percent, current_entry);
> >  			if (symbol_conf.show_total_period) {
> > @@ -2101,10 +2103,10 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> >  			}
> >  		}
> >  
> > -		if (change_color)
> > +		if (wops->change_color)
> >  			color = obj__set_color(obj, HE_COLORSET_ADDR);
> >  		obj__printf(obj, bf);
> > -		if (change_color)
> > +		if (wops->change_color)
> >  			obj__set_color(obj, color);
> >  
> >  		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
> > @@ -2114,17 +2116,6 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati
> >  
> >  }
> >  
> > -void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> > -			    struct annotation_write_ops *wops)
> > -{
> > -	__annotation_line__write(al, notes, wops->first_line, wops->current_entry,
> > -				 wops->change_color, wops->width, wops->obj,
> > -				 annotate_opts.percent_type,
> > -				 wops->set_color, wops->set_percent_color,
> > -				 wops->set_jumps_percent_color, wops->printf,
> > -				 wops->write_graph);
> > -}
> > -
> >  int symbol__annotate2(struct map_symbol *ms, struct evsel *evsel,
> >  		      struct arch **parch)
> >  {
> > -- 
> > 2.49.0

Re: [PATCH 3/6] perf annotate: Pass annotation_print_data to annotaiton_line__write()

[Arnaldo Carvalho de Melo]

There is a typo in the subject: annotaiton_line__write()
                                      ^^

- Arnaldo

On Sat, May 31, 2025 at 11:52:59PM -0700, Namhyung Kim wrote:
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/annotate.c | 13 ++++++++++--
>  tools/perf/util/annotate.c        | 35 +++++++++++++++++--------------
>  tools/perf/util/annotate.h        | 15 +++++++++++--
>  3 files changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index af3e52ce4da62eb9..0057fb485e85aaaf 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -28,6 +28,8 @@ struct annotate_browser {
>  	struct annotation_line	   *selection;
>  	struct arch		   *arch;
>  	struct hist_entry	   *he;
> +	struct debuginfo	   *dbg;
> +	struct evsel		   *evsel;
>  	bool			    searching_backwards;
>  	char			    search_bf[128];
>  };
> @@ -108,12 +110,18 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>  		.printf			 = annotate_browser__printf,
>  		.write_graph		 = annotate_browser__write_graph,
>  	};
> +	struct annotation_print_data apd = {
> +		.he = ab->he,
> +		.arch = ab->arch,
> +		.evsel = ab->evsel,
> +		.dbg = ab->dbg,
> +	};
>  
>  	/* The scroll bar isn't being used */
>  	if (!browser->navkeypressed)
>  		ops.width += 1;
>  
> -	annotation_line__write(al, notes, &ops);
> +	annotation_line__write(al, notes, &ops, &apd);
>  
>  	if (ops.current_entry)
>  		ab->selection = al;
> @@ -902,7 +910,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  			continue;
>  		}
>  		case 'P':
> -			map_symbol__annotation_dump(ms, evsel);
> +			map_symbol__annotation_dump(ms, evsel, browser->he);
>  			continue;
>  		case 't':
>  			if (symbol_conf.show_total_period) {
> @@ -987,6 +995,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>  			.use_navkeypressed = true,
>  		},
>  		.he = he,
> +		.evsel = evsel,
>  	};
>  	struct dso *dso;
>  	int ret = -1, err;
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 48fac08c4c9502b1..7df726b99541a571 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -765,15 +765,6 @@ __hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
>  			    struct debuginfo *dbg, struct disasm_line *dl,
>  			    int *type_offset);
>  
> -struct annotation_print_data {
> -	struct hist_entry *he;
> -	struct evsel *evsel;
> -	struct arch *arch;
> -	struct debuginfo *dbg;
> -	u64 start;
> -	int addr_fmt_width;
> -};
> -
>  static int
>  annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
>  		       struct annotation_options *opts, int printed,
> @@ -1230,7 +1221,6 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
>  	struct annotation_print_data apd = {
>  		.he = he,
>  		.evsel = evsel,
> -		.start = map__rip_2objdump(map, sym->start),
>  	};
>  	int printed = 2, queue_len = 0;
>  	int more = 0;
> @@ -1357,7 +1347,8 @@ static void FILE__write_graph(void *fp, int graph)
>  	fputs(s, fp);
>  }
>  
> -static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
> +static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp,
> +				     struct annotation_print_data *apd)
>  {
>  	struct annotation *notes = symbol__annotation(sym);
>  	struct annotation_write_ops wops = {
> @@ -1374,7 +1365,7 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
>  	list_for_each_entry(al, &notes->src->source, node) {
>  		if (annotation_line__filter(al))
>  			continue;
> -		annotation_line__write(al, notes, &wops);
> +		annotation_line__write(al, notes, &wops, apd);
>  		fputc('\n', fp);
>  		wops.first_line = false;
>  	}
> @@ -1382,13 +1373,18 @@ static int symbol__annotate_fprintf2(struct symbol *sym, FILE *fp)
>  	return 0;
>  }
>  
> -int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
> +int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
> +				struct hist_entry *he)
>  {
>  	const char *ev_name = evsel__name(evsel);
>  	char buf[1024];
>  	char *filename;
>  	int err = -1;
>  	FILE *fp;
> +	struct annotation_print_data apd = {
> +		.he = he,
> +		.evsel = evsel,
> +	};
>  
>  	if (asprintf(&filename, "%s.annotation", ms->sym->name) < 0)
>  		return -1;
> @@ -1404,7 +1400,7 @@ int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel)
>  
>  	fprintf(fp, "%s() %s\nEvent: %s\n\n",
>  		ms->sym->name, dso__long_name(map__dso(ms->map)), ev_name);
> -	symbol__annotate_fprintf2(ms->sym, fp);
> +	symbol__annotate_fprintf2(ms->sym, fp, &apd);
>  
>  	fclose(fp);
>  	err = 0;
> @@ -1655,6 +1651,10 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
>  	struct symbol *sym = ms->sym;
>  	struct rb_root source_line = RB_ROOT;
>  	struct hists *hists = evsel__hists(evsel);
> +	struct annotation_print_data apd = {
> +		.he = he,
> +		.evsel = evsel,
> +	};
>  	char buf[1024];
>  	int err;
>  
> @@ -1677,7 +1677,7 @@ int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
>  	hists__scnprintf_title(hists, buf, sizeof(buf));
>  	fprintf(stdout, "%s, [percent: %s]\n%s() %s\n",
>  		buf, percent_type_str(annotate_opts.percent_type), sym->name, dso__long_name(dso));
> -	symbol__annotate_fprintf2(sym, stdout);
> +	symbol__annotate_fprintf2(sym, stdout, &apd);
>  
>  	annotated_source__purge(symbol__annotation(sym)->src);
>  
> @@ -1935,7 +1935,8 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
>  }
>  
>  void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> -			    struct annotation_write_ops *wops)
> +			    struct annotation_write_ops *wops,
> +			    struct annotation_print_data *apd)
>  {
>  	bool current_entry = wops->current_entry;
>  	double percent_max = annotation_line__max_percent(al, annotate_opts.percent_type);
> @@ -2112,6 +2113,8 @@ void annotation_line__write(struct annotation_line *al, struct annotation *notes
>  		disasm_line__write(disasm_line(al), notes, obj, bf, sizeof(bf), obj__printf, obj__write_graph);
>  
>  		obj__printf(obj, "%-*s", width - pcnt_width - cycles_width - 3 - printed, bf);
> +
> +		(void)apd;
>  	}
>  
>  }
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 9cd723b8b987db1a..bc195bd0429e039d 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -199,8 +199,18 @@ struct annotation_write_ops {
>  	void (*write_graph)(void *obj, int graph);
>  };
>  
> +struct annotation_print_data {
> +	struct hist_entry *he;
> +	struct evsel *evsel;
> +	struct arch *arch;
> +	struct debuginfo *dbg;
> +	u64 start;
> +	int addr_fmt_width;
> +};
> +
>  void annotation_line__write(struct annotation_line *al, struct annotation *notes,
> -			    struct annotation_write_ops *ops);
> +			    struct annotation_write_ops *ops,
> +			    struct annotation_print_data *apd);
>  
>  int __annotation__scnprintf_samples_period(struct annotation *notes,
>  					   char *bf, size_t size,
> @@ -462,7 +472,8 @@ void symbol__annotate_zero_histogram(struct symbol *sym, struct evsel *evsel);
>  void symbol__annotate_decay_histogram(struct symbol *sym, struct evsel *evsel);
>  void annotated_source__purge(struct annotated_source *as);
>  
> -int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel);
> +int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel,
> +				struct hist_entry *he);
>  
>  bool ui__has_annotation(void);
>  
> -- 
> 2.49.0

Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Arnaldo Carvalho de Melo]

On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> Support data type display with a key press so that users can toggle the
> output dynamically on TUI.

'd', 'T', control+d, control+t are all available, why 'y'? :-)

Apart from these minor nits,

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 686fa54e545e275c..cd1d452035a265d3 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  		"b             Toggle percent base [period/hits]\n"
>  		"B             Branch counter abbr list (Optional)\n"
>  		"?             Search string backwards\n"
> -		"f             Toggle showing offsets to full address\n");
> +		"f             Toggle showing offsets to full address\n"
> +		"y             Toggle data type display\n");
>  			continue;
>  		case 'r':
>  			script_browse(NULL, NULL);
> @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  		case 'f':
>  			annotation__toggle_full_addr(notes, ms);
>  			continue;
> +		case 'y':
> +			annotate_opts.code_with_type ^= 1;
> +			if (browser->dbg == NULL)
> +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> +			continue;
>  		case K_LEFT:
>  		case '<':
>  		case '>':
> @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
>  
>  	ret = annotate_browser__run(&browser, evsel, hbt);
>  
> -	if (annotate_opts.code_with_type)
> -		debuginfo__delete(browser.dbg);
> +	debuginfo__delete(browser.dbg);
>  	if(not_annotated)
>  		annotated_source__purge(notes->src);
>  
> -- 
> 2.49.0
> 

Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Arnaldo Carvalho de Melo]

On Thu, Jun 05, 2025 at 05:46:22PM -0300, Arnaldo Carvalho de Melo wrote:
> On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> > Support data type display with a key press so that users can toggle the
> > output dynamically on TUI.
> 
> 'd', 'T', control+d, control+t are all available, why 'y'? :-)
> 
> Apart from these minor nits,

There is one issue I noticed now that Ingo mentioned earlier, if you
press 'y' to get Data type display we better add some visual cue that
this is toggled, like was done for source code view in:

commit c6043d35c0f3eb5bcbeb6309e10c4ae33d203841
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Tue Apr 8 19:03:43 2025 -0300

    perf ui browser annotate: Show in the title the source code view toggle
    
    Ingo reported that having a visual cue if the source code view is
    enabled will help in noticing a bug when no source is presented.
    
    Change the title scnprintf routine for the annotation browser to do
    that.
    
    Suggested-by: Ingo Molnar <mingo@kernel.org>

Also I did it with a simple perf.data file, no data stuff in it,
probably it is best to show that if what is required for it to work is
present in the perf.data file.

Or to tell what is needed for that hotkey/feature to work, which I think
is even better and more informative.

- Arnaldo
 
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> - Arnaldo
>  
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 686fa54e545e275c..cd1d452035a265d3 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		"b             Toggle percent base [period/hits]\n"
> >  		"B             Branch counter abbr list (Optional)\n"
> >  		"?             Search string backwards\n"
> > -		"f             Toggle showing offsets to full address\n");
> > +		"f             Toggle showing offsets to full address\n"
> > +		"y             Toggle data type display\n");
> >  			continue;
> >  		case 'r':
> >  			script_browse(NULL, NULL);
> > @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		case 'f':
> >  			annotation__toggle_full_addr(notes, ms);
> >  			continue;
> > +		case 'y':
> > +			annotate_opts.code_with_type ^= 1;
> > +			if (browser->dbg == NULL)
> > +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> > +			continue;
> >  		case K_LEFT:
> >  		case '<':
> >  		case '>':
> > @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >  
> >  	ret = annotate_browser__run(&browser, evsel, hbt);
> >  
> > -	if (annotate_opts.code_with_type)
> > -		debuginfo__delete(browser.dbg);
> > +	debuginfo__delete(browser.dbg);
> >  	if(not_annotated)
> >  		annotated_source__purge(notes->src);
> >  
> > -- 
> > 2.49.0
> > 

Re: [PATCH 2/6] perf annotate: Remove __annotation_line__write()

[Namhyung Kim]

Hi Arnaldo,

On Thu, Jun 05, 2025 at 04:59:51PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Jun 05, 2025 at 04:55:26PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Sat, May 31, 2025 at 11:52:58PM -0700, Namhyung Kim wrote:
> > > Get rid of the internal function and convert function arguments into
> > > local variables if they are used more then twice.
> > 
> > "more than once"? "twice or more"?
> 
> Well, its "more than twice", as you did:
> 
> -               if (change_color)
> +               if (wops->change_color)
>                         color = obj__set_color(obj, HE_COLORSET_ADDR);
>                 obj__printf(obj, bf);
> -               if (change_color)
> +               if (wops->change_color)
>                         obj__set_color(obj, color);
> 
> Ok, I wouldn't touch this, i.e. would leave all as local variables that
> way the patch would be smaller :-)

I see.

> 
> So its just 'then once' to 'than once' the fixup.

Oops, will fix in v2.

Thanks,
Namhyung

Re: [PATCH 3/6] perf annotate: Pass annotation_print_data to annotaiton_line__write()

[Namhyung Kim]

On Thu, Jun 05, 2025 at 05:44:44PM -0300, Arnaldo Carvalho de Melo wrote:
> There is a typo in the subject: annotaiton_line__write()
>                                       ^^

Will fix, thanks!
Namhyung

Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Namhyung Kim]

On Thu, Jun 05, 2025 at 05:46:22PM -0300, Arnaldo Carvalho de Melo wrote:
> On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> > Support data type display with a key press so that users can toggle the
> > output dynamically on TUI.
> 
> 'd', 'T', control+d, control+t are all available, why 'y'? :-)
> 
> Apart from these minor nits,

Maybe 'T'.  I thought 'y' won't be used by others later but it may be
used by 'y/n' questions.

> 
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks!
Namhyung

>  
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > index 686fa54e545e275c..cd1d452035a265d3 100644
> > --- a/tools/perf/ui/browsers/annotate.c
> > +++ b/tools/perf/ui/browsers/annotate.c
> > @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		"b             Toggle percent base [period/hits]\n"
> >  		"B             Branch counter abbr list (Optional)\n"
> >  		"?             Search string backwards\n"
> > -		"f             Toggle showing offsets to full address\n");
> > +		"f             Toggle showing offsets to full address\n"
> > +		"y             Toggle data type display\n");
> >  			continue;
> >  		case 'r':
> >  			script_browse(NULL, NULL);
> > @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
> >  		case 'f':
> >  			annotation__toggle_full_addr(notes, ms);
> >  			continue;
> > +		case 'y':
> > +			annotate_opts.code_with_type ^= 1;
> > +			if (browser->dbg == NULL)
> > +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> > +			continue;
> >  		case K_LEFT:
> >  		case '<':
> >  		case '>':
> > @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> >  
> >  	ret = annotate_browser__run(&browser, evsel, hbt);
> >  
> > -	if (annotate_opts.code_with_type)
> > -		debuginfo__delete(browser.dbg);
> > +	debuginfo__delete(browser.dbg);
> >  	if(not_annotated)
> >  		annotated_source__purge(notes->src);
> >  
> > -- 
> > 2.49.0
> > 

Re: [PATCH 6/6] perf annotate: Add 'y' hot key to toggle data type display

[Namhyung Kim]

On Thu, Jun 05, 2025 at 06:12:38PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Jun 05, 2025 at 05:46:22PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Sat, May 31, 2025 at 11:53:02PM -0700, Namhyung Kim wrote:
> > > Support data type display with a key press so that users can toggle the
> > > output dynamically on TUI.
> > 
> > 'd', 'T', control+d, control+t are all available, why 'y'? :-)
> > 
> > Apart from these minor nits,
> 
> There is one issue I noticed now that Ingo mentioned earlier, if you
> press 'y' to get Data type display we better add some visual cue that
> this is toggled, like was done for source code view in:
> 
> commit c6043d35c0f3eb5bcbeb6309e10c4ae33d203841
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Tue Apr 8 19:03:43 2025 -0300
> 
>     perf ui browser annotate: Show in the title the source code view toggle
>     
>     Ingo reported that having a visual cue if the source code view is
>     enabled will help in noticing a bug when no source is presented.
>     
>     Change the title scnprintf routine for the annotation browser to do
>     that.
>     
>     Suggested-by: Ingo Molnar <mingo@kernel.org>

Yep, I'll do that as well.

> 
> Also I did it with a simple perf.data file, no data stuff in it,
> probably it is best to show that if what is required for it to work is
> present in the perf.data file.
> 
> Or to tell what is needed for that hotkey/feature to work, which I think
> is even better and more informative.

Sure, I'll add a message that the dso has no DWARF info.

Thanks,
Namhyung

> 
> - Arnaldo
>  
> > Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/ui/browsers/annotate.c | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> > > index 686fa54e545e275c..cd1d452035a265d3 100644
> > > --- a/tools/perf/ui/browsers/annotate.c
> > > +++ b/tools/perf/ui/browsers/annotate.c
> > > @@ -827,7 +827,8 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > >  		"b             Toggle percent base [period/hits]\n"
> > >  		"B             Branch counter abbr list (Optional)\n"
> > >  		"?             Search string backwards\n"
> > > -		"f             Toggle showing offsets to full address\n");
> > > +		"f             Toggle showing offsets to full address\n"
> > > +		"y             Toggle data type display\n");
> > >  			continue;
> > >  		case 'r':
> > >  			script_browse(NULL, NULL);
> > > @@ -947,6 +948,11 @@ static int annotate_browser__run(struct annotate_browser *browser,
> > >  		case 'f':
> > >  			annotation__toggle_full_addr(notes, ms);
> > >  			continue;
> > > +		case 'y':
> > > +			annotate_opts.code_with_type ^= 1;
> > > +			if (browser->dbg == NULL)
> > > +				browser->dbg = debuginfo__new(dso__long_name(map__dso(ms->map)));
> > > +			continue;
> > >  		case K_LEFT:
> > >  		case '<':
> > >  		case '>':
> > > @@ -1035,8 +1041,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> > >  
> > >  	ret = annotate_browser__run(&browser, evsel, hbt);
> > >  
> > > -	if (annotate_opts.code_with_type)
> > > -		debuginfo__delete(browser.dbg);
> > > +	debuginfo__delete(browser.dbg);
> > >  	if(not_annotated)
> > >  		annotated_source__purge(notes->src);
> > >  
> > > -- 
> > > 2.49.0
> > >