midx: implement writing incremental MIDX bitmaps

Now that the pack-bitmap machinery has learned how to read and interact with an incremental MIDX bitmap, teach the pack-bitmap-write.c machinery (and relevant callers from within the MIDX machinery) to write such bitmaps. The details for doing so are mostly straightforward. The main changes are as follows: - find_object_pos() now makes use of an extra MIDX parameter which is used to locate the bit positions of objects which are from previous layers (and thus do not exist in the current layer's pack_order field). (Note also that the pack_order field is moved into struct write_midx_context to further simplify the callers for write_midx_bitmap()). - bitmap_writer_build_type_index() first determines how many objects precede the current bitmap layer and offsets the bits it sets in each respective type-level bitmap by that amount so they can be OR'd together. Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Mar 20, 2025 at 13:57 UTC 27afc272c49137460fe9e58e1fcbe4c1d377b304
5 files changed +179 -37
builtin/pack-objects.c
+2 -1
@@ -1397,7 +1397,8 @@ static void write_pack_file(void)
1397
1398 if (write_bitmap_index) {
1399 bitmap_writer_init(&bitmap_writer,
1400 - the_repository, &to_pack);
1400 + the_repository, &to_pack,
1401 + NULL);
1402 bitmap_writer_set_checksum(&bitmap_writer, hash);
1403 bitmap_writer_build_type_index(&bitmap_writer,
1404 written_list);
midx-write.c
+38 -19
@@ -647,16 +647,22 @@ static uint32_t *midx_pack_order(struct write_midx_context *ctx)
647 return pack_order;
648 }
649
650 -static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
651 - struct write_midx_context *ctx)
650 +static void write_midx_reverse_index(struct write_midx_context *ctx,
651 + const char *object_dir,
652 + unsigned char *midx_hash)
653 {
654 struct strbuf buf = STRBUF_INIT;
655 char *tmp_file;
656
657 trace2_region_enter("midx", "write_midx_reverse_index", ctx->repo);
658
658 - strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex_algop(midx_hash,
659 - ctx->repo->hash_algo));
659 + if (ctx->incremental)
660 + get_split_midx_filename_ext(ctx->repo->hash_algo, &buf,
661 + object_dir, midx_hash,
662 + MIDX_EXT_REV);
663 + else
664 + get_midx_filename_ext(ctx->repo->hash_algo, &buf, object_dir,
665 + midx_hash, MIDX_EXT_REV);
666
667 tmp_file = write_rev_file_order(ctx->repo->hash_algo, NULL, ctx->pack_order,
668 ctx->entries_nr, midx_hash, WRITE_REV);
@@ -829,22 +835,29 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr
835 return cb.commits;
836 }
837
832 -static int write_midx_bitmap(struct repository *r, const char *midx_name,
838 +static int write_midx_bitmap(struct write_midx_context *ctx,
839 + const char *object_dir,
840 const unsigned char *midx_hash,
841 struct packing_data *pdata,
842 struct commit **commits,
843 uint32_t commits_nr,
837 - uint32_t *pack_order,
844 unsigned flags)
845 {
846 int ret, i;
847 uint16_t options = 0;
848 struct bitmap_writer writer;
849 struct pack_idx_entry **index;
844 - char *bitmap_name = xstrfmt("%s-%s.bitmap", midx_name,
845 - hash_to_hex_algop(midx_hash, r->hash_algo));
850 + struct strbuf bitmap_name = STRBUF_INIT;
851 +
852 + trace2_region_enter("midx", "write_midx_bitmap", ctx->repo);
853
847 - trace2_region_enter("midx", "write_midx_bitmap", r);
854 + if (ctx->incremental)
855 + get_split_midx_filename_ext(ctx->repo->hash_algo, &bitmap_name,
856 + object_dir, midx_hash,
857 + MIDX_EXT_BITMAP);
858 + else
859 + get_midx_filename_ext(ctx->repo->hash_algo, &bitmap_name,
860 + object_dir, midx_hash, MIDX_EXT_BITMAP);
861
862 if (flags & MIDX_WRITE_BITMAP_HASH_CACHE)
863 options |= BITMAP_OPT_HASH_CACHE;
@@ -861,7 +874,8 @@ static int write_midx_bitmap(struct repository *r, const char *midx_name,
874 for (i = 0; i < pdata->nr_objects; i++)
875 index[i] = &pdata->objects[i].idx;
876
864 - bitmap_writer_init(&writer, r, pdata);
877 + bitmap_writer_init(&writer, ctx->repo, pdata,
878 + ctx->incremental ? ctx->base_midx : NULL);
879 bitmap_writer_show_progress(&writer, flags & MIDX_PROGRESS);
880 bitmap_writer_build_type_index(&writer, index);
881
@@ -879,7 +893,7 @@ static int write_midx_bitmap(struct repository *r, const char *midx_name,
893 * bitmap_writer_finish().
894 */
895 for (i = 0; i < pdata->nr_objects; i++)
882 - index[pack_order[i]] = &pdata->objects[i].idx;
896 + index[ctx->pack_order[i]] = &pdata->objects[i].idx;
897
898 bitmap_writer_select_commits(&writer, commits, commits_nr);
899 ret = bitmap_writer_build(&writer);
@@ -887,14 +901,14 @@ static int write_midx_bitmap(struct repository *r, const char *midx_name,
901 goto cleanup;
902
903 bitmap_writer_set_checksum(&writer, midx_hash);
890 - bitmap_writer_finish(&writer, index, bitmap_name, options);
904 + bitmap_writer_finish(&writer, index, bitmap_name.buf, options);
905
906 cleanup:
907 free(index);
894 - free(bitmap_name);
908 + strbuf_release(&bitmap_name);
909 bitmap_writer_free(&writer);
910
897 - trace2_region_leave("midx", "write_midx_bitmap", r);
911 + trace2_region_leave("midx", "write_midx_bitmap", ctx->repo);
912
913 return ret;
914 }
@@ -1077,8 +1091,6 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1091 ctx.repo = r;
1092
1093 ctx.incremental = !!(flags & MIDX_WRITE_INCREMENTAL);
1080 - if (ctx.incremental && (flags & MIDX_WRITE_BITMAP))
1081 - die(_("cannot write incremental MIDX with bitmap"));
1094
1095 if (ctx.incremental)
1096 strbuf_addf(&midx_name,
@@ -1119,6 +1131,13 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1131 if (ctx.incremental) {
1132 struct multi_pack_index *m = ctx.base_midx;
1133 while (m) {
1134 + if (flags & MIDX_WRITE_BITMAP && load_midx_revindex(m)) {
1135 + error(_("could not load reverse index for MIDX %s"),
1136 + hash_to_hex_algop(get_midx_checksum(m),
1137 + m->repo->hash_algo));
1138 + result = 1;
1139 + goto cleanup;
1140 + }
1141 ctx.num_multi_pack_indexes_before++;
1142 m = m->base_midx;
1143 }
@@ -1387,7 +1406,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1406
1407 if (flags & MIDX_WRITE_REV_INDEX &&
1408 git_env_bool("GIT_TEST_MIDX_WRITE_REV", 0))
1390 - write_midx_reverse_index(midx_name.buf, midx_hash, &ctx);
1409 + write_midx_reverse_index(&ctx, object_dir, midx_hash);
1410
1411 if (flags & MIDX_WRITE_BITMAP) {
1412 struct packing_data pdata;
@@ -1410,8 +1429,8 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
1429 FREE_AND_NULL(ctx.entries);
1430 ctx.entries_nr = 0;
1431
1413 - if (write_midx_bitmap(r, midx_name.buf, midx_hash, &pdata,
1414 - commits, commits_nr, ctx.pack_order,
1432 + if (write_midx_bitmap(&ctx, object_dir,
1433 + midx_hash, &pdata, commits, commits_nr,
1434 flags) < 0) {
1435 error(_("could not write multi-pack bitmap"));
1436 result = 1;
pack-bitmap-write.c
+49 -16
@@ -26,6 +26,8 @@
26 #include "alloc.h"
27 #include "refs.h"
28 #include "strmap.h"
29 +#include "midx.h"
30 +#include "pack-revindex.h"
31
32 struct bitmapped_commit {
33 struct commit *commit;
@@ -43,7 +45,8 @@ static inline int bitmap_writer_nr_selected_commits(struct bitmap_writer *writer
45 }
46
47 void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
46 - struct packing_data *pdata)
48 + struct packing_data *pdata,
49 + struct multi_pack_index *midx)
50 {
51 memset(writer, 0, sizeof(struct bitmap_writer));
52 if (writer->bitmaps)
@@ -51,6 +54,7 @@ void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
54 writer->bitmaps = kh_init_oid_map();
55 writer->pseudo_merge_commits = kh_init_oid_map();
56 writer->to_pack = pdata;
57 + writer->midx = midx;
58
59 string_list_init_dup(&writer->pseudo_merge_groups);
60
@@ -113,6 +117,11 @@ void bitmap_writer_build_type_index(struct bitmap_writer *writer,
117 struct pack_idx_entry **index)
118 {
119 uint32_t i;
120 + uint32_t base_objects = 0;
121 +
122 + if (writer->midx)
123 + base_objects = writer->midx->num_objects +
124 + writer->midx->num_objects_in_base;
125
126 writer->commits = ewah_new();
127 writer->trees = ewah_new();
@@ -142,19 +151,19 @@ void bitmap_writer_build_type_index(struct bitmap_writer *writer,
151
152 switch (real_type) {
153 case OBJ_COMMIT:
145 - ewah_set(writer->commits, i);
154 + ewah_set(writer->commits, i + base_objects);
155 break;
156
157 case OBJ_TREE:
149 - ewah_set(writer->trees, i);
158 + ewah_set(writer->trees, i + base_objects);
159 break;
160
161 case OBJ_BLOB:
153 - ewah_set(writer->blobs, i);
162 + ewah_set(writer->blobs, i + base_objects);
163 break;
164
165 case OBJ_TAG:
157 - ewah_set(writer->tags, i);
166 + ewah_set(writer->tags, i + base_objects);
167 break;
168
169 default:
@@ -207,19 +216,37 @@ void bitmap_writer_push_commit(struct bitmap_writer *writer,
216 static uint32_t find_object_pos(struct bitmap_writer *writer,
217 const struct object_id *oid, int *found)
218 {
210 - struct object_entry *entry = packlist_find(writer->to_pack, oid);
219 + struct object_entry *entry;
220 +
221 + entry = packlist_find(writer->to_pack, oid);
222 + if (entry) {
223 + uint32_t base_objects = 0;
224 + if (writer->midx)
225 + base_objects = writer->midx->num_objects +
226 + writer->midx->num_objects_in_base;
227
212 - if (!entry) {
228 if (found)
214 - *found = 0;
215 - warning("Failed to write bitmap index. Packfile doesn't have full closure "
216 - "(object %s is missing)", oid_to_hex(oid));
217 - return 0;
229 + *found = 1;
230 + return oe_in_pack_pos(writer->to_pack, entry) + base_objects;
231 + } else if (writer->midx) {
232 + uint32_t at, pos;
233 +
234 + if (!bsearch_midx(oid, writer->midx, &at))
235 + goto missing;
236 + if (midx_to_pack_pos(writer->midx, at, &pos) < 0)
237 + goto missing;
238 +
239 + if (found)
240 + *found = 1;
241 + return pos;
242 }
243
244 +missing:
245 if (found)
221 - *found = 1;
222 - return oe_in_pack_pos(writer->to_pack, entry);
246 + *found = 0;
247 + warning("Failed to write bitmap index. Packfile doesn't have full closure "
248 + "(object %s is missing)", oid_to_hex(oid));
249 + return 0;
250 }
251
252 static void compute_xor_offsets(struct bitmap_writer *writer)
@@ -586,7 +613,7 @@ int bitmap_writer_build(struct bitmap_writer *writer)
613 struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
614 struct prio_queue tree_queue = { NULL };
615 struct bitmap_index *old_bitmap;
589 - uint32_t *mapping;
616 + uint32_t *mapping = NULL;
617 int closed = 1; /* until proven otherwise */
618
619 if (writer->show_progress)
@@ -1021,7 +1048,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
1048 struct strbuf tmp_file = STRBUF_INIT;
1049 struct hashfile *f;
1050 off_t *offsets = NULL;
1024 - uint32_t i;
1051 + uint32_t i, base_objects;
1052
1053 struct bitmap_disk_header header;
1054
@@ -1047,6 +1074,12 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
1074 if (options & BITMAP_OPT_LOOKUP_TABLE)
1075 CALLOC_ARRAY(offsets, writer->to_pack->nr_objects);
1076
1077 + if (writer->midx)
1078 + base_objects = writer->midx->num_objects +
1079 + writer->midx->num_objects_in_base;
1080 + else
1081 + base_objects = 0;
1082 +
1083 for (i = 0; i < bitmap_writer_nr_selected_commits(writer); i++) {
1084 struct bitmapped_commit *stored = &writer->selected[i];
1085 int commit_pos = oid_pos(&stored->commit->object.oid, index,
@@ -1055,7 +1088,7 @@ void bitmap_writer_finish(struct bitmap_writer *writer,
1088
1089 if (commit_pos < 0)
1090 BUG(_("trying to write commit not in index"));
1058 - stored->commit_pos = commit_pos;
1091 + stored->commit_pos = commit_pos + base_objects;
1092 }
1093
1094 write_selected_commits_v1(writer, f, offsets);
pack-bitmap.h
+3 -1
@@ -111,6 +111,7 @@ struct bitmap_writer {
111
112 kh_oid_map_t *bitmaps;
113 struct packing_data *to_pack;
114 + struct multi_pack_index *midx; /* if appending to a MIDX chain */
115
116 struct bitmapped_commit *selected;
117 unsigned int selected_nr, selected_alloc;
@@ -125,7 +126,8 @@ struct bitmap_writer {
126 };
127
128 void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
128 - struct packing_data *pdata);
129 + struct packing_data *pdata,
130 + struct multi_pack_index *midx);
131 void bitmap_writer_show_progress(struct bitmap_writer *writer, int show);
132 void bitmap_writer_set_checksum(struct bitmap_writer *writer,
133 const unsigned char *sha1);
t/t5334-incremental-multi-pack-index.sh
+87
@@ -44,4 +44,91 @@ test_expect_success 'convert incremental to non-incremental' '
44
45 compare_results_with_midx 'non-incremental MIDX conversion'
46
47 +write_midx_layer () {
48 + n=1
49 + if test -f $midx_chain
50 + then
51 + n="$(($(wc -l <$midx_chain) + 1))"
52 + fi
53 +
54 + for i in 1 2
55 + do
56 + test_commit $n.$i &&
57 + git repack -d || return 1
58 + done &&
59 + git multi-pack-index write --bitmap --incremental
60 +}
61 +
62 +test_expect_success 'write initial MIDX layer' '
63 + git repack -ad &&
64 + write_midx_layer
65 +'
66 +
67 +test_expect_success 'read bitmap from first MIDX layer' '
68 + git rev-list --test-bitmap 1.2
69 +'
70 +
71 +test_expect_success 'write another MIDX layer' '
72 + write_midx_layer
73 +'
74 +
75 +test_expect_success 'midx verify with multiple layers' '
76 + test_path_is_file "$midx_chain" &&
77 + test_line_count = 2 "$midx_chain" &&
78 +
79 + git multi-pack-index verify
80 +'
81 +
82 +test_expect_success 'read bitmap from second MIDX layer' '
83 + git rev-list --test-bitmap 2.2
84 +'
85 +
86 +test_expect_success 'read earlier bitmap from second MIDX layer' '
87 + git rev-list --test-bitmap 1.2
88 +'
89 +
90 +test_expect_success 'show object from first pack' '
91 + git cat-file -p 1.1
92 +'
93 +
94 +test_expect_success 'show object from second pack' '
95 + git cat-file -p 2.2
96 +'
97 +
98 +for reuse in false single multi
99 +do
100 + test_expect_success "full clone (pack.allowPackReuse=$reuse)" '
101 + rm -fr clone.git &&
102 +
103 + git config pack.allowPackReuse $reuse &&
104 + git clone --no-local --bare . clone.git
105 + '
106 +done
107 +
108 +test_expect_success 'relink existing MIDX layer' '
109 + rm -fr "$midxdir" &&
110 +
111 + GIT_TEST_MIDX_WRITE_REV=1 git multi-pack-index write --bitmap &&
112 +
113 + midx_hash="$(test-tool read-midx --checksum $objdir)" &&
114 +
115 + test_path_is_file "$packdir/multi-pack-index" &&
116 + test_path_is_file "$packdir/multi-pack-index-$midx_hash.bitmap" &&
117 + test_path_is_file "$packdir/multi-pack-index-$midx_hash.rev" &&
118 +
119 + test_commit another &&
120 + git repack -d &&
121 + git multi-pack-index write --bitmap --incremental &&
122 +
123 + test_path_is_missing "$packdir/multi-pack-index" &&
124 + test_path_is_missing "$packdir/multi-pack-index-$midx_hash.bitmap" &&
125 + test_path_is_missing "$packdir/multi-pack-index-$midx_hash.rev" &&
126 +
127 + test_path_is_file "$midxdir/multi-pack-index-$midx_hash.midx" &&
128 + test_path_is_file "$midxdir/multi-pack-index-$midx_hash.bitmap" &&
129 + test_path_is_file "$midxdir/multi-pack-index-$midx_hash.rev" &&
130 + test_line_count = 2 "$midx_chain"
131 +
132 +'
133 +
134 test_done