perf srcline: Use long-running addr2line per DSO
authorTony Garnock-Jones <tonyg@leastfixedpoint.com>
Thu, 16 Sep 2021 12:09:39 +0000 (14:09 +0200)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Mon, 4 Oct 2021 12:29:07 +0000 (09:29 -0300)
Invoking addr2line in a separate subprocess, one for each required
lookup, takes a terribly long time.

This patch introduces a long-running addr2line process for each DSO,
*DRAMATICALLY* speeding up runs of perf.

What used to take tens of minutes now takes tens of seconds.

Debian bug report about this issue:

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=911815

Signed-off-by: Tony Garnock-Jones <tonyg@leastfixedpoint.com>
Tested-by: Ian Rogers <irogers@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20210916120939.453536-1-tonyg@leastfixedpoint.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/util/srcline.c

index 5b7d6c16d33fe80b3af83cdcd6adbe197ed1c455..af468e3bb6fabc6acda557e63a8c9211b0437827 100644 (file)
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <inttypes.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/types.h>
 
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -15,6 +17,7 @@
 #include "srcline.h"
 #include "string2.h"
 #include "symbol.h"
+#include "subcmd/run-command.h"
 
 bool srcline_full_filename;
 
@@ -119,6 +122,8 @@ static struct symbol *new_inline_sym(struct dso *dso,
        return inline_sym;
 }
 
+#define MAX_INLINE_NEST 1024
+
 #ifdef HAVE_LIBBFD_SUPPORT
 
 /*
@@ -273,8 +278,6 @@ static void addr2line_cleanup(struct a2l_data *a2l)
        free(a2l);
 }
 
-#define MAX_INLINE_NEST 1024
-
 static int inline_list__append_dso_a2l(struct dso *dso,
                                       struct inline_node *node,
                                       struct symbol *sym)
@@ -361,26 +364,14 @@ void dso__free_a2l(struct dso *dso)
        dso->a2l = NULL;
 }
 
-static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
-                                       struct dso *dso, struct symbol *sym)
-{
-       struct inline_node *node;
-
-       node = zalloc(sizeof(*node));
-       if (node == NULL) {
-               perror("not enough memory for the inline node");
-               return NULL;
-       }
-
-       INIT_LIST_HEAD(&node->val);
-       node->addr = addr;
-
-       addr2line(dso_name, addr, NULL, NULL, dso, true, node, sym);
-       return node;
-}
-
 #else /* HAVE_LIBBFD_SUPPORT */
 
+struct a2l_subprocess {
+       struct child_process addr2line;
+       FILE *to_child;
+       FILE *from_child;
+};
+
 static int filename_split(char *filename, unsigned int *line_nr)
 {
        char *sep;
@@ -402,114 +393,285 @@ static int filename_split(char *filename, unsigned int *line_nr)
        return 0;
 }
 
-static int addr2line(const char *dso_name, u64 addr,
-                    char **file, unsigned int *line_nr,
-                    struct dso *dso __maybe_unused,
-                    bool unwind_inlines __maybe_unused,
-                    struct inline_node *node __maybe_unused,
-                    struct symbol *sym __maybe_unused)
+static void addr2line_subprocess_cleanup(struct a2l_subprocess *a2l)
 {
-       FILE *fp;
-       char cmd[PATH_MAX];
-       char *filename = NULL;
-       size_t len;
-       int ret = 0;
+       if (a2l->addr2line.pid != -1) {
+               kill(a2l->addr2line.pid, SIGKILL);
+               finish_command(&a2l->addr2line); /* ignore result, we don't care */
+               a2l->addr2line.pid = -1;
+       }
 
-       scnprintf(cmd, sizeof(cmd), "addr2line -e %s %016"PRIx64,
-                 dso_name, addr);
+       if (a2l->to_child != NULL) {
+               fclose(a2l->to_child);
+               a2l->to_child = NULL;
+       }
 
-       fp = popen(cmd, "r");
-       if (fp == NULL) {
-               pr_warning("popen failed for %s\n", dso_name);
-               return 0;
+       if (a2l->from_child != NULL) {
+               fclose(a2l->from_child);
+               a2l->from_child = NULL;
+       }
+
+       free(a2l);
+}
+
+static struct a2l_subprocess *addr2line_subprocess_init(const char *path)
+{
+       const char *argv[] = { "addr2line", "-e", path, "-i", "-f", NULL };
+       struct a2l_subprocess *a2l = zalloc(sizeof(*a2l));
+       int start_command_status = 0;
+
+       if (a2l == NULL)
+               goto out;
+
+       a2l->to_child = NULL;
+       a2l->from_child = NULL;
+
+       a2l->addr2line.pid = -1;
+       a2l->addr2line.in = -1;
+       a2l->addr2line.out = -1;
+       a2l->addr2line.no_stderr = 1;
+
+       a2l->addr2line.argv = argv;
+       start_command_status = start_command(&a2l->addr2line);
+       a2l->addr2line.argv = NULL; /* it's not used after start_command; avoid dangling pointers */
+
+       if (start_command_status != 0) {
+               pr_warning("could not start addr2line for %s: start_command return code %d\n",
+                          path,
+                          start_command_status);
+               goto out;
        }
 
-       if (getline(&filename, &len, fp) < 0 || !len) {
-               pr_warning("addr2line has no output for %s\n", dso_name);
+       a2l->to_child = fdopen(a2l->addr2line.in, "w");
+       if (a2l->to_child == NULL) {
+               pr_warning("could not open write-stream to addr2line of %s\n", path);
                goto out;
        }
 
-       ret = filename_split(filename, line_nr);
-       if (ret != 1) {
-               free(filename);
+       a2l->from_child = fdopen(a2l->addr2line.out, "r");
+       if (a2l->from_child == NULL) {
+               pr_warning("could not open read-stream from addr2line of %s\n", path);
                goto out;
        }
 
-       *file = filename;
+       return a2l;
 
 out:
-       pclose(fp);
-       return ret;
+       if (a2l)
+               addr2line_subprocess_cleanup(a2l);
+
+       return NULL;
 }
 
-void dso__free_a2l(struct dso *dso __maybe_unused)
+static int read_addr2line_record(struct a2l_subprocess *a2l,
+                                char **function,
+                                char **filename,
+                                unsigned int *line_nr)
 {
+       /*
+        * Returns:
+        * -1 ==> error
+        * 0 ==> sentinel (or other ill-formed) record read
+        * 1 ==> a genuine record read
+        */
+       char *line = NULL;
+       size_t line_len = 0;
+       unsigned int dummy_line_nr = 0;
+       int ret = -1;
+
+       if (function != NULL)
+               zfree(function);
+
+       if (filename != NULL)
+               zfree(filename);
+
+       if (line_nr != NULL)
+               *line_nr = 0;
+
+       if (getline(&line, &line_len, a2l->from_child) < 0 || !line_len)
+               goto error;
+
+       if (function != NULL)
+               *function = strdup(strim(line));
+
+       zfree(&line);
+       line_len = 0;
+
+       if (getline(&line, &line_len, a2l->from_child) < 0 || !line_len)
+               goto error;
+
+       if (filename_split(line, line_nr == NULL ? &dummy_line_nr : line_nr) == 0) {
+               ret = 0;
+               goto error;
+       }
+
+       if (filename != NULL)
+               *filename = strdup(line);
+
+       zfree(&line);
+       line_len = 0;
+
+       return 1;
+
+error:
+       free(line);
+       if (function != NULL)
+               zfree(function);
+       if (filename != NULL)
+               zfree(filename);
+       return ret;
 }
 
-static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
-                                       struct dso *dso __maybe_unused,
-                                       struct symbol *sym)
+static int inline_list__append_record(struct dso *dso,
+                                     struct inline_node *node,
+                                     struct symbol *sym,
+                                     const char *function,
+                                     const char *filename,
+                                     unsigned int line_nr)
 {
-       FILE *fp;
-       char cmd[PATH_MAX];
-       struct inline_node *node;
-       char *filename = NULL;
-       char *funcname = NULL;
-       size_t filelen, funclen;
-       unsigned int line_nr = 0;
+       struct symbol *inline_sym = new_inline_sym(dso, sym, function);
 
-       scnprintf(cmd, sizeof(cmd), "addr2line -e %s -i -f %016"PRIx64,
-                 dso_name, addr);
+       return inline_list__append(inline_sym, srcline_from_fileline(filename, line_nr), node);
+}
 
-       fp = popen(cmd, "r");
-       if (fp == NULL) {
-               pr_err("popen failed for %s\n", dso_name);
-               return NULL;
+static int addr2line(const char *dso_name, u64 addr,
+                    char **file, unsigned int *line_nr,
+                    struct dso *dso,
+                    bool unwind_inlines,
+                    struct inline_node *node,
+                    struct symbol *sym __maybe_unused)
+{
+       struct a2l_subprocess *a2l = dso->a2l;
+       char *record_function = NULL;
+       char *record_filename = NULL;
+       unsigned int record_line_nr = 0;
+       int record_status = -1;
+       int ret = 0;
+       size_t inline_count = 0;
+
+       if (!a2l) {
+               dso->a2l = addr2line_subprocess_init(dso_name);
+               a2l = dso->a2l;
        }
 
-       node = zalloc(sizeof(*node));
-       if (node == NULL) {
-               perror("not enough memory for the inline node");
+       if (a2l == NULL) {
+               if (!symbol_conf.disable_add2line_warn)
+                       pr_warning("%s %s: addr2line_subprocess_init failed\n", __func__, dso_name);
                goto out;
        }
 
-       INIT_LIST_HEAD(&node->val);
-       node->addr = addr;
-
-       /* addr2line -f generates two lines for each inlined functions */
-       while (getline(&funcname, &funclen, fp) != -1) {
-               char *srcline;
-               struct symbol *inline_sym;
+       /*
+        * Send our request and then *deliberately* send something that can't be interpreted as
+        * a valid address to ask addr2line about (namely, ","). This causes addr2line to first
+        * write out the answer to our request, in an unbounded/unknown number of records, and
+        * then to write out the lines "??" and "??:0", so that we can detect when it has
+        * finished giving us anything useful. We have to be careful about the first record,
+        * though, because it may be genuinely unknown, in which case we'll get two sets of
+        * "??"/"??:0" lines.
+        */
+       if (fprintf(a2l->to_child, "%016"PRIx64"\n,\n", addr) < 0 || fflush(a2l->to_child) != 0) {
+               pr_warning("%s %s: could not send request\n", __func__, dso_name);
+               goto out;
+       }
 
-               strim(funcname);
+       switch (read_addr2line_record(a2l, &record_function, &record_filename, &record_line_nr)) {
+       case -1:
+               pr_warning("%s %s: could not read first record\n", __func__, dso_name);
+               goto out;
+       case 0:
+               /*
+                * The first record was invalid, so return failure, but first read another
+                * record, since we asked a junk question and have to clear the answer out.
+                */
+               switch (read_addr2line_record(a2l, NULL, NULL, NULL)) {
+               case -1:
+                       pr_warning("%s %s: could not read delimiter record\n", __func__, dso_name);
+                       break;
+               case 0:
+                       /* As expected. */
+                       break;
+               default:
+                       pr_warning("%s %s: unexpected record instead of sentinel",
+                                  __func__, dso_name);
+                       break;
+               }
+               goto out;
+       default:
+               break;
+       }
 
-               if (getline(&filename, &filelen, fp) == -1)
-                       goto out;
+       if (file) {
+               *file = strdup(record_filename);
+               ret = 1;
+       }
+       if (line_nr)
+               *line_nr = record_line_nr;
 
-               if (filename_split(filename, &line_nr) != 1)
+       if (unwind_inlines) {
+               if (node && inline_list__append_record(dso, node, sym,
+                                                      record_function,
+                                                      record_filename,
+                                                      record_line_nr)) {
+                       ret = 0;
                        goto out;
+               }
+       }
 
-               srcline = srcline_from_fileline(filename, line_nr);
-               inline_sym = new_inline_sym(dso, sym, funcname);
-
-               if (inline_list__append(inline_sym, srcline, node) != 0) {
-                       free(srcline);
-                       if (inline_sym && inline_sym->inlined)
-                               symbol__delete(inline_sym);
-                       goto out;
+       /* We have to read the records even if we don't care about the inline info. */
+       while ((record_status = read_addr2line_record(a2l,
+                                                     &record_function,
+                                                     &record_filename,
+                                                     &record_line_nr)) == 1) {
+               if (unwind_inlines && node && inline_count++ < MAX_INLINE_NEST) {
+                       if (inline_list__append_record(dso, node, sym,
+                                                      record_function,
+                                                      record_filename,
+                                                      record_line_nr)) {
+                               ret = 0;
+                               goto out;
+                       }
+                       ret = 1; /* found at least one inline frame */
                }
        }
 
 out:
-       pclose(fp);
-       free(filename);
-       free(funcname);
+       free(record_function);
+       free(record_filename);
+       return ret;
+}
 
-       return node;
+void dso__free_a2l(struct dso *dso)
+{
+       struct a2l_subprocess *a2l = dso->a2l;
+
+       if (!a2l)
+               return;
+
+       addr2line_subprocess_cleanup(a2l);
+
+       dso->a2l = NULL;
 }
 
 #endif /* HAVE_LIBBFD_SUPPORT */
 
+static struct inline_node *addr2inlines(const char *dso_name, u64 addr,
+                                       struct dso *dso, struct symbol *sym)
+{
+       struct inline_node *node;
+
+       node = zalloc(sizeof(*node));
+       if (node == NULL) {
+               perror("not enough memory for the inline node");
+               return NULL;
+       }
+
+       INIT_LIST_HEAD(&node->val);
+       node->addr = addr;
+
+       addr2line(dso_name, addr, NULL, NULL, dso, true, node, sym);
+       return node;
+}
+
 /*
  * Number of addr2line failures (without success) before disabling it for that
  * dso.