Subject: Re: [PATCH 00/14] refs: batch refname availability checks

From: "brian m. carlson" <[email-masked]>
Date: Tue, 18 Feb 2025 17:10:22 +0000

On 2025-02-17 at 15:50:14, Patrick Steinhardt wrote:
> But more importantly, this refactoring also has a positive effect when
> updating references in a repository with preexisting refs, which I
> consider to be the more realistic scenario. The following benchmark
> creates 10k refs with 100k preexisting refs.
> 
> With the "files" backend we see a modest improvement:
> 
>     Benchmark 1: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = master)
>       Time (mean ± σ):     470.1 ms ±   5.4 ms    [User: 104.5 ms, System: 363.1 ms]
>       Range (min … max):   465.7 ms … 484.3 ms    10 runs
> 
>     Benchmark 2: update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD)
>       Time (mean ± σ):     407.8 ms ±   5.4 ms    [User: 66.0 ms, System: 340.0 ms]
>       Range (min … max):   399.9 ms … 417.6 ms    10 runs
> 
>     Summary
>       update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = HEAD) ran
>         1.15 ± 0.02 times faster than update-ref: create many refs (refformat = files, preexisting = 100000, new = 10000, revision = master)
> 
> But with the "reftable" backend we see an almost 5x improvement, where
> it's now ~15x faster than the "files" backend:
> 
>     Benchmark 1: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = master)
>       Time (mean ± σ):     153.9 ms ±   2.0 ms    [User: 96.5 ms, System: 56.6 ms]
>       Range (min … max):   150.5 ms … 158.4 ms    18 runs
> 
>     Benchmark 2: update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD)
>       Time (mean ± σ):      32.2 ms ±   1.2 ms    [User: 27.6 ms, System: 4.3 ms]
>       Range (min … max):    29.8 ms …  38.6 ms    71 runs
> 
>     Summary
>       update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = HEAD) ran
>         4.78 ± 0.19 times faster than update-ref: create many refs (refformat = reftable, preexisting = 100000, new = 10000, revision = master)

I'm glad to see this performance speedup.  That's a really nice
improvement.

> The series is structured as follows:
> 
>   - Patches 1 to 4 implement the logic to skip ambiguity checks in
>     git-update-ref(1).
> 
>   - Patch 5 and 6 introduce batched checks.
> 
>   - Patch 7 deduplicates the ref prefix checks.
> 
>   - Patch 8 to 14 implement the infrastructure to reseek iterators.
> 
>   - Patch 15 starts to reuse iterators for nested ref checks.

I took a look at this series and I didn't find anything that stood out
to me as a problem.  I will say that the reftable code isn't my forte,
so please don't take this as a formal review, but I am definitely
positive on the series in the general sense.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA