From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>,
Kan Liang <kan.liang@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/6] perf annotate-data: Add hist_entry__annotate_data_tui()
Date: Wed, 10 Apr 2024 17:21:01 -0300 [thread overview]
Message-ID: <Zhb0raj1yW8JhuFW@x1> (raw)
In-Reply-To: <20240409235000.1893969-4-namhyung@kernel.org>
On Tue, Apr 09, 2024 at 04:49:57PM -0700, Namhyung Kim wrote:
> Support data type profiling output on TUI.
Added the follow to the commit log message, to make reviewing easier.
As followup patches I think having the DSO name together with the type
is important, also I think we could have a first menu with all the pairs
of DSO/type, sorted top down by the types with most samples, wdyt?
Applied.
- Arnaldo
Committer testing:
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 annotate --data-type
100.00 0 312 struct predicate {
0.00 0 8 PRED_FUNC pred_func;
0.00 8 8 char* p_name;
0.00 16 4 enum predicate_type p_type;
0.00 20 4 enum predicate_precedence p_prec;
0.00 24 1 _Bool side_effects;
0.00 25 1 _Bool no_default_print;
0.00 26 1 _Bool need_stat;
0.00 27 1 _Bool need_type;
0.00 28 1 _Bool need_inum;
0.00 32 4 enum EvaluationCost p_cost;
0.00 36 4 float est_success_rate;
0.00 40 1 _Bool literal_control_chars;
0.00 41 1 _Bool artificial;
0.00 48 8 char* arg_text;
<SNIP>
Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> 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 | 5 +-
> tools/perf/util/annotate-data.h | 5 +-
> 5 files changed, 317 insertions(+), 6 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 99c5dcdfc9df..1cd857400038 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -1814,9 +1814,12 @@ static void print_annotated_data_type(struct annotated_data_type *mem_type,
> printf(";\n");
> }
>
> -void hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel)
> +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");
> +
> + /* move to the next entry */
> + return '>';
> }
> diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
> index 037e2622b7a3..9a6d9b519724 100644
> --- a/tools/perf/util/annotate-data.h
> +++ b/tools/perf/util/annotate-data.h
> @@ -11,6 +11,7 @@ struct annotated_op_loc;
> struct debuginfo;
> struct evsel;
> struct hist_entry;
> +struct hist_browser_timer;
> struct map_symbol;
> struct thread;
>
> @@ -141,7 +142,9 @@ struct annotated_data_stat {
> };
> extern struct annotated_data_stat ann_data_stat;
>
> -void hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel);
> +int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel);
> +int hist_entry__annotate_data_tui(struct hist_entry *he, struct evsel *evsel,
> + struct hist_browser_timer *hbt);
>
> #ifdef HAVE_DWARF_SUPPORT
>
> --
> 2.44.0.478.gd926399ef9-goog
>
next prev parent reply other threads:[~2024-04-10 20:21 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 23:49 [PATCHSET 0/6] perf annotate: Add TUI support for data type profiling (v1) Namhyung Kim
2024-04-09 23:49 ` [PATCH 1/6] perf annotate: Show progress of sample processing Namhyung Kim
2024-04-09 23:49 ` [PATCH 2/6] perf annotate-data: Add hist_entry__annotate_data_tty() Namhyung Kim
2024-04-09 23:49 ` [PATCH 3/6] perf annotate-data: Add hist_entry__annotate_data_tui() Namhyung Kim
2024-04-10 20:21 ` Arnaldo Carvalho de Melo [this message]
2024-04-10 21:04 ` Arnaldo Carvalho de Melo
2024-04-10 21:05 ` Arnaldo Carvalho de Melo
2024-04-10 21:12 ` Arnaldo Carvalho de Melo
2024-04-10 21:17 ` Arnaldo Carvalho de Melo
2024-04-10 21:20 ` Arnaldo Carvalho de Melo
2024-04-10 21:32 ` Arnaldo Carvalho de Melo
2024-04-10 23:49 ` Namhyung Kim
2024-04-09 23:49 ` [PATCH 4/6] perf annotate-data: Support event group display in TUI Namhyung Kim
2024-04-10 19:52 ` Arnaldo Carvalho de Melo
2024-04-10 20:24 ` Arnaldo Carvalho de Melo
2024-04-10 20:38 ` Arnaldo Carvalho de Melo
2024-04-09 23:49 ` [PATCH 5/6] perf report: Add a menu item to annotate data type " Namhyung Kim
2024-04-10 20:46 ` Arnaldo Carvalho de Melo
2024-04-10 20:50 ` Arnaldo Carvalho de Melo
2024-04-11 0:30 ` Namhyung Kim
2024-04-09 23:50 ` [PATCH 6/6] perf report: Do not collect sample histogram unnecessarily Namhyung Kim
2024-04-10 16:29 ` [PATCHSET 0/6] perf annotate: Add TUI support for data type profiling (v1) Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zhb0raj1yW8JhuFW@x1 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.