| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aKZADpgsywwnXfnF@x1>
Date: Wed, 20 Aug 2025 18:37:18 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v5 12/12] perf annotate: Use a hashmap to save type data
On Fri, Aug 15, 2025 at 08:16:35PM -0700, Namhyung Kim wrote:
> It can slowdown annotation browser if objdump is processing large DWARF
> data. Let's add a hashmap to save the data type info for each line.
>
> Note that this is needed for TUI only because stdio only processes each
> line once. TUI will display the same line whenever it refreshes the
> screen.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/ui/browsers/annotate.c | 34 ++++++++++++++++++++++++++++++-
> tools/perf/util/annotate.c | 27 ++++++++++++++++++++++--
> tools/perf/util/annotate.h | 2 ++
> 3 files changed, 60 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index 9aa3c1ba22f52789..9677a3763a290a3d 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -6,6 +6,7 @@
> #include "../../util/debug.h"
> #include "../../util/debuginfo.h"
> #include "../../util/dso.h"
> +#include "../../util/hashmap.h"
> #include "../../util/hist.h"
> #include "../../util/sort.h"
> #include "../../util/map.h"
> @@ -15,6 +16,7 @@
> #include "../../util/evlist.h"
> #include "../../util/thread.h"
> #include <inttypes.h>
> +#include <linux/err.h>
> #include <linux/kernel.h>
> #include <linux/string.h>
> #include <linux/zalloc.h>
> @@ -36,6 +38,7 @@ struct annotate_browser {
> struct hist_entry *he;
> struct debuginfo *dbg;
> struct evsel *evsel;
> + struct hashmap *type_hash;
> bool searching_backwards;
> char search_bf[128];
> };
> @@ -43,6 +46,16 @@ struct annotate_browser {
> /* A copy of target hist_entry for perf top. */
> static struct hist_entry annotate_he;
>
> +static size_t type_hash(long key, void *ctx __maybe_unused)
> +{
> + return key;
> +}
> +
> +static bool type_equal(long key1, long key2, void *ctx __maybe_unused)
> +{
> + return key1 == key2;
> +}
> +
> static inline struct annotation *browser__annotation(struct ui_browser *browser)
> {
> struct map_symbol *ms = browser->priv;
> @@ -130,6 +143,9 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
> if (!browser->navkeypressed)
> ops.width += 1;
>
> + if (!IS_ERR_OR_NULL(ab->type_hash))
> + apd.type_hash = ab->type_hash;
> +
> annotation_line__write(al, notes, &ops, &apd);
>
> if (ops.current_entry)
> @@ -1051,6 +1067,10 @@ static int annotate_browser__run(struct annotate_browser *browser,
> annotate_opts.code_with_type ^= 1;
> if (browser->dbg == NULL)
> browser->dbg = dso__debuginfo(map__dso(ms->map));
> + if (browser->type_hash == NULL) {
> + browser->type_hash = hashmap__new(type_hash, type_equal,
> + /*ctx=*/NULL);
> + }
> annotate_browser__show(&browser->b, title, help);
> annotate_browser__debuginfo_warning(browser);
> continue;
> @@ -1145,8 +1165,10 @@ 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)
> + if (annotate_opts.code_with_type) {
> browser.dbg = dso__debuginfo(dso);
> + browser.type_hash = hashmap__new(type_hash, type_equal, /*ctx=*/NULL);
> + }
>
> browser.b.width = notes->src->widths.max_line_len;
> browser.b.nr_entries = notes->src->nr_entries;
> @@ -1159,6 +1181,16 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms,
> ret = annotate_browser__run(&browser, evsel, hbt);
>
> debuginfo__delete(browser.dbg);
> +
> + if (!IS_ERR_OR_NULL(browser.type_hash)) {
> + struct hashmap_entry *cur;
> + size_t bkt;
> +
> + hashmap__for_each_entry(browser.type_hash, cur, bkt)
> + free(cur->pvalue);
zfree(&cur->pvalue);
> + hashmap__free(browser.type_hash);
> + }
> +
> if (not_annotated && !notes->src->tried_source)
> annotated_source__purge(notes->src);
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bea3457a00632fd7..77414e04d99bb4f2 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1954,11 +1954,17 @@ int annotation_br_cntr_entry(char **str, int br_cntr_nr,
> return -ENOMEM;
> }
>
> +struct type_hash_entry {
> + struct annotated_data_type *type;
> + int offset;
> +};
> +
> static int disasm_line__snprint_type_info(struct disasm_line *dl,
> char *buf, int len,
> struct annotation_print_data *apd)
> {
> - struct annotated_data_type *data_type;
> + struct annotated_data_type *data_type = NULL;
> + struct type_hash_entry *entry = NULL;
> char member[256];
> int offset = 0;
> int printed;
> @@ -1968,7 +1974,24 @@ static int disasm_line__snprint_type_info(struct disasm_line *dl,
> if (!annotate_opts.code_with_type || apd->dbg == NULL)
> return 1;
>
> - data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
> + if (apd->type_hash) {
> + hashmap__find(apd->type_hash, dl->al.offset, &entry);
> + if (entry != NULL) {
> + data_type = entry->type;
> + offset = entry->offset;
> + }
> + }
> + if (data_type == NULL)
> + data_type = __hist_entry__get_data_type(apd->he, apd->arch, apd->dbg, dl, &offset);
add space?
> + if (apd->type_hash && entry == NULL) {
> + entry = malloc(sizeof(*entry));
Is the 'entry' variable needed anywhere else? Not, so could be declared
here to save a line at the start of the function. Or is it used in a
later patch outside of this scope?
> + if (entry != NULL) {
> + entry->type = data_type;
> + entry->offset = offset;
> + hashmap__add(apd->type_hash, dl->al.offset, entry);
> + }
> + }
> +
> if (!needs_type_info(data_type))
> return 1;
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 86e858f5bf173152..eaf6c8aa7f473959 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -204,6 +204,8 @@ struct annotation_print_data {
> struct evsel *evsel;
> struct arch *arch;
> struct debuginfo *dbg;
> + /* save data type info keyed by al->offset */
> + struct hashmap *type_hash;
> /* It'll be set in hist_entry__annotate_printf() */
> int addr_fmt_width;
> };
> --
> 2.50.1
>
Powered by blists - more mailing lists