Subject: Re: [PATCH v3 03/12] t/perf: add pack-objects filter and path-walk benchmark

From: Taylor Blau <[email-masked]>
Date: Mon, 11 May 2026 21:11:21 -0400

On Mon, May 11, 2026 at 06:13:00PM +0000, Derrick Stolee via GitGitGadget wrote:
> ---
>  t/perf/p5315-pack-objects-filter.sh | 129 ++++++++++++++++++++++++++++
>  1 file changed, 129 insertions(+)
>  create mode 100755 t/perf/p5315-pack-objects-filter.sh

This is great, thank you for adding these as part of the series! :-)

> diff --git a/t/perf/p5315-pack-objects-filter.sh b/t/perf/p5315-pack-objects-filter.sh
> new file mode 100755
> index 0000000000..b009039c89
> --- /dev/null
> +++ b/t/perf/p5315-pack-objects-filter.sh
> @@ -0,0 +1,129 @@
> +#!/bin/sh
> +
> +test_description='Tests pack-objects performance with filters and --path-walk'
> +. ./perf-lib.sh
> +
> +test_perf_large_repo
> +
> +test_expect_success 'setup filter inputs' '
> +	# Sample a few depth-2 directories from the test repo to build
> +	# a cone-mode sparse-checkout definition.  The sampling picks
> +	# directories at evenly-spaced positions so the choice is stable
> +	# and scales to repos of any shape.
> +
> +	git ls-tree -d --name-only HEAD >top-dirs &&
> +	top_nr=$(wc -l <top-dirs) &&
> +
> +	>depth2-dirs &&
> +	while read tdir
> +	do
> +		git ls-tree -d --name-only "HEAD:$tdir" 2>/dev/null |
> +		sed "s|^|$tdir/|" >>depth2-dirs || return 1
> +	done <top-dirs &&

Not a huge deal, but I think we should avoid having ls-tree on the
left-hand side of the pipe here. I would perhaps write this as:

    while read tdir
    do
        git ls-tree -d --name-only "HEAD:$tdir" || return 1
    done <top-dirs >depth2-dirs.raw &&
    sed -e "s|^|$tdir/|" depth2-dirs.raw >depth2-dirs &&

(Moving sed outside of the main loop cuts down on the process count, but
isn't a huge deal. I think the main thing I wanted to suggest here was
avoiding 'git ls-tree ... | sed'.)

The rest of the test looks great to me.

Thanks,
Taylor