pack-revindex: introduce `pack.readReverseIndex`

Since 1615c567b8 (Documentation/config/pack.txt: advertise
'pack.writeReverseIndex', 2021-01-25), we have had the
`pack.writeReverseIndex` configuration option, which tells Git whether
or not it is allowed to write a ".rev" file when indexing a pack.

Introduce a complementary configuration knob, `pack.readReverseIndex` to
control whether or not Git will read any ".rev" file(s) that may be
available on disk.

This option is useful for debugging, as well as disabling the effect of
".rev" files in certain instances.

This is useful because of the trade-off[^1] between the time it takes to
generate a reverse index (slow from scratch, fast when reading an
existing ".rev" file), and the time it takes to access a record (the
opposite).

For example, even though it is faster to use the on-disk reverse index
when computing the on-disk size of a packed object, it is slower to
enumerate the same value for all objects.

Here are a couple of examples from linux.git. When computing the above
for a single object, using the on-disk reverse index is significantly
faster:

    $ git rev-parse HEAD >in
    $ hyperfine -L v false,true 'git.compile -c pack.readReverseIndex={v} cat-file --batch-check="%(objectsize:disk)" <in'
    Benchmark 1: git.compile -c pack.readReverseIndex=false cat-file --batch-check="%(objectsize:disk)" <in
      Time (mean ± σ):     302.5 ms ±  12.5 ms    [User: 258.7 ms, System: 43.6 ms]
      Range (min … max):   291.1 ms … 328.1 ms    10 runs

    Benchmark 2: git.compile -c pack.readReverseIndex=true cat-file --batch-check="%(objectsize:disk)" <in
      Time (mean ± σ):       3.9 ms ±   0.3 ms    [User: 1.6 ms, System: 2.4 ms]
      Range (min … max):     2.0 ms …   4.4 ms    801 runs

    Summary
      'git.compile -c pack.readReverseIndex=true cat-file --batch-check="%(objectsize:disk)" <in' ran
       77.29 ± 7.14 times faster than 'git.compile -c pack.readReverseIndex=false cat-file --batch-check="%(objectsize:disk)" <in'

, but when instead trying to compute the on-disk object size for all
objects in the repository, using the ".rev" file is a disadvantage over
creating the reverse index from scratch:

    $ hyperfine -L v false,true 'git.compile -c pack.readReverseIndex={v} cat-file --batch-check="%(objectsize:disk)" --batch-all-objects'
    Benchmark 1: git.compile -c pack.readReverseIndex=false cat-file --batch-check="%(objectsize:disk)" --batch-all-objects
      Time (mean ± σ):      8.258 s ±  0.035 s    [User: 7.949 s, System: 0.308 s]
      Range (min … max):    8.199 s …  8.293 s    10 runs

    Benchmark 2: git.compile -c pack.readReverseIndex=true cat-file --batch-check="%(objectsize:disk)" --batch-all-objects
      Time (mean ± σ):     16.976 s ±  0.107 s    [User: 16.706 s, System: 0.268 s]
      Range (min … max):   16.839 s … 17.105 s    10 runs

    Summary
      'git.compile -c pack.readReverseIndex=false cat-file --batch-check="%(objectsize:disk)" --batch-all-objects' ran
	2.06 ± 0.02 times faster than 'git.compile -c pack.readReverseIndex=true cat-file --batch-check="%(objectsize:disk)" --batch-all-objects'

Luckily, the results when running `git cat-file` with `--unordered` are
closer together:

    $ hyperfine -L v false,true 'git.compile -c pack.readReverseIndex={v} cat-file --unordered --batch-check="%(objectsize:disk)" --batch-all-objects'
    Benchmark 1: git.compile -c pack.readReverseIndex=false cat-file --unordered --batch-check="%(objectsize:disk)" --batch-all-objects
      Time (mean ± σ):      5.066 s ±  0.105 s    [User: 4.792 s, System: 0.274 s]
      Range (min … max):    4.943 s …  5.220 s    10 runs

    Benchmark 2: git.compile -c pack.readReverseIndex=true cat-file --unordered --batch-check="%(objectsize:disk)" --batch-all-objects
      Time (mean ± σ):      6.193 s ±  0.069 s    [User: 5.937 s, System: 0.255 s]
      Range (min … max):    6.145 s …  6.356 s    10 runs

    Summary
      'git.compile -c pack.readReverseIndex=false cat-file --unordered --batch-check="%(objectsize:disk)" --batch-all-objects' ran
        1.22 ± 0.03 times faster than 'git.compile -c pack.readReverseIndex=true cat-file --unordered --batch-check="%(objectsize:disk)" --batch-all-objects'

Because the equilibrium point between these two is highly machine- and
repository-dependent, allow users to configure whether or not they will
read any ".rev" file(s) with this configuration knob.

[^1]: Generating a reverse index in memory takes O(N) time (where N is
  the number of objects in the repository), since we use a radix sort.
  Reading an entry from an on-disk ".rev" file is slower since each
  operation is bound by disk I/O instead of memory I/O.

  In order to compute the on-disk size of a packed object, we need to
  find the offset of our object, and the adjacent object (the on-disk
  size difference of these two). Finding the first offset requires a
  binary search. Finding the latter involves a single .rev lookup.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 53093d99969cc090e4727a570eb667a6fba445d4..7db7fed46673cd9b5ef3447dc258e3a2aa88de1c 100644 (file)
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -171,6 +171,12 @@ pack.writeBitmapLookupTable::
        beneficial in repositories that have relatively large bitmap
        indexes. Defaults to false.
 
+pack.readReverseIndex::
+       When true, git will read any .rev file(s) that may be available
+       (see: linkgit:gitformat-pack[5]). When false, the reverse index
+       will be generated from scratch and stored in memory. Defaults to
+       true.
+
 pack.writeReverseIndex::
        When true, git will write a corresponding .rev file (see:
        linkgit:gitformat-pack[5])

diff --git a/pack-revindex.c b/pack-revindex.c
index 44e1b3fed9570f93fce64d09eff5fc5d9c0758ed..29f5358b2564cbcd8dda4f47ac2a99c38f26b23d 100644 (file)
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -291,7 +291,10 @@ int load_pack_revindex(struct repository *r, struct packed_git *p)
        if (p->revindex || p->revindex_data)
                return 0;
 
-       if (!load_pack_revindex_from_disk(p))
+       prepare_repo_settings(r);
+
+       if (r->settings.pack_read_reverse_index &&
+           !load_pack_revindex_from_disk(p))
                return 0;
        else if (!create_pack_revindex_in_memory(p))
                return 0;

diff --git a/repo-settings.c b/repo-settings.c
index 0a6c0b381fe13b50fb14b5b6e31ca06c0d44f695..bdd7640ab0be3a53c527e6fb5411d648fa942222 100644 (file)
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -63,6 +63,7 @@ void prepare_repo_settings(struct repository *r)
        repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
        repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);
        repo_cfg_bool(r, "index.skiphash", &r->settings.index_skip_hash, r->settings.index_skip_hash);
+       repo_cfg_bool(r, "pack.readreverseindex", &r->settings.pack_read_reverse_index, 1);
 
        /*
         * The GIT_TEST_MULTI_PACK_INDEX variable is special in that

diff --git a/repository.h b/repository.h
index 15a8afc5fb5b31b12bc3495663f9f3ca2f399c7e..ed73e799b61c66121b0b038351af2b3e8ab96b06 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -37,6 +37,7 @@ struct repo_settings {
        int fetch_write_commit_graph;
        int command_requires_full_index;
        int sparse_index;
+       int pack_read_reverse_index;
 
        struct fsmonitor_settings *fsmonitor; /* lazily loaded */
 

diff --git a/t/t5325-reverse-index.sh b/t/t5325-reverse-index.sh
index 48c14d85764fff3e431a8722d87dd427399bc067..66171c1d67b39661cd838e296fc49f92de76a6f9 100755 (executable)
--- a/t/t5325-reverse-index.sh
+++ b/t/t5325-reverse-index.sh
@@ -96,6 +96,17 @@ test_expect_success 'reverse index is not generated when available on disk' '
                --batch-check="%(objectsize:disk)" <tip
 '
 
+test_expect_success 'reverse index is ignored when pack.readReverseIndex is false' '
+       test_index_pack true &&
+       test_path_is_file $rev &&
+
+       test_config pack.readReverseIndex false &&
+
+       git rev-parse HEAD >tip &&
+       GIT_TEST_REV_INDEX_DIE_ON_DISK=1 git cat-file \
+               --batch-check="%(objectsize:disk)" <tip
+'
+
 test_expect_success 'revindex in-memory vs on-disk' '
        git init repo &&
        test_when_finished "rm -fr repo" &&