Re: [PATCH v2] packed-refs: use `fwrite()` when passing refs verbatim
Toon Claes <toon@iotcl.com> writes:
Show 17 quoted lines
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The `write_with_updates()` function uses a `struct ref_iterator` to
>> iterate over all refs to write to the temporary packed-refs file. It
>> receives the iterator from `packed_ref_iterator_begin()` which takes a
>> snapshot of the 'packed-refs' file.
>>
>> While writing to the new packed-refs file, writes are routed via
>> `write_packed_entry()` which uses `fprintf()`. Even for references which
>> haven't changed, we use the same mechanism. Instead, let's track the
>> position of unchanged references in the snapshot iterator and directly
>> use `fwrite()`.
>>
>> With this, any sanitation which was happening as a side of reformatting
>
> s/side/side effect/ ?
>
Show 32 quoted lines
>> is now lost. But that was never the job of this section of the code,
>> since the main intention is to simply rewrite the remaining refs post
>> deletion of the selective few.
>
> Agreed.
>
>>
>> This removes the unnecessary formatting operation involved. We can see a
>> consistent ~20% performance improvement when deleting from packed
>> references.
>>
>> Benchmark 1: update-ref: delete ref (refcount = 100000, revision = master)
>> Time (mean ± σ): 28.7 ms ± 1.7 ms [User: 22.5 ms, System: 5.9 ms]
>> Range (min … max): 26.7 ms … 33.3 ms 46 runs
>>
>> Benchmark 2: update-ref: delete ref (refcount = 100000, revision = b4/kn-speedup-packed-refs)
>> Time (mean ± σ): 23.8 ms ± 1.2 ms [User: 17.5 ms, System: 6.0 ms]
>> Range (min … max): 22.1 ms … 27.7 ms 56 runs
>>
>> Summary
>> update-ref: delete ref (refcount = 100000, revision = b4/kn-speedup-packed-refs) ran
>> 1.21 ± 0.09 times faster than update-ref: delete ref (refformat = files, refcount = 100000, revision = master)
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> Changes in v2:
>> - Instead of using the existing function, introduce a new
>> `write_packed_entry_raw()`.
>> - Modify the commit to also note that we lose sanitization.
>
> s/commit/commit message/
>
This doesn't go into the commit itself, so I'll leave it as is.
Show 7 quoted lines
>> - Link to v1: https://patch.msgid.link/20260930-kn-speedup-packed-refs-v1-1-111cd03d9b0e@gmail.com
>
> Okay, I'm okay with this version.
>
> --
> Laters,
> Toon
Karthik Nayak <karthik.188@gmail.com>