Re: [PATCH 02/30] read-cache: add index.computeHash config option

>
> From: Derrick Stolee <derrickstolee@github.com>
>
> The previous change allowed skipping the hashing portion of the
> hashwrite API, using it instead as a buffered write API. Disabling the
> hashwrite can be particularly helpful when the write operation is in a
> critical path.
>
> One such critical path is the writing of the index. This operation is so
> critical that the sparse index was created specifically to reduce the
> size of the index to make these writes (and reads) faster.
>
> Following a similar approach to one used in the microsoft/git fork [1],
> add a new config option that allows disabling this hashing during the
> index write. The cost is that we can no longer validate the contents for
> corruption-at-rest using the trailing hash.
>
> [1] https://github.com/microsoft/git/commit/21fed2d91410f45d85279467f21d717a2db45201
>
> While older Git versions will not recognize the null hash as a special
> case, the file format itself is still being met in terms of its
> structure. Using this null hash will still allow Git operations to
> function across older versions.
>
> The one exception is 'git fsck' which checks the hash of the index file.
> Here, we disable this check if the trailing hash is all zeroes. We add a
> warning to the config option that this may cause undesirable behavior
> with older Git versions.
>
> As a quick comparison, I tested 'git update-index --force-write' with
> and without index.computHash=false on a copy of the Linux kernel
> repository.
>
> Benchmark 1: with hash
>   Time (mean ± σ):      46.3 ms ±  13.8 ms    [User: 34.3 ms, System: 11.9 ms]
>   Range (min … max):    34.3 ms …  79.1 ms    82 runs
>
> Benchmark 2: without hash
>   Time (mean ± σ):      26.0 ms ±   7.9 ms    [User: 11.8 ms, System: 14.2 ms]
>   Range (min … max):    16.3 ms …  42.0 ms    69 runs
>
> Summary
>   'without hash' ran
>     1.78 ± 0.76 times faster than 'with hash'
>
> These performance benefits are substantial enough to allow users the
> ability to opt-in to this feature, even with the potential confusion
> with older 'git fsck' versions.

>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  Documentation/config/index.txt |  8 ++++++++
>  read-cache.c                   | 22 +++++++++++++++++++++-
>  t/t1600-index.sh               |  8 ++++++++
>  3 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt
> index 75f3a2d1054..709ba72f622 100644
> --- a/Documentation/config/index.txt
> +++ b/Documentation/config/index.txt
> @@ -30,3 +30,11 @@ index.version::
>         Specify the version with which new index files should be
>         initialized.  This does not affect existing repositories.
>         If `feature.manyFiles` is enabled, then the default is 4.
> +
> +index.computeHash::
> +       When enabled, compute the hash of the index file as it is written
> +       and store the hash at the end of the content. This is enabled by
> +       default.
> ++
> +If you disable `index.computHash`, then older Git clients may report that
> +your index is corrupt during `git fsck`.
> diff --git a/read-cache.c b/read-cache.c
> index 32024029274..f24d96de4d3 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1817,6 +1817,8 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
>         git_hash_ctx c;
>         unsigned char hash[GIT_MAX_RAWSZ];
>         int hdr_version;
> +       int all_zeroes = 1;
> +       unsigned char *start, *end;
>
>         if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
>                 return error(_("bad signature 0x%08x"), hdr->hdr_signature);
> @@ -1827,10 +1829,23 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
>         if (!verify_index_checksum)
>                 return 0;
>
> +       end = (unsigned char *)hdr + size;
> +       start = end - the_hash_algo->rawsz;
> +       while (start < end) {
> +               if (*start != 0) {
> +                       all_zeroes = 0;
> +                       break;
> +               }
> +               start++;
> +       }
> +
> +       if (all_zeroes)
> +               return 0;
> +
>         the_hash_algo->init_fn(&c);
>         the_hash_algo->update_fn(&c, hdr, size - the_hash_algo->rawsz);
>         the_hash_algo->final_fn(hash, &c);
> -       if (!hasheq(hash, (unsigned char *)hdr + size - the_hash_algo->rawsz))
> +       if (!hasheq(hash, end - the_hash_algo->rawsz))
>                 return error(_("bad index file sha1 signature"));
>         return 0;
>  }
> @@ -2917,9 +2932,14 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
>         int ieot_entries = 1;
>         struct index_entry_offset_table *ieot = NULL;
>         int nr, nr_threads;
> +       int compute_hash;
>
>         f = hashfd(tempfile->fd, tempfile->filename.buf);
>
> +       if (!git_config_get_maybe_bool("index.computehash", &compute_hash) &&
> +           !compute_hash)
> +               f->skip_hash = 1;
> +
>         for (i = removed = extended = 0; i < entries; i++) {
>                 if (cache[i]->ce_flags & CE_REMOVE)
>                         removed++;
> diff --git a/t/t1600-index.sh b/t/t1600-index.sh
> index 010989f90e6..24ab90ca047 100755
> --- a/t/t1600-index.sh
> +++ b/t/t1600-index.sh
> @@ -103,4 +103,12 @@ test_expect_success 'index version config precedence' '
>         test_index_version 0 true 2 2
>  '
>
> +test_expect_success 'index.computeHash config option' '
> +       (
> +               rm -f .git/index &&
> +               git -c index.computeHash=false add a &&
> +               git fsck
> +       )
> +'
> +
>  test_done
> --
> gitgitgadget