git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/4] t/perf: avoid testing bitmaps without lookup table
Date: Thu, 17 Apr 2025 17:12:20 -0400	[thread overview]
Message-ID: <8cc5952e594b78ffb2ba4bcaabd62a8e5b8fe72a.1744924321.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1744924321.git.me@ttaylorr.com>

In a previous commit, the setting which controls whether or not the
pack- and MIDX-bitmap machinery writes a lookup table,
'pack.writeBitmapLookupTable' was enabled by default.

As a result, we can clean up many of our bitmap-related performance
tests. Many of the relevant performance tests look something like:

    test_it () {
      test_expect_success 'setup pack.writeBitmapLookupTable' '
        git config pack.writeBitmapLookupTable '"$1"'
      '

      # ...
    }

    test_it true
    test_it false

, where the two invocations of 'test_it' run the tests with and without
bitmap lookup tables enabled.

But now that lookup tables are enabled by default and have proven to be
a performance win, let's avoid benchmarking what is now an uncommon and
non-default scenario.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 t/perf/p5310-pack-bitmaps.sh         |  47 +++++-------
 t/perf/p5311-pack-bitmaps-fetch.sh   |  76 +++++++++----------
 t/perf/p5326-multi-pack-bitmaps.sh   | 107 ++++++++++++---------------
 t/perf/p5333-pseudo-merge-bitmaps.sh |   1 -
 4 files changed, 102 insertions(+), 129 deletions(-)

diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh
index b1399f1007..52f9fca14b 100755
--- a/t/perf/p5310-pack-bitmaps.sh
+++ b/t/perf/p5310-pack-bitmaps.sh
@@ -4,37 +4,28 @@ test_description='Tests pack performance using bitmaps'
 . ./perf-lib.sh
 . "${TEST_DIRECTORY}/perf/lib-bitmap.sh"
 
-test_lookup_pack_bitmap () {
-	test_expect_success 'start the test from scratch' '
-		rm -rf * .git
-	'
+test_expect_success 'start the test from scratch' '
+	rm -rf * .git
+'
 
-	test_perf_large_repo
+test_perf_large_repo
 
-	# note that we do everything through config,
-	# since we want to be able to compare bitmap-aware
-	# git versus non-bitmap git
-	#
-	# 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
-	'
+# note that we do everything through config,
+# since we want to be able to compare bitmap-aware
+# git versus non-bitmap git
+#
+# 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
+'
 
-	# 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
-	'
+# 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 "enable lookup table: $1" '
-		git config pack.writeBitmapLookupTable '"$1"'
-	'
-
-	test_pack_bitmap
-}
-
-test_lookup_pack_bitmap false
-test_lookup_pack_bitmap true
+test_pack_bitmap
 
 test_done
diff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh
index 047efb995d..75b05a600e 100755
--- a/t/perf/p5311-pack-bitmaps-fetch.sh
+++ b/t/perf/p5311-pack-bitmaps-fetch.sh
@@ -3,52 +3,46 @@
 test_description='performance of fetches from bitmapped packs'
 . ./perf-lib.sh
 
-test_fetch_bitmaps () {
-	test_expect_success 'setup test directory' '
-		rm -fr * .git
-	'
-
-	test_perf_default_repo
+test_expect_success 'setup test directory' '
+	rm -fr * .git
+'
 
-	test_expect_success 'create bitmapped server repo' '
-		git config pack.writebitmaps true &&
-		git config pack.writeBitmapLookupTable '"$1"' &&
-		git repack -ad
-	'
+test_perf_default_repo
 
-	# simulate a fetch from a repository that last fetched N days ago, for
-	# various values of N. We do so by following the first-parent chain,
-	# and assume the first entry in the chain that is N days older than the current
-	# HEAD is where the HEAD would have been then.
-	for days in 1 2 4 8 16 32 64 128; do
-		title=$(printf '%10s' "($days days)")
-		test_expect_success "setup revs from $days days ago" '
-			now=$(git log -1 --format=%ct HEAD) &&
-			then=$(($now - ($days * 86400))) &&
-			tip=$(git rev-list -1 --first-parent --until=$then HEAD) &&
-			{
-				echo HEAD &&
-				echo ^$tip
-			} >revs
-		'
+test_expect_success 'create bitmapped server repo' '
+	git config pack.writebitmaps true &&
+	git repack -ad
+'
 
-		test_perf "server $title (lookup=$1)" '
-			git pack-objects --stdout --revs \
-					--thin --delta-base-offset \
-					<revs >tmp.pack
-		'
+# simulate a fetch from a repository that last fetched N days ago, for
+# various values of N. We do so by following the first-parent chain,
+# and assume the first entry in the chain that is N days older than the current
+# HEAD is where the HEAD would have been then.
+for days in 1 2 4 8 16 32 64 128; do
+	title=$(printf '%10s' "($days days)")
+	test_expect_success "setup revs from $days days ago" '
+		now=$(git log -1 --format=%ct HEAD) &&
+		then=$(($now - ($days * 86400))) &&
+		tip=$(git rev-list -1 --first-parent --until=$then HEAD) &&
+		{
+			echo HEAD &&
+			echo ^$tip
+		} >revs
+	'
 
-		test_size "size   $title" '
-			test_file_size tmp.pack
-		'
+	test_perf "server $title" '
+		git pack-objects --stdout --revs \
+				--thin --delta-base-offset \
+				<revs >tmp.pack
+	'
 
-		test_perf "client $title (lookup=$1)" '
-			git index-pack --stdin --fix-thin <tmp.pack
-		'
-	done
-}
+	test_size "size   $title" '
+		test_file_size tmp.pack
+	'
 
-test_fetch_bitmaps true
-test_fetch_bitmaps false
+	test_perf "client $title" '
+		git index-pack --stdin --fix-thin <tmp.pack
+	'
+done
 
 test_done
diff --git a/t/perf/p5326-multi-pack-bitmaps.sh b/t/perf/p5326-multi-pack-bitmaps.sh
index d082e6cacb..9f32582dec 100755
--- a/t/perf/p5326-multi-pack-bitmaps.sh
+++ b/t/perf/p5326-multi-pack-bitmaps.sh
@@ -4,64 +4,53 @@ test_description='Tests performance using midx bitmaps'
 . ./perf-lib.sh
 . "${TEST_DIRECTORY}/perf/lib-bitmap.sh"
 
-test_bitmap () {
-	local enabled="$1"
-
-	test_expect_success "remove existing repo (lookup=$enabled)" '
-		rm -fr * .git
-	'
-
-	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 "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_partial_bitmap
-}
-
-test_bitmap false
-test_bitmap true
+test_expect_success "remove existing repo" '
+	rm -fr * .git
+'
+
+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_done
diff --git a/t/perf/p5333-pseudo-merge-bitmaps.sh b/t/perf/p5333-pseudo-merge-bitmaps.sh
index 2e8b1d2635..91b1c06745 100755
--- a/t/perf/p5333-pseudo-merge-bitmaps.sh
+++ b/t/perf/p5333-pseudo-merge-bitmaps.sh
@@ -11,7 +11,6 @@ test_expect_success 'setup' '
 		-c bitmapPseudoMerge.all.threshold=now \
 		-c bitmapPseudoMerge.all.stableThreshold=never \
 		-c bitmapPseudoMerge.all.maxMerges=64 \
-		-c pack.writeBitmapLookupTable=true \
 		repack -adb
 '
 
-- 
2.49.0.226.g0e6cae136d


  parent reply	other threads:[~2025-04-17 21:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-17 21:12 [PATCH 0/4] pack-bitmap: enable lookup tables by default, misc. cleanups Taylor Blau
2025-04-17 21:12 ` [PATCH 1/4] pack-bitmap: write lookup table extension by default Taylor Blau
2025-04-17 22:04   ` Junio C Hamano
2025-04-18  9:33     ` Jeff King
2025-04-18 15:44       ` Junio C Hamano
2025-04-18 21:52         ` Taylor Blau
2025-04-17 21:12 ` [PATCH 2/4] p5312: removed duplicate performance test script Taylor Blau
2025-04-17 22:08   ` Junio C Hamano
2025-04-18 21:57     ` Taylor Blau
2025-04-17 21:12 ` Taylor Blau [this message]
2025-04-17 22:21   ` [PATCH 3/4] t/perf: avoid testing bitmaps without lookup table Junio C Hamano
2025-04-18  4:24     ` Junio C Hamano
2025-04-18 10:02       ` Jeff King
2025-04-17 21:12 ` [PATCH 4/4] t/perf/lib-bitmap.sh: avoid test_perf during setup Taylor Blau
2025-04-17 22:22   ` Junio C Hamano
2025-04-18 10:17   ` Jeff King
2025-05-02 21:21 ` [PATCH 0/4] pack-bitmap: enable lookup tables by default, misc. cleanups Junio C Hamano
2025-05-05  7:11   ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8cc5952e594b78ffb2ba4bcaabd62a8e5b8fe72a.1744924321.git.me@ttaylorr.com \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).