diff mbox series

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

Message ID 96c0041688f6139c17611203f98274988ced25ab.1656249018.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 June 26, 2022, 1:10 p.m. UTC 
From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>

Add performance tests to verify the performance of lookup table.

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)                   295.94(250.45+15.24)
5310.5: simulated clone                                 12.52(5.07+1.40)
5310.6: simulated fetch                                 1.89(2.94+0.24)
5310.7: pack to file (bitmap)                           41.39(20.33+7.20)
5310.8: rev-list (commits)                              0.98(0.59+0.12)
5310.9: rev-list (objects)                              3.40(3.27+0.10)
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.23(0.16+0.06)
5310.12: rev-list count with blob:none                  0.26(0.18+0.07)
5310.13: rev-list count with blob:limit=1k              6.45(5.94+0.37)
5310.14: rev-list count with tree:0                     0.26(0.18+0.07)
5310.15: simulated partial clone                        4.99(3.19+0.45)
5310.19: repack to disk (lookup=true)                   269.67(174.70+21.33)
5310.20: simulated clone                                11.03(5.07+1.11)
5310.21: simulated fetch                                0.79(0.79+0.17)
5310.22: pack to file (bitmap)                          43.03(20.28+7.43)
5310.23: rev-list (commits)                             0.86(0.54+0.09)
5310.24: rev-list (objects)                             3.35(3.26+0.07)
5310.25: rev-list with tag negated via --not		0.05(0.00+0.03)
	 --all (objects)
5310.26: rev-list with negative tag (objects)           0.22(0.16+0.05)
5310.27: rev-list count with blob:none                  0.22(0.16+0.05)
5310.28: rev-list count with blob:limit=1k              6.45(5.87+0.31)
5310.29: rev-list count with tree:0                     0.22(0.16+0.05)
5310.30: simulated partial clone                        5.17(3.12+0.48)

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

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

Comments

Taylor Blau June 27, 2022, 9:53 p.m. UTC | #1 
On Sun, Jun 26, 2022 at 01:10:16PM +0000, Abhradeep Chakraborty via GitGitGadget wrote:
> From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
>
> Add performance tests to verify the performance of lookup table.
>
> 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)                   295.94(250.45+15.24)
> 5310.5: simulated clone                                 12.52(5.07+1.40)
> 5310.6: simulated fetch                                 1.89(2.94+0.24)
> 5310.7: pack to file (bitmap)                           41.39(20.33+7.20)
> 5310.8: rev-list (commits)                              0.98(0.59+0.12)
> 5310.9: rev-list (objects)                              3.40(3.27+0.10)
> 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.23(0.16+0.06)
> 5310.12: rev-list count with blob:none                  0.26(0.18+0.07)
> 5310.13: rev-list count with blob:limit=1k              6.45(5.94+0.37)
> 5310.14: rev-list count with tree:0                     0.26(0.18+0.07)
> 5310.15: simulated partial clone                        4.99(3.19+0.45)
> 5310.19: repack to disk (lookup=true)                   269.67(174.70+21.33)
> 5310.20: simulated clone                                11.03(5.07+1.11)
> 5310.21: simulated fetch                                0.79(0.79+0.17)
> 5310.22: pack to file (bitmap)                          43.03(20.28+7.43)
> 5310.23: rev-list (commits)                             0.86(0.54+0.09)
> 5310.24: rev-list (objects)                             3.35(3.26+0.07)
> 5310.25: rev-list with tag negated via --not		0.05(0.00+0.03)
> 	 --all (objects)
> 5310.26: rev-list with negative tag (objects)           0.22(0.16+0.05)
> 5310.27: rev-list count with blob:none                  0.22(0.16+0.05)
> 5310.28: rev-list count with blob:limit=1k              6.45(5.87+0.31)
> 5310.29: rev-list count with tree:0                     0.22(0.16+0.05)
> 5310.30: simulated partial clone                        5.17(3.12+0.48)
>
> Test 4-15 are tested without using lookup table. Same tests are
> repeated in 16-30 (using lookup table).
>
> Signed-off-by: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
> Mentored-by: Taylor Blau <me@ttaylorr.com>
> Co-Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> ---
>  t/perf/p5310-pack-bitmaps.sh       | 77 ++++++++++++++-----------
>  t/perf/p5326-multi-pack-bitmaps.sh | 93 ++++++++++++++++--------------
>  2 files changed, 94 insertions(+), 76 deletions(-)
>
> diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh
> index 7ad4f237bc3..6ff42bdd391 100755
> --- a/t/perf/p5310-pack-bitmaps.sh
> +++ b/t/perf/p5310-pack-bitmaps.sh
> @@ -16,39 +16,48 @@ test_expect_success 'setup bitmap config' '
>  	git config pack.writebitmaps 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_perf 'repack to disk' '
> -	git repack -ad
> -'
> -
> -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 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 () {
> +    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
> +	'

I think this "create tags" step can happen outside of the test_bitmap()
function, since it should only need to be done once, right?

> +	test_expect_success "use lookup table: $enabled" '
> +		git config pack.writeBitmapLookupTable '"$enabled"'
> +	'
> +
> +	test_perf "repack to disk (lookup=$enabled)" '
> +		git repack -ad
> +	'

And I think these two tests could be combined, since this could just
become:

    git -c pack.writeBitmapLookupTable "$enabled" repack -ad

right?

> +	test_full_bitmap
> +
> +    test_expect_success "create partial bitmap state (lookup=$enabled)" '

There is some funky spacing going on here, at least in my email client.
Could you double check that tabs are used consistently here?

Thanks,
Taylor
Abhradeep Chakraborty June 28, 2022, 7:58 a.m. UTC | #2 
Taylor Blau <me@ttaylorr.com> wrote:

> I think this "create tags" step can happen outside of the test_bitmap()
> function, since it should only need to be done once, right?

Yeah, I also think the same. That's why I tried to not include in the
Function but for some reason, one test is failing -

  perf 24 - rev-list with tag negated via --not --all (objects):
  running: 
  		git rev-list perf-tag --not --all --use-bitmap-index --objects >/dev/null
	
  fatal: ambiguous argument 'perf-tag': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git <command> [<revision>...] -- [<file>...]'
  not ok 24 - rev-list with tag negated via --not --all (objects)

One thing to note here is that the first `test_bitmap` call always
Passes. But the second `test_bitmap` call fails due to above error.
It throws error irrespective of any parameters for second `test_bitmap`.

If I put it inside the function it doesn't throw any error! 

For this reason, I put it into the function. Do you have any idea
why this happend?

> And I think these two tests could be combined, since this could just
> become:
>
>    git -c pack.writeBitmapLookupTable "$enabled" repack -ad
>
> right?

Yeah, sure.

> There is some funky spacing going on here, at least in my email client.
> Could you double check that tabs are used consistently here?

This is due to my editor's spacing issues. All seems fine when I look at
it in my editor. But actually it is not. Fixing it.

Thanks :)
Taylor Blau June 29, 2022, 8:40 p.m. UTC | #3 
On Tue, Jun 28, 2022 at 01:28:43PM +0530, Abhradeep Chakraborty wrote:
> Taylor Blau <me@ttaylorr.com> wrote:
>
> > I think this "create tags" step can happen outside of the test_bitmap()
> > function, since it should only need to be done once, right?
>
> Yeah, I also think the same. That's why I tried to not include in the
> Function but for some reason, one test is failing -
>
>   perf 24 - rev-list with tag negated via --not --all (objects):
>   running:
>   		git rev-list perf-tag --not --all --use-bitmap-index --objects >/dev/null
>
>   fatal: ambiguous argument 'perf-tag': unknown revision or path not in the working tree.
>   Use '--' to separate paths from revisions, like this:
>   'git <command> [<revision>...] -- [<file>...]'
>   not ok 24 - rev-list with tag negated via --not --all (objects)
>
> One thing to note here is that the first `test_bitmap` call always
> Passes. But the second `test_bitmap` call fails due to above error.
> It throws error irrespective of any parameters for second `test_bitmap`.
>
> If I put it inside the function it doesn't throw any error!
>
> For this reason, I put it into the function. Do you have any idea
> why this happend?

I think that it's because we delete all of the refs in the test that
creates a partial bitmap state. So anything that relies on perf-tag
existing after that test runs will definitely not work :).

My original suggestion was misguided there, unless we wanted to make the
aforementioned test (the one that creates the partial bitmap state)
restore the ref state after it finishes running, but I don't think
that's worthwhile.

Thanks,
Taylor
diff mbox series

Patch

diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh
index 7ad4f237bc3..6ff42bdd391 100755
--- a/t/perf/p5310-pack-bitmaps.sh
+++ b/t/perf/p5310-pack-bitmaps.sh
@@ -16,39 +16,48 @@  test_expect_success 'setup bitmap config' '
 	git config pack.writebitmaps 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_perf 'repack to disk' '
-	git repack -ad
-'
-
-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 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 () {
+    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_perf "repack to disk (lookup=$enabled)" '
+		git repack -ad
+	'
+
+	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 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_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..d67e7437493 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