Re: [PATCH 08/10] name-rev: pre-size buffer in get_parent_name()

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 5, 2020, 03:19 UTC
Message-ID: <9b98835d-d94c-19f5-42ba-d583a33306cc@gmail.com>
In-Reply-To: <c7f22d8d-10f8-0c97-672e-0a50182901e0@web.de>

On 2/4/2020 4:24 PM, René Scharfe wrote:

Show 15 quoted lines

> We can calculate the size of new name easily and precisely. Open-code
> the xstrfmt() calls and grow the buffers as needed before filling them.
> This provides a surprisingly large benefit when working with the
> Chromium repository; here are the numbers measured using hyperfine
> before:
> 
> Benchmark #1: ./git -C ../chromium/src name-rev --all
>   Time (mean ± σ):      5.822 s ±  0.013 s    [User: 5.304 s, System: 0.516 s]
>   Range (min … max):    5.803 s …  5.837 s    10 runs
> 
> ... and with this patch:
> 
> Benchmark #1: ./git -C ../chromium/src name-rev --all
>   Time (mean ± σ):      1.527 s ±  0.003 s    [User: 1.015 s, System: 0.511 s]
>   Range (min … max):    1.524 s …  1.535 s    10 runs

Nice!

> +	if (name->generation > 0) {
> +		strbuf_grow(&sb, len +
> +			    1 + decimal_width(name->generation) +
> +			    1 + decimal_width(parent_number));

Just curious: these strbuf_grow() calls are what _really_ improve the performance, right? If you dropped them, then can we expect the performance to be similar to the old code?

Show 15 quoted lines

> +		strbuf_addf(&sb, "%.*s~%d^%d", (int)len, name->tip_name,
> +			    name->generation, parent_number);
> +	} else {
> +		strbuf_grow(&sb, len +
> +			    1 + decimal_width(parent_number));
> +		strbuf_addf(&sb, "%.*s^%d", (int)len, name->tip_name,
> +			    parent_number);
> +	}
> +	return strbuf_detach(&sb, NULL);
>  }
> 
>  static void name_rev(struct commit *start_commit,
> --
> 2.25.0
>