2024-04-11 03:33:15

by Namhyung Kim

[permalink] [raw]
Subject: [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2)

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



2024-04-11 03:33:23

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/7] perf annotate: Show progress of sample processing

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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2024-04-11 03:33:46

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/7] perf annotate-data: Add hist_entry__annotate_data_tty()

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

Reviewed-by: Ian Rogers <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2024-04-11 03:34:03

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 4/7] perf annotate-data: Add hist_entry__annotate_data_tui()

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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
---
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


2024-04-11 03:34:09

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 5/7] perf annotate-data: Support event group display in TUI

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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
---
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


2024-04-11 03:34:42

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 6/7] perf report: Add a menu item to annotate data type in TUI

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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2024-04-11 03:35:19

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 7/7] perf report: Do not collect sample histogram unnecessarily

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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2024-04-11 03:55:56

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/7] perf annotate-data: Skip sample histogram for stack canary

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

Fixes: b3c95109c131 ("perf annotate-data: Add stack canary type")
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2024-04-11 11:02:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCHSET 0/7] perf annotate: Add TUI support for data type profiling (v2)

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