git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org,  Elijah Newren <newren@gmail.com>,
	 Jeff King <peff@peff.net>
Subject: Re: [PATCH 2/4] p5312: removed duplicate performance test script
Date: Thu, 17 Apr 2025 15:08:59 -0700	[thread overview]
Message-ID: <xmqqh62me8zo.fsf@gitster.g> (raw)
In-Reply-To: <51c4604e16c886d888138f2b513e4d3407b10728.1744924321.git.me@ttaylorr.com> (Taylor Blau's message of "Thu, 17 Apr 2025 17:12:17 -0400")

Taylor Blau <me@ttaylorr.com> writes:

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

"removed" -> "remove"???

> 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?

> 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

  reply	other threads:[~2025-04-17 22:09 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 [this message]
2025-04-18 21:57     ` Taylor Blau
2025-04-17 21:12 ` [PATCH 3/4] t/perf: avoid testing bitmaps without lookup table Taylor Blau
2025-04-17 22:21   ` 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=xmqqh62me8zo.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.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).