Commit 037f1b67 authored by Namhyung Kim's avatar Namhyung Kim Committed by Arnaldo Carvalho de Melo

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: default avatarNamhyung Kim <namhyung@kernel.org>
Tested-by: default avatarArnaldo 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.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent b2f70c99
...@@ -1345,16 +1345,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die) ...@@ -1345,16 +1345,9 @@ static int find_data_type_die(struct data_loc_info *dloc, Dwarf_Die *type_die)
*/ */
struct annotated_data_type *find_data_type(struct data_loc_info *dloc) 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); struct dso *dso = map__dso(dloc->ms->map);
Dwarf_Die type_die; 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. * The type offset is the same as instruction offset by default.
* But when finding a global variable, the offset won't be valid. * 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) ...@@ -1364,13 +1357,9 @@ struct annotated_data_type *find_data_type(struct data_loc_info *dloc)
dloc->fbreg = -1; dloc->fbreg = -1;
if (find_data_type_die(dloc, &type_die) < 0) if (find_data_type_die(dloc, &type_die) < 0)
goto out; return NULL;
result = dso__findnew_data_type(dso, &type_die);
out: return dso__findnew_data_type(dso, &type_die);
debuginfo__delete(dloc->di);
return result;
} }
static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_entries) static int alloc_data_type_histograms(struct annotated_data_type *adt, int nr_entries)
......
...@@ -123,9 +123,9 @@ struct data_loc_info { ...@@ -123,9 +123,9 @@ struct data_loc_info {
u64 var_addr; u64 var_addr;
u8 cpumode; u8 cpumode;
struct annotated_op_loc *op; struct annotated_op_loc *op;
struct debuginfo *di;
/* These are used internally */ /* These are used internally */
struct debuginfo *di;
int fbreg; int fbreg;
bool fb_cfa; bool fb_cfa;
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "srcline.h" #include "srcline.h"
#include "units.h" #include "units.h"
#include "debug.h" #include "debug.h"
#include "debuginfo.h"
#include "annotate.h" #include "annotate.h"
#include "annotate-data.h" #include "annotate-data.h"
#include "evsel.h" #include "evsel.h"
...@@ -2333,6 +2334,20 @@ u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset, ...@@ -2333,6 +2334,20 @@ u64 annotate_calc_pcrel(struct map_symbol *ms, u64 ip, int offset,
return map__rip_2objdump(ms->map, addr); 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 * hist_entry__get_data_type - find data type for given hist entry
* @he: hist entry * @he: hist entry
...@@ -2367,6 +2382,27 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he) ...@@ -2367,6 +2382,27 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
return NULL; 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 */ /* Make sure it has the disasm of the function */
if (symbol__annotate(ms, evsel, &arch) < 0) { if (symbol__annotate(ms, evsel, &arch) < 0) {
ann_data_stat.no_insn++; ann_data_stat.no_insn++;
...@@ -2411,6 +2447,7 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he) ...@@ -2411,6 +2447,7 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
.ip = ms->sym->start + dl->al.offset, .ip = ms->sym->start + dl->al.offset,
.cpumode = he->cpumode, .cpumode = he->cpumode,
.op = op_loc, .op = op_loc,
.di = di_cache.dbg,
}; };
if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE) if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE)
......
...@@ -543,4 +543,6 @@ struct annotated_basic_block { ...@@ -543,4 +543,6 @@ struct annotated_basic_block {
int annotate_get_basic_blocks(struct symbol *sym, s64 src, s64 dst, int annotate_get_basic_blocks(struct symbol *sym, s64 src, s64 dst,
struct list_head *head); struct list_head *head);
void debuginfo_cache__delete(void);
#endif /* __PERF_ANNOTATE_H */ #endif /* __PERF_ANNOTATE_H */
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
#include "util.h" #include "util.h"
#include "arch/common.h" #include "arch/common.h"
#include "units.h" #include "units.h"
#include "annotate.h"
#include <internal/lib.h> #include <internal/lib.h>
#ifdef HAVE_ZSTD_SUPPORT #ifdef HAVE_ZSTD_SUPPORT
...@@ -304,6 +305,7 @@ void perf_session__delete(struct perf_session *session) ...@@ -304,6 +305,7 @@ void perf_session__delete(struct perf_session *session)
return; return;
auxtrace__free(session); auxtrace__free(session);
auxtrace_index__free(&session->auxtrace_index); auxtrace_index__free(&session->auxtrace_index);
debuginfo_cache__delete();
perf_session__destroy_kernel_maps(session); perf_session__destroy_kernel_maps(session);
perf_decomp__release_events(session->decomp_data.decomp); perf_decomp__release_events(session->decomp_data.decomp);
perf_env__exit(&session->header.env); perf_env__exit(&session->header.env);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment