diff options
author | Namhyung Kim <namhyung@kernel.org> | 2024-08-06 01:46:48 +0200 |
---|---|---|
committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2024-08-08 14:34:43 +0200 |
commit | 037f1b67e81c2500c94625b089cfd1ecc76a18b5 (patch) | |
tree | c211e69ab0968bf2e884f3b5790ff06531ec4fb3 | |
parent | perf hist: Fix reference counting of branch_info (diff) | |
download | linux-037f1b67e81c2500c94625b089cfd1ecc76a18b5.tar.xz linux-037f1b67e81c2500c94625b089cfd1ecc76a18b5.zip |
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 <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20240805234648.1453689-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
-rw-r--r-- | tools/perf/util/annotate-data.c | 15 | ||||
-rw-r--r-- | tools/perf/util/annotate-data.h | 2 | ||||
-rw-r--r-- | tools/perf/util/annotate.c | 37 | ||||
-rw-r--r-- | tools/perf/util/annotate.h | 2 | ||||
-rw-r--r-- | tools/perf/util/session.c | 2 |
5 files changed, 44 insertions, 14 deletions
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c index 734acdd8c4b7..f125ac5f0bda 100644 --- a/tools/perf/util/annotate-data.c +++ b/tools/perf/util/annotate-data.c @@ -1345,16 +1345,9 @@ out: */ struct annotated_data_type *find_data_type(struct data_loc_info *dloc) { - struct annotated_data_type *result = NULL; struct dso *dso = map__dso(dloc->ms->map); Dwarf_Die type_die; - dloc->di = debuginfo__new(dso__long_name(dso)); - if (dloc->di == NULL) { - pr_debug_dtp("cannot get the debug info\n"); - return NULL; - } - /* * The type offset is the same as instruction offset by default. * But when finding a global variable, the offset won't be valid. @@ -1364,13 +1357,9 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc) dloc->fbreg = -1; if (find_data_type_die(dloc, &type_die) < 0) - goto out; - - result = dso__findnew_data_type(dso, &type_die); + return NULL; -out: - debuginfo__delete(dloc->di); - return result; + return dso__findnew_data_type(dso, &type_die); } static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_entries) diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h index 992b7ce4bd11..37a1a3b68e0b 100644 --- a/tools/perf/util/annotate-data.h +++ b/tools/perf/util/annotate-data.h @@ -123,9 +123,9 @@ struct data_loc_info { u64 var_addr; u8 cpumode; struct annotated_op_loc *op; + struct debuginfo *di; /* These are used internally */ - struct debuginfo *di; int fbreg; bool fb_cfa; 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) diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 64e70d716ff1..0e52558baa01 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -543,4 +543,6 @@ struct annotated_basic_block { int annotate_get_basic_blocks(struct symbol *sym, s64 src, s64 dst, struct list_head *head); +void debuginfo_cache__delete(void); + #endif /* __PERF_ANNOTATE_H */ diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 5596bed1b8c8..f9072e003367 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -36,6 +36,7 @@ #include "util.h" #include "arch/common.h" #include "units.h" +#include "annotate.h" #include <internal/lib.h> #ifdef HAVE_ZSTD_SUPPORT @@ -304,6 +305,7 @@ void perf_session__delete(struct perf_session *session) return; auxtrace__free(session); auxtrace_index__free(&session->auxtrace_index); + debuginfo_cache__delete(); perf_session__destroy_kernel_maps(session); perf_decomp__release_events(session->decomp_data.decomp); perf_env__exit(&session->header.env); |