Re: [PATCH 0/3] pack-objects: support bitmaps and delta-islands with `--path-walk`

[Taylor Blau <me@ttaylorr.com>]

On Fri, May 29, 2026 at 05:28:32PM -0400, Derrick Stolee wrote:
> > My reading here is that we get significantly smaller packs (i.e. all
> > 'test_size' tests drop from HEAD~3 to HEAD) in the same amount of time
> > (i.e. that all 'test_perf' tests are roughly flat).
>
> The sizes don't shrink, and in one case increases by a small amount. I'm
> happy to count those cases as noise from multi-threaded delta calculations
> being less deterministic.
>
> The _time_ taken to compute the packfiles is what decreases, though, which
> is promising.
>
> > That lines up with my expectation here, which is that even though we're
> > using bitmaps at read time, that's effectively seeding the verbatim pack
> > reuse over a significantly smaller pack, producing a much smaller output
> > pack as a result.
>
> Can you double-check this reasoning with my read of the data? The size
> isn't changing, but the computation time is.

Yeah, that's right, and my apologies for being in a slight rush when
sending this to you ;-).

The size staying flat makes sense, since both packs were generated with
--path-walk, we're just changing the way they're served. In HEAD~3, that
pack is generated on-the-fly and sent over to the client. At HEAD, we're
doing verbatim reuse over an already-existing pack which we got via
repacking (also generated with --path-walk).

So I think you get to the same end-result (more or less, modulo usual
delta patching via pack-reuse), but the time to get there drops
significantly since we don't have to (re)compute the pack.

> > +test_repack_with_args () {
> > +	args="$@"
> > +	export args
> > +
> > +	test_perf "repack with $args" '
> > +		git repack -adf $args
> > +	'
> > +
> > +	test_size "repack size with $args" '
> > +		gitdir=$(git rev-parse --git-dir) &&
> > +		pack=$(ls $gitdir/objects/pack/pack-*.pack) &&
> > +		test_file_size "$pack"
> > +	'
> > +}
> > +
> I see that these tests are extracted from test_all_... below:
>
> [...]
>
> Because the --use-bitmap-index and --write-bitmap-index args
> aren't appropriate across these different commands.

Exactly.

> nit: the diff would be more obvious if test_repack_with_args
> was defined after test_all_with_args so the hunk of existing
> tests wouldn't appear in the diff.

Fair enough :-).

> >  for version in 1 2
> >  do
> > -	test_all_with_args --name-hash-version=$version
> > +	arg="--name-hash-version=$version" &&
> > +
> > +	test_all_with_args "$arg" &&
> > +	test_repack_with_args "$arg" || return 1
> >  done
> >
> >  test_all_with_args --path-walk
> > +test_repack_with_args --path-walk
> > +
> > +# inverted order here: we want to test using reachability bitmaps on a
> > +# pack written with --path-walk
> > +test_repack_with_args --path-walk --write-bitmap-index
> > +test_all_with_args --use-bitmap-index
>
> So this allows us to test all of the different modes.
>
> > --- >8 ---
> >
> > I don't have a strong opinion on whether or not we should include that
> > in this series or elsewhere.
>
> I'm interested to see some results of your new p5313 test
> to make sure that we are getting expected size changes for
> the repack, since the p5311 tests were more focused on the
> thin fetch pack (and didn't show a change in size).
>
> For that, I'd be interested to see this test be included in
> a patch for future reference, too.

Will do.

Thanks,
Taylor