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

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

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