Re: [PATCH 2/3] read-cache: use hashfile instead of git_hash_ctx
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Mar 29, 2021, 15:04 UTC
- Message-ID
- <8b368872-bb36-db61-20d7-47aa5af0ec37@gmail.com>
- In-Reply-To
- <e2611bbc007a4583af55e006136c2bc74351a7de.1616785928.git.gitgitgadget@gmail.com>
On 3/26/2021 3:12 PM, Derrick Stolee via GitGitGadget wrote:
Show 7 quoted lines
>
> - 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.
Thanks, -Stolee