From 037f1b67e81c2500c94625b089cfd1ecc76a18b5 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 5 Aug 2024 16:46:48 -0700 Subject: perf annotate: Cache debuginfo for data type profiling In find_data_type(), it creates and deletes a debug info whenver it tries to find data type for a sample. This is inefficient and it most likely accesses the same binary again and again. Let's add a single entry cache the debug info structure for the last DSO. Depending on sample data, it usually gives me 2~3x (and sometimes more) speed ups. Note that this will introduce a little difference in the output due to the order of checking stack operations. It used to check the stack ops before checking the availability of debug info but I moved it after the symbol check. So it'll report stack operations in DSOs without debug info as unknown. But I think it's ok and better to have the checking near the caching logic. Committer testing: root@x1:~# perf mem record -a sleep 5s root@x1:~# perf evlist cpu_atom/mem-loads,ldlat=30/P cpu_atom/mem-stores/P dummy:u root@x1:~# diff -u before after --- before 2024-08-08 09:33:53.880780784 -0300 +++ after 2024-08-08 09:35:13.917325041 -0300 @@ -81,8 +81,8 @@ # Overhead Data Type # ........ ......... # - 55.43% (unknown) - 11.61% (stack operation) + 55.56% (unknown) + 11.48% (stack operation) 4.93% struct pcpu_hot 3.26% unsigned int 2.48% struct Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Athira Rajeev Cc: Ian Rogers Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20240805234648.1453689-1-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'tools/perf/util/annotate.c') diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index eafe8d65052e..a87d2e97e10d 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -25,6 +25,7 @@ #include "srcline.h" #include "units.h" #include "debug.h" +#include "debuginfo.h" #include "annotate.h" #include "annotate-data.h" #include "evsel.h" @@ -2333,6 +2334,20 @@ u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset, return map__rip_2objdump(ms->map, addr); } +static struct debuginfo_cache { + struct dso *dso; + struct debuginfo *dbg; +} di_cache; + +void debuginfo_cache__delete(void) +{ + dso__put(di_cache.dso); + di_cache.dso = NULL; + + debuginfo__delete(di_cache.dbg); + di_cache.dbg = NULL; +} + /** * hist_entry__get_data_type - find data type for given hist entry * @he: hist entry @@ -2367,6 +2382,27 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he) return NULL; } + /* + * di_cache holds a pair of values, but code below assumes + * di_cache.dso can be compared/updated and di_cache.dbg can be + * read/updated independently from each other. That assumption only + * holds in single threaded code. + */ + assert(perf_singlethreaded); + + if (map__dso(ms->map) != di_cache.dso) { + dso__put(di_cache.dso); + di_cache.dso = dso__get(map__dso(ms->map)); + + debuginfo__delete(di_cache.dbg); + di_cache.dbg = debuginfo__new(dso__long_name(di_cache.dso)); + } + + if (di_cache.dbg == NULL) { + ann_data_stat.no_dbginfo++; + return NULL; + } + /* Make sure it has the disasm of the function */ if (symbol__annotate(ms, evsel, &arch) < 0) { ann_data_stat.no_insn++; @@ -2411,6 +2447,7 @@ retry: .ip = ms->sym->start + dl->al.offset, .cpumode = he->cpumode, .op = op_loc, + .di = di_cache.dbg, }; if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE) -- cgit