Re: [PATCH 2/4] p5312: removed duplicate performance test script

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 17, 2025, 22:08 UTC
Message-ID: <xmqqh62me8zo.fsf@gitster.g>
In-Reply-To: <51c4604e16c886d888138f2b513e4d3407b10728.1744924321.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> Subject: Re: [PATCH 2/4] p5312: removed duplicate performance test script

"removed" -> "remove"???

Show 5 quoted lines

> When the reachability bitmap format learned to read and write a lookup
> table containing the set of commits which received reachability bitmaps,
> commit 761416ef91 (bitmap-lookup-table: add performance tests for lookup
> table, 2022-08-14) added that mirrored p5310 but with reverse indexes
> enabled.

"added that" -> "added a <something> that"???

> Later on in a8dd7e05b1 (config: enable `pack.writeReverseIndex` by
> default, 2023-04-12), we enabled reverse indexes by default, which made
> these two tests indistinguishable from one another. Commit a8dd7e05b1
> should have removed p5312 as a duplicate, but didn't do so.

Or to retain the same coverage, it should have explicitly disabled reverse index in one of the tests, while allowing the other to use the reverse index enabled by default, perhaps?

Show 48 quoted lines

> Correct that by removing p5312 as a functional duplicate of p5310.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  t/perf/p5312-pack-bitmaps-revs.sh | 34 -------------------------------
>  1 file changed, 34 deletions(-)
>  delete mode 100755 t/perf/p5312-pack-bitmaps-revs.sh
>
> diff --git a/t/perf/p5312-pack-bitmaps-revs.sh b/t/perf/p5312-pack-bitmaps-revs.sh
> deleted file mode 100755
> index ceec60656b..0000000000
> --- a/t/perf/p5312-pack-bitmaps-revs.sh
> +++ /dev/null
> @@ -1,34 +0,0 @@
> -#!/bin/sh
> -
> -test_description='Tests pack performance using bitmaps (rev index enabled)'
> -. ./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_perf_large_repo
> -
> -	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 "enable lookup table: $1" '
> -		git config pack.writeBitmapLookupTable '"$1"'
> -	'
> -
> -	test_pack_bitmap
> -}
> -
> -test_lookup_pack_bitmap false
> -test_lookup_pack_bitmap true
> -
> -test_done