Re: [PATCH 2/3] read-cache: use hashfile instead of git_hash_ctx

[Derrick Stolee <stolee@gmail.com>]

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