Skip to content

Commit 0a37451

Browse files
ttaylorrgitster
authored andcommitted
pack-objects: support reachability bitmaps with --path-walk
When 'pack-objects' is invoked with '--path-walk', it prevents us from using reachability bitmaps. This behavior dates back to 70664d2 (pack-objects: add --path-walk option, 2025-05-16), which included a comment in the relevant portion of the command-line arguments handling that read as follows: /* * We must disable the bitmaps because we are removing * the --objects / --objects-edge[-aggressive] options. */ In fb2c309 (pack-objects: pass --objects with --path-walk, 2026-05-02), path-walk learned to pass '--objects' again, but still kept bitmap traversal disabled. That leaves two useful cases unsupported: * A path-walk repack that writes bitmaps does not give the bitmap selector any commits, because path-walk reveals commits through `add_objects_by_path()` rather than through `show_commit()`, where `index_commit_for_bitmap()` is normally called. * An invocation like "git pack-objects --use-bitmap-index --path-walk" never tries an existing bitmap, even when one is available and could answer the request. Fortunately for us, neither restriction is required. * On the writing side: teach the path-walk object callback to call `index_commit_for_bitmap()` for commits that it adds to the pack. That gives the bitmap selector the commit candidates it would have seen from the regular traversal. * For bitmap reading, keep passing '--objects' to the internal rev_list machinery, but stop clearing `use_bitmap_index`. If an existing bitmap can answer the request, use it; otherwise fall back to path-walk's own enumeration. As a result, we can see significantly reduced pack generation times from p5311 (with our `GIT_PERF_REPO` set to a recent clone of the fluentui repository) before this commit: Test HEAD^ HEAD ---------------------------------------------------------------------------------------- 5311.40: server (1 days, --path-walk) 1.43(1.39+0.04) 0.01(0.01+0.00) -99.3% 5311.41: size (1 days, --path-walk) 139.6K 139.7K +0.0% 5311.42: client (1 days, --path-walk) 0.02(0.02+0.00) 0.02(0.02+0.00) +0.0% 5311.44: server (2 days, --path-walk) 1.43(1.39+0.04) 0.01(0.00+0.00) -99.3% 5311.45: size (2 days, --path-walk) 139.6K 139.7K +0.0% 5311.46: client (2 days, --path-walk) 0.02(0.02+0.00) 0.02(0.02+0.00) +0.0% 5311.48: server (4 days, --path-walk) 1.44(1.39+0.04) 0.01(0.01+0.00) -99.3% 5311.49: size (4 days, --path-walk) 238.1K 238.1K +0.0% 5311.50: client (4 days, --path-walk) 0.03(0.03+0.00) 0.03(0.03+0.00) +0.0% 5311.52: server (8 days, --path-walk) 1.43(1.39+0.03) 0.01(0.00+0.00) -99.3% 5311.53: size (8 days, --path-walk) 344.9K 344.9K +0.0% 5311.54: client (8 days, --path-walk) 0.07(0.07+0.00) 0.07(0.08+0.00) +0.0% 5311.56: server (16 days, --path-walk) 1.47(1.44+0.03) 0.10(0.08+0.01) -93.2% 5311.57: size (16 days, --path-walk) 844.0K 844.0K +0.0% 5311.58: client (16 days, --path-walk) 0.09(0.09+0.00) 0.09(0.09+0.00) +0.0% 5311.60: server (32 days, --path-walk) 1.52(1.50+0.05) 0.14(0.15+0.02) -90.8% 5311.61: size (32 days, --path-walk) 4.2M 4.2M +0.1% 5311.62: client (32 days, --path-walk) 0.34(0.48+0.02) 0.34(0.45+0.05) +0.0% 5311.64: server (64 days, --path-walk) 1.55(1.52+0.06) 0.15(0.15+0.04) -90.3% 5311.65: size (64 days, --path-walk) 6.4M 6.4M -0.0% 5311.66: client (64 days, --path-walk) 0.51(0.79+0.05) 0.51(0.80+0.06) +0.0% 5311.68: server (128 days, --path-walk) 1.59(1.57+0.06) 0.16(0.21+0.01) -89.9% 5311.69: size (128 days, --path-walk) 8.4M 8.4M -0.0% 5311.70: client (128 days, --path-walk) 0.72(1.44+0.08) 0.71(1.47+0.09) -1.4% We get the same size of output pack, but this commit allows us to do so in a significantly shorter amount of time. Intuitively, we're generating the same pack (hence the unchanged 'test_size' output from run to run), but varying how we get there. Before this commit, pack-objects prefers '--path-walk' to '--use-bitmap-index', so we generate the output pack by performing a normal '--path-walk' traversal. With this commit, we are operating over a *repacked* state (that itself was done with a '--path-walk' traversal), but are able to perform pack-reuse on that repacked state via bitmaps. When comparing the size of the repacked pack with/without '--path-walk' on the previous commit versus this one, we see that (a) the repacked size improves significantly with '--path-walk', and that (b) writing bitmaps during repacking does not regress this improvement: Test HEAD^ HEAD ---------------------------------------------------------------------------------------- 5311.3: size of bitmapped pack 558.4M 558.5M +0.0% 5311.38: size of bitmapped pack (--path-walk) 164.4M 164.4M +0.0% (Note that to observe an improvement here, we must repack with '-F' in order to avoid reusing non-'--path-walk' deltas, which would otherwise skew our results.) There is one wrinkle when it comes to '--boundary', which we must not pass into the bitmap walk in the presence of both '--path-walk' and '--use-bitmap-index'. Path-walk needs boundary commits when it performs its own traversal, in order to discover bases for thin packs, but the bitmap traversal does not expect this. Work around this by setting `revs->boundary` as late as possible within the '--path-walk' traversal, after any bitmap attempt has either succeeded or declined to answer the request. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 00f7a12 commit 0a37451

4 files changed

Lines changed: 70 additions & 8 deletions

File tree

‎Documentation/git-pack-objects.adoc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,10 @@ will be automatically changed to version `1`.
402402
of filenames that cause collisions in Git's default name-hash
403403
algorithm.
404404
+
405-
Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
406-
ignored in the presence of `--path-walk`. The `--path-walk` option
405+
Incompatible with `--delta-islands`. When `--use-bitmap-index` is
406+
specified with `--path-walk`, a successful bitmap traversal is used for
407+
object enumeration, with path-walk remaining as the fallback traversal
408+
when the bitmap cannot satisfy the request. The `--path-walk` option
407409
supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
408410
`tree:0`, `object:type=<type>`, and `sparse:<oid>`. These supported filter
409411
types can be combined with the `combine:<spec>+<spec>` form.

‎builtin/pack-objects.c‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4742,6 +4742,15 @@ static int add_objects_by_path(const char *path,
47424742
continue;
47434743

47444744
add_object_entry(oid, type, path, exclude);
4745+
4746+
if (type == OBJ_COMMIT && write_bitmap_index) {
4747+
struct commit *commit;
4748+
4749+
commit = lookup_commit(the_repository, oid);
4750+
if (!commit)
4751+
die(_("could not find commit %s"), oid_to_hex(oid));
4752+
index_commit_for_bitmap(commit);
4753+
}
47454754
}
47464755

47474756
oe_end = to_pack.nr_objects;
@@ -4774,6 +4783,13 @@ static int get_object_list_path_walk(struct rev_info *revs)
47744783
info.path_fn = add_objects_by_path;
47754784
info.path_fn_data = &processed;
47764785

4786+
/*
4787+
* Path-walk needs boundary commits to discover thin-pack bases, but
4788+
* bitmap traversal does not understand the boundary state. Set it
4789+
* here so any prior bitmap attempt sees the usual non-boundary walk.
4790+
*/
4791+
revs->boundary = 1;
4792+
47774793
/*
47784794
* Allow the --[no-]sparse option to be interesting here, if only
47794795
* for testing purposes. Paths with no interesting objects will not
@@ -5205,9 +5221,7 @@ int cmd_pack_objects(int argc,
52055221
}
52065222
}
52075223
if (path_walk) {
5208-
strvec_push(&rp, "--boundary");
52095224
strvec_push(&rp, "--objects");
5210-
use_bitmap_index = 0;
52115225
} else if (thin) {
52125226
use_internal_rev_list = 1;
52135227
strvec_push(&rp, shallow

‎t/perf/p5311-pack-bitmaps-fetch.sh‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,30 @@ test_description='performance of fetches from bitmapped packs'
44
. ./perf-lib.sh
55

66
test_fetch_bitmaps () {
7+
argv=$1
8+
export argv
9+
710
test_expect_success 'setup test directory' '
811
rm -fr * .git
912
'
1013

1114
test_perf_default_repo
1215

13-
test_expect_success 'create bitmapped server repo' '
16+
test_expect_success "create bitmapped server repo ${argv:+($argv)}" '
1417
git config pack.writebitmaps true &&
15-
git repack -ad
18+
git repack -adF $argv
19+
'
20+
21+
test_size "size of bitmapped pack ${argv:+($argv)}" '
22+
test_file_size .git/objects/pack/pack-*.pack
1623
'
1724

1825
# simulate a fetch from a repository that last fetched N days ago, for
1926
# various values of N. We do so by following the first-parent chain,
2027
# and assume the first entry in the chain that is N days older than the current
2128
# HEAD is where the HEAD would have been then.
2229
for days in 1 2 4 8 16 32 64 128; do
23-
title=$(printf '%10s' "($days days)")
30+
title=$(printf '%10s' "($days days${argv:+, $argv})")
2431
test_expect_success "setup revs from $days days ago" '
2532
now=$(git log -1 --format=%ct HEAD) &&
2633
then=$(($now - ($days * 86400))) &&
@@ -47,6 +54,9 @@ test_fetch_bitmaps () {
4754
done
4855
}
4956

50-
test_fetch_bitmaps
57+
for argv in '' --path-walk
58+
do
59+
test_fetch_bitmaps $argv || return 1
60+
done
5161

5262
test_done

‎t/t5310-pack-bitmaps.sh‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,42 @@ test_bitmap_cases
577577

578578
sane_unset GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL
579579

580+
test_expect_success 'path-walk repack can write and use bitmap indexes' '
581+
test_when_finished "rm -rf path-walk-bitmap" &&
582+
git init path-walk-bitmap &&
583+
(
584+
cd path-walk-bitmap &&
585+
test_commit first &&
586+
test_commit second &&
587+
test_commit third &&
588+
589+
git repack -a -d -b --path-walk &&
590+
git rev-list --test-bitmap --use-bitmap-index HEAD &&
591+
592+
git rev-parse HEAD >in &&
593+
594+
git rev-list --objects --no-object-names HEAD >expect.raw &&
595+
sort expect.raw >expect &&
596+
597+
for reuse in true false
598+
do
599+
: >trace.txt &&
600+
601+
GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
602+
git -c pack.allowPackReuse=$reuse pack-objects \
603+
--stdout --revs --path-walk --use-bitmap-index \
604+
<in >out.pack &&
605+
test_grep "\"category\":\"bitmap\",\"key\":\"bitmap/hits\"" trace.txt &&
606+
607+
git index-pack out.pack &&
608+
609+
list_packed_objects out.idx >actual.raw &&
610+
sort actual.raw >actual &&
611+
test_cmp expect actual || return 1
612+
done
613+
)
614+
'
615+
580616
test_expect_success 'incremental repack fails when bitmaps are requested' '
581617
test_commit more-1 &&
582618
test_must_fail git repack -d 2>err &&

0 commit comments

Comments
 (0)