@matheusbsbs / matheus-bagliano / commits / dbcf611617

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>

Taylor Blau committed Apr 12, 2023 at 18:20 UTC dbcf611617cc21560d2887e92aaa756f8fd681d8
5 files changed +23 -1
Documentation/config/pack.txt
+6
@@ -171,6 +171,12 @@ pack.writeBitmapLookupTable::
171 beneficial in repositories that have relatively large bitmap
172 indexes. Defaults to false.
173
174 +pack.readReverseIndex::
175 + When true, git will read any .rev file(s) that may be available
176 + (see: linkgit:gitformat-pack[5]). When false, the reverse index
177 + will be generated from scratch and stored in memory. Defaults to
178 + true.
179 +
180 pack.writeReverseIndex::
181 When true, git will write a corresponding .rev file (see:
182 linkgit:gitformat-pack[5])
pack-revindex.c
+4 -1
@@ -291,7 +291,10 @@ int load_pack_revindex(struct repository *r, struct packed_git *p)
291 if (p->revindex || p->revindex_data)
292 return 0;
293
294 - if (!load_pack_revindex_from_disk(p))
294 + prepare_repo_settings(r);
295 +
296 + if (r->settings.pack_read_reverse_index &&
297 + !load_pack_revindex_from_disk(p))
298 return 0;
299 else if (!create_pack_revindex_in_memory(p))
300 return 0;
repo-settings.c
+1
@@ -63,6 +63,7 @@ void prepare_repo_settings(struct repository *r)
63 repo_cfg_bool(r, "core.multipackindex", &r->settings.core_multi_pack_index, 1);
64 repo_cfg_bool(r, "index.sparse", &r->settings.sparse_index, 0);
65 repo_cfg_bool(r, "index.skiphash", &r->settings.index_skip_hash, r->settings.index_skip_hash);
66 + repo_cfg_bool(r, "pack.readreverseindex", &r->settings.pack_read_reverse_index, 1);
67
68 /*
69 * The GIT_TEST_MULTI_PACK_INDEX variable is special in that
repository.h
+1
@@ -37,6 +37,7 @@ struct repo_settings {
37 int fetch_write_commit_graph;
38 int command_requires_full_index;
39 int sparse_index;
40 + int pack_read_reverse_index;
41
42 struct fsmonitor_settings *fsmonitor; /* lazily loaded */
43
t/t5325-reverse-index.sh
+11
@@ -96,6 +96,17 @@ test_expect_success 'reverse index is not generated when available on disk' '
96 --batch-check="%(objectsize:disk)" <tip
97 '
98
99 +test_expect_success 'reverse index is ignored when pack.readReverseIndex is false' '
100 + test_index_pack true &&
101 + test_path_is_file $rev &&
102 +
103 + test_config pack.readReverseIndex false &&
104 +
105 + git rev-parse HEAD >tip &&
106 + GIT_TEST_REV_INDEX_DIE_ON_DISK=1 git cat-file \
107 + --batch-check="%(objectsize:disk)" <tip
108 +'
109 +
110 test_expect_success 'revindex in-memory vs on-disk' '
111 git init repo &&
112 test_when_finished "rm -fr repo" &&