Re: [PATCH 1/5] perf annotate: avoid reallocation in objdump parsing

Arnaldo Carvalho de Melo
2019-10-14T14:15:09Z

Em Thu, Oct 10, 2019 at 11:36:45AM -0700, Ian Rogers escreveu:
> Objdump output is parsed using getline which allocates memory for the
> read. Getline will realloc if the memory is too small, but currently the
> line is always freed after the call.
> Simplify parse_objdump_line by performing the reading in symbol__disassemble.

Thanks, applied.

- Arnaldo

> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/annotate.c | 26 ++++++++++++++------------
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index e830eadfca2a..1487849a191a 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1485,24 +1485,17 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
> * means that it's not a disassembly line so should be treated differently.
> * The ops.raw part will be parsed further according to type of the instruction.
> */
> -static int symbol__parse_objdump_line(struct symbol *sym, FILE *file,
> +static int symbol__parse_objdump_line(struct symbol *sym,
> struct annotate_args *args,
> - int *line_nr)
> + char *line, int *line_nr)
> {
> struct map *map = args->ms.map;
> struct annotation *notes = symbol__annotation(sym);
> struct disasm_line *dl;
> - char *line = NULL, *parsed_line, *tmp, *tmp2;
> - size_t line_len;
> + char *parsed_line, *tmp, *tmp2;
> s64 line_ip, offset = -1;
> regmatch_t match[2];
>
> - if (getline(&line, &line_len, file) < 0)
> - return -1;
> -
> - if (!line)
> - return -1;
> -
> line_ip = -1;
> parsed_line = strim(line);
>
> @@ -1539,7 +1532,6 @@ static int symbol__parse_objdump_line(struct symbol *sym, FILE *file,
> args->ms.sym = sym;
>
> dl = disasm_line__new(args);
> - free(line);
> (*line_nr)++;
>
> if (dl == NULL)
> @@ -1855,6 +1847,8 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> int lineno = 0;
> int nline;
> pid_t pid;
> + char *line;
> + size_t line_len;
> int err = dso__disassemble_filename(dso, symfs_filename, sizeof(symfs_filename));
>
> if (err)
> @@ -1943,18 +1937,26 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> goto out_free_command;
> }
>
> + /* Storage for getline. */
> + line = NULL;
> + line_len = 0;
> +
> nline = 0;
> while (!feof(file)) {
> + if (getline(&line, &line_len, file) < 0 || !line)
> + break;
> +
> /*
> * The source code line number (lineno) needs to be kept in
> * across calls to symbol__parse_objdump_line(), so that it
> * can associate it with the instructions till the next one.
> * See disasm_line__new() and struct disasm_line::line_nr.
> */
> - if (symbol__parse_objdump_line(sym, file, args, &lineno) < 0)
> + if (symbol__parse_objdump_line(sym, args, line, &lineno) < 0)
> break;
> nline++;
> }
> + free(line);
>
> if (nline == 0)
> pr_err("No output from %s\n", command);
> --
> 2.23.0.581.g78d2f28ef7-goog

--

- Arnaldo