Re: [PATCH v2 2/4] pack-objects: support reachability bitmaps with `--path-walk`

[Taylor Blau <me@ttaylorr.com>]

On Fri, Jun 12, 2026 at 09:24:32AM -0400, Derrick Stolee wrote:
> On 6/2/2026 6:21 PM, Taylor Blau wrote:
> > When 'pack-objects' is invoked with '--path-walk', it prevents us from
> > using reachability bitmaps.
>
> My earlier response focused on the _use_ of bitmaps when creating a
> packfile, but your patch also enables _writing_ bitmaps with the
> --path-walk option, which is significant and potentially more
> interesting from my perspective: we have evidence that --path-walk
> can produce significantly smaller packfiles than the standard
> algorithm, and once those packfiles are created we can benefit from
> that size in later packfile creation steps by reusing those deltas.

I am perhaps splitting hairs here, but I would frame the use of bitmaps
when reading with "--path-walk" as "either/or" not "both/and". The main
goal of this patch is to enable us to still generate bitmaps when
*writing* a pack with "--path-walk".

> Even more important here is that we have demonstrated examples of repos
> that change their packfile size when using the --path-walk method. We
> should demonstrate that the size continues to shrink with --path-walk
> even when producing a matching .bitmap file with --write-bitmap-index.

That's fair. One way to do this would be to:

--- 8< ---
diff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh
index 1b115d921a1..c1aed3e2aef 100755
--- a/t/perf/p5311-pack-bitmaps-fetch.sh
+++ b/t/perf/p5311-pack-bitmaps-fetch.sh
@@ -18,6 +18,10 @@ test_fetch_bitmaps () {
 		git repack -ad $argv
 	'

+	test_size "size of bitmapped pack ${argv:+($argv)}" '
+		test_file_size .git/objects/pack/pack-*.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
--- >8 ---

, which gives us:

    Test                                            HEAD^             HEAD
    ----------------------------------------------------------------------------------------
    5311.3: size of bitmapped pack                           278.8M            278.8M -0.0%
    5311.38: size of bitmapped pack (--path-walk)            278.7M            278.7M +0.0%

(eliding other tests). I considered whether there are other interesting
tests, but I think "repack" is the right layer to run perf tests, since
you're always writing a closed pack. We could try different subsets of
the repository's objects (which would also have to be closed), but I
don't think this is that interesting.

> The other thing that I notice here is that the bitmaps will need to
> compute their reachable object set independently from the path-walk
> algorithm. But I suppose that already happens separately from the
> revision-walk approach that normally produces the packfile contents.

Right. The only wrinkle here is how we handle the internal traversal's
"--boundary" option, but see the last paragraph in the commit message
for details on why the proposed approach is OK.

> >From my perspective, the point of integrating these two things are:
>
> 1. Reachability bitmaps make it much faster to discover the reachable
>    set and reuse bits of existing packfiles. (Your performance table
>    demonstrates this is true.)
>
> 2. The --path-walk option can shrink packfile sizes by grouping
>    trees and blobs by path before those paths collide in the name-hash
>    sort. (I haven't seen evidence that this is happening.)
>
> With evidence of (1) and not (2), it's not clear from the data that
> these features are integrating completely. Without looking at the
> code, those numbers would be the same if we had instead swapped the
> preference of "the --path-walk option disables bitmaps" to "bitmaps
> disable --path-walk".

Let me know if modifying the perf test as above (and including the
relevant results in the commit message) would be sufficient in
addressing your concern.

Thanks,
Taylor