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

On 3/29/2021 11:04 AM, Derrick Stolee wrote:

On 3/26/2021 3:12 PM, Derrick Stolee via GitGitGadget wrote:

quoted

 
-	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