Re: [PATCH 2/3] read-cache: use hashfile instead of git_hash_ctx
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Mar 29, 2021, 19:10 UTC
- Message-ID
- <244d8b93-9d53-d660-ce08-d8b2362f75a8@gmail.com>
- In-Reply-To
- <8b368872-bb36-db61-20d7-47aa5af0ec37@gmail.com>
On 3/29/2021 11:04 AM, Derrick Stolee wrote:
Show 21 quoted lines
> 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