Re: [PATCH 1/4] strbuf: add strbuf_add_uint()

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <l.s.r@web.de> writes:

>> Right now we use unsigned_add_overflows(), and then do the actual add.
>> Using __builtin_add_overflow() might be faster.
> Curious.  Clang and GCC emit the same instructions for our
> unsigned_add_overflows() vs. __builtin_add_overflow() on x64, but clang
> on ARM64 fails to elide the comparison: https://godbolt.org/z/91d35KofM
>
> Which explains why this patch:
>
> --- 8< ---
> diff --git a/strbuf.c b/strbuf.c
> index 3e04addc22..4c2bd1e66f 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -106,12 +106,13 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)
>  void strbuf_grow(struct strbuf *sb, size_t extra)
>  {
>  	int new_buf = !sb->alloc;
> -	if (unsigned_add_overflows(extra, 1) ||
> -	    unsigned_add_overflows(sb->len, extra + 1))
> +	size_t len;
> +	if (__builtin_add_overflow(extra, 1, &len) ||
> +	    __builtin_add_overflow(sb->len, len, &len))
>  		die("you want to use way too much memory");
>  	if (new_buf)
>  		sb->buf = NULL;
> -	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
> +	ALLOC_GROW(sb->buf, len, sb->alloc);
>  	if (new_buf)
>  		sb->buf[0] = '\0';
>  }
> --- >8 ---
>
> ... gives a speedup on my Apple M1 with Apple's clang:
>
> Benchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
>   Time (mean ± σ):     119.6 ms ±   0.2 ms    [User: 112.9 ms, System: 5.6 ms]
>   Range (min … max):   119.3 ms … 120.1 ms    24 runs
>
> Benchmark 2: ./git cat-file --batch-all-objects --batch-check='%(objectname)'
>   Time (mean ± σ):     117.3 ms ±   0.2 ms    [User: 110.4 ms, System: 5.8 ms]
>   Range (min … max):   117.1 ms … 117.6 ms    24 runs
>
> Summary
>   ./git cat-file --batch-all-objects --batch-check='%(objectname)' ran
>     1.02 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
>
> ... but has no effect with GCC 15.2.

Interesting.  A few percent is indeed impressive in such a micro
optimization, and it is satisfying to see that the compilers can see
what our unsigned_add_overflows() plus the actual add does and makes
your above patch a no-op.