diff mbox series

[v3,5/6] bitmap-lookup-table: add performance tests for lookup table

Message ID a155c1e2ebacf54c451a069499325cdf280606fc.1656924376.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series bitmap: integrate a lookup table extension to the bitmap format | expand

Commit Message

Abhradeep Chakraborty July 4, 2022, 8:46 a.m. UTC
From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>

Add performance tests to verify the performance of lookup table with
`pack.writeReverseIndex` enabled. This is to check the performance
when the above configuration is set.

Lookup table makes Git run faster in most of the cases. Below is the
result of `t/perf/p5310-pack-bitmaps.sh`.`perf/p5326-multi-pack-bitmaps.sh`
gives similar result. The repository used in the test is linux kernel.

Test                                                      this tree
---------------------------------------------------------------------------
5310.4: repack to disk (lookup=false)                   296.55(256.53+14.52)
5310.5: simulated clone                                 15.64(8.88+1.39)
5310.6: simulated fetch                                 1.65(2.75+0.20)
5310.7: pack to file (bitmap)                           48.71(30.20+7.58)
5310.8: rev-list (commits)                              0.61(0.41+0.08)
5310.9: rev-list (objects)                              4.38(4.26+0.09)
5310.10: rev-list with tag negated via --not            0.07(0.02+0.04)
         --all (objects)
5310.11: rev-list with negative tag (objects)           0.05(0.01+0.03)
5310.12: rev-list count with blob:none                  0.08(0.03+0.04)
5310.13: rev-list count with blob:limit=1k              7.29(6.92+0.30)
5310.14: rev-list count with tree:0                     0.08(0.03+0.04)
5310.15: simulated partial clone                        9.45(8.12+0.41)
5310.19: repack to disk (lookup=true)                   255.92(188.13+20.47)
5310.20: simulated clone                                13.78(8.84+1.09)
5310.21: simulated fetch                                0.52(0.63+0.14)
5310.22: pack to file (bitmap)                          44.34(28.94+6.84)
5310.23: rev-list (commits)                             0.48(0.31+0.06)
5310.24: rev-list (objects)                             4.02(3.93+0.07)
5310.25: rev-list with tag negated via --not            0.04(0.00+0.03)
         --all (objects)
5310.26: rev-list with negative tag (objects)           0.04(0.00+0.03)
5310.27: rev-list count with blob:none                  0.04(0.01+0.03)
5310.28: rev-list count with blob:limit=1k              6.48(6.23+0.22)
5310.29: rev-list count with tree:0                     0.04(0.01+0.03)
5310.30: simulated partial clone                        8.30(7.21+0.36)

Test 4-15 are tested without using lookup table. Same tests are
repeated in 16-30 (using lookup table).

Mentored-by: Taylor Blau <me@ttaylorr.com>
Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
---
 t/perf/p5310-pack-bitmaps.sh       | 66 ++++++++++++---------
 t/perf/p5326-multi-pack-bitmaps.sh | 93 ++++++++++++++++--------------
 2 files changed, 89 insertions(+), 70 deletions(-)

Comments

Taylor Blau July 15, 2022, 2:53 a.m. UTC | #1
On Mon, Jul 04, 2022 at 08:46:15AM +0000, Abhradeep Chakraborty via GitGitGadget wrote:
> From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
>
> Add performance tests to verify the performance of lookup table with
> `pack.writeReverseIndex` enabled. This is to check the performance
> when the above configuration is set.
>
> Lookup table makes Git run faster in most of the cases. Below is the
> result of `t/perf/p5310-pack-bitmaps.sh`.`perf/p5326-multi-pack-bitmaps.sh`
> gives similar result. The repository used in the test is linux kernel.
>
> Test                                                      this tree
> ---------------------------------------------------------------------------
> 5310.4: repack to disk (lookup=false)                   296.55(256.53+14.52)

Having "lookup=false" in this test definitely helps visually
differentiate which tests have a bitmap with and without the lookup
table.

I think we should take a slightly different approach for these
performance tests. I think the first change to the t/perf tests in this
series should only enable `pack.writeReverseIndex`. That patch would be
a good place to highlight the benefit of enabling the on-disk reverse
index by showing a before and after of running p5310 before and after
that commit.

Then the patch after that should look like this one, which runs the
suite with and without the lookup table. That should give us a sense of:

  - bitmaps without a lookup table or reverse index
  - bitmaps without a lookup table, but with a reverse index
  - bitamps with a reverse index and a lookup table

...which I think are the most interesting combinations (I wouldn't
expect many or any users to have lookup tables enabled without reverse
indexes).

I think that would allow us to drop the last patch in this version of
the series. But I'm definitely open to other testing strategies for the
performance tests (including this one!) if you have different thoughts
about what the best way to go about this is.

Thanks,
Taylor
Abhradeep Chakraborty July 15, 2022, 6:23 p.m. UTC | #2
On Fri, Jul 15, 2022 at 8:23 AM Taylor Blau <me@ttaylorr.com> wrote:
>
> Having "lookup=false" in this test definitely helps visually
> differentiate which tests have a bitmap with and without the lookup
> table.
>
> I think we should take a slightly different approach for these
> performance tests. I think the first change to the t/perf tests in this
> series should only enable `pack.writeReverseIndex`. That patch would be
> a good place to highlight the benefit of enabling the on-disk reverse
> index by showing a before and after of running p5310 before and after
> that commit.
>
> Then the patch after that should look like this one, which runs the
> suite with and without the lookup table. That should give us a sense of:
>
>   - bitmaps without a lookup table or reverse index
>   - bitmaps without a lookup table, but with a reverse index
>   - bitamps with a reverse index and a lookup table
>
> ...which I think are the most interesting combinations (I wouldn't
> expect many or any users to have lookup tables enabled without reverse
> indexes).
>
> I think that would allow us to drop the last patch in this version of
> the series. But I'm definitely open to other testing strategies for the
> performance tests (including this one!) if you have different thoughts
> about what the best way to go about this is.

Got it. Thanks !

> Thanks,
> Taylor
diff mbox series

Patch

diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh
index 7ad4f237bc3..1ad3c3f14c6 100755
--- a/t/perf/p5310-pack-bitmaps.sh
+++ b/t/perf/p5310-pack-bitmaps.sh
@@ -13,42 +13,52 @@  test_perf_large_repo
 # We intentionally use the deprecated pack.writebitmaps
 # config so that we can test against older versions of git.
 test_expect_success 'setup bitmap config' '
-	git config pack.writebitmaps true
+	git config pack.writebitmaps true &&
+	git config pack.writeReverseIndex true
 '
 
-# we need to create the tag up front such that it is covered by the repack and
-# thus by generated bitmaps.
-test_expect_success 'create tags' '
-	git tag --message="tag pointing to HEAD" perf-tag HEAD
-'
+test_bitmap () {
+	local enabled="$1"
 
-test_perf 'repack to disk' '
-	git repack -ad
-'
+	# we need to create the tag up front such that it is covered by the repack and
+	# thus by generated bitmaps.
+	test_expect_success 'create tags' '
+		git tag --message="tag pointing to HEAD" perf-tag HEAD
+	'
 
-test_full_bitmap
+	test_expect_success "use lookup table: $enabled" '
+		git config pack.writeBitmapLookupTable '"$enabled"'
+	'
 
-test_expect_success 'create partial bitmap state' '
-	# pick a commit to represent the repo tip in the past
-	cutoff=$(git rev-list HEAD~100 -1) &&
-	orig_tip=$(git rev-parse HEAD) &&
+	test_perf "repack to disk (lookup=$enabled)" '
+		git repack -ad
+	'
 
-	# now kill off all of the refs and pretend we had
-	# just the one tip
-	rm -rf .git/logs .git/refs/* .git/packed-refs &&
-	git update-ref HEAD $cutoff &&
+	test_full_bitmap
 
-	# and then repack, which will leave us with a nice
-	# big bitmap pack of the "old" history, and all of
-	# the new history will be loose, as if it had been pushed
-	# up incrementally and exploded via unpack-objects
-	git repack -Ad &&
+	test_expect_success "create partial bitmap state (lookup=$enabled)" '
+		# pick a commit to represent the repo tip in the past
+		cutoff=$(git rev-list HEAD~100 -1) &&
+		orig_tip=$(git rev-parse HEAD) &&
 
-	# and now restore our original tip, as if the pushes
-	# had happened
-	git update-ref HEAD $orig_tip
-'
+		# now kill off all of the refs and pretend we had
+		# just the one tip
+		rm -rf .git/logs .git/refs/* .git/packed-refs &&
+		git update-ref HEAD $cutoff &&
+
+		# and then repack, which will leave us with a nice
+		# big bitmap pack of the "old" history, and all of
+		# the new history will be loose, as if it had been pushed
+		# up incrementally and exploded via unpack-objects
+		git repack -Ad &&
+
+		# and now restore our original tip, as if the pushes
+		# had happened
+		git update-ref HEAD $orig_tip
+	'
+}
 
-test_partial_bitmap
+test_bitmap false
+test_bitmap true
 
 test_done
diff --git a/t/perf/p5326-multi-pack-bitmaps.sh b/t/perf/p5326-multi-pack-bitmaps.sh
index f2fa228f16a..c8cc68185a1 100755
--- a/t/perf/p5326-multi-pack-bitmaps.sh
+++ b/t/perf/p5326-multi-pack-bitmaps.sh
@@ -6,47 +6,56 @@  test_description='Tests performance using midx bitmaps'
 
 test_perf_large_repo
 
-# we need to create the tag up front such that it is covered by the repack and
-# thus by generated bitmaps.
-test_expect_success 'create tags' '
-	git tag --message="tag pointing to HEAD" perf-tag HEAD
-'
-
-test_expect_success 'start with bitmapped pack' '
-	git repack -adb
-'
-
-test_perf 'setup multi-pack index' '
-	git multi-pack-index write --bitmap
-'
-
-test_expect_success 'drop pack bitmap' '
-	rm -f .git/objects/pack/pack-*.bitmap
-'
-
-test_full_bitmap
-
-test_expect_success 'create partial bitmap state' '
-	# pick a commit to represent the repo tip in the past
-	cutoff=$(git rev-list HEAD~100 -1) &&
-	orig_tip=$(git rev-parse HEAD) &&
-
-	# now pretend we have just one tip
-	rm -rf .git/logs .git/refs/* .git/packed-refs &&
-	git update-ref HEAD $cutoff &&
-
-	# and then repack, which will leave us with a nice
-	# big bitmap pack of the "old" history, and all of
-	# the new history will be loose, as if it had been pushed
-	# up incrementally and exploded via unpack-objects
-	git repack -Ad &&
-	git multi-pack-index write --bitmap &&
-
-	# and now restore our original tip, as if the pushes
-	# had happened
-	git update-ref HEAD $orig_tip
-'
-
-test_partial_bitmap
+test_bitmap () {
+	local enabled="$1"
+
+	# we need to create the tag up front such that it is covered by the repack and
+	# thus by generated bitmaps.
+	test_expect_success 'create tags' '
+		git tag --message="tag pointing to HEAD" perf-tag HEAD
+	'
+
+	test_expect_success "use lookup table: $enabled" '
+		git config pack.writeBitmapLookupTable '"$enabled"'
+	'
+
+	test_expect_success "start with bitmapped pack (lookup=$enabled)" '
+		git repack -adb
+	'
+
+	test_perf "setup multi-pack index (lookup=$enabled)" '
+		git multi-pack-index write --bitmap
+	'
+
+	test_expect_success "drop pack bitmap (lookup=$enabled)" '
+		rm -f .git/objects/pack/pack-*.bitmap
+	'
+
+	test_full_bitmap
+
+	test_expect_success "create partial bitmap state (lookup=$enabled)" '
+		# pick a commit to represent the repo tip in the past
+		cutoff=$(git rev-list HEAD~100 -1) &&
+		orig_tip=$(git rev-parse HEAD) &&
+
+		# now pretend we have just one tip
+		rm -rf .git/logs .git/refs/* .git/packed-refs &&
+		git update-ref HEAD $cutoff &&
+
+		# and then repack, which will leave us with a nice
+		# big bitmap pack of the "old" history, and all of
+		# the new history will be loose, as if it had been pushed
+		# up incrementally and exploded via unpack-objects
+		git repack -Ad &&
+		git multi-pack-index write --bitmap &&
+
+		# and now restore our original tip, as if the pushes
+		# had happened
+		git update-ref HEAD $orig_tip
+	'
+}
+
+test_bitmap false
+test_bitmap true
 
 test_done