On 3/29/2021 11:04 AM, Derrick Stolee wrote:
> On 3/26/2021 3:12 PM, Derrick Stolee via GitGitGadget wrote:
>>
>> - if (ce_flush(&c, newfd, istate->oid.hash))
>> - return -1;
>> + finalize_hashfile(f, istate->oid.hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM);
>> if (close_tempfile_gently(tempfile)) {
>> error(_("could not close '%s'"), get_tempfile_path(tempfile));
>> return -1;
>
> It was bothering me all weekend why this change made index writes
> slower. The reason was this CSUM_FSYNC. Other performance measurement
> (instructions, branches, branch misses, etc.) are all really close to
> the old code, so this I/O is the only explanation. And truly, it is
> the case.
>
> It seems that we avoid an fsync() on the index exactly for this perf
> reason, but we don't on other applications of the hashfile API because
> writes are not critical paths.
>
> I'll update this series in a v2 soon that has some other improvements
> that I noticed while digging deep into things.
Small update: the pull request [1] has the latest changes that I
think will work for these needs. However, they will conflict with
the upcoming changes to ds/sparse-index to use index extensions.
[1] https://github.com/gitgitgadget/git/pull/916
I'll pause this series and re-send on top of ds/sparse-index
after it solidifies.
Thanks,
-Stolee
