diff mbox series

[v4,13/13] midx: implement writing incremental MIDX bitmaps

Message ID d0d564685bc66df71b4c3ed1093452e58e0f54fd.1741983492.git.me@ttaylorr.com (mailing list archive)
State Superseded
Headers show
Series midx: incremental multi-pack indexes, part two | expand

Commit Message

Taylor Blau March 14, 2025, 8:19 p.m. UTC
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>
---
 builtin/pack-objects.c                  |  3 +-
 ewah/ewah_bitmap.c                      |  2 +-
 ewah/ewok.h                             |  2 +-
 midx-write.c                            | 57 +++++++++++------
 pack-bitmap-write.c                     | 65 ++++++++++++++-----
 pack-bitmap.c                           | 10 +--
 pack-bitmap.h                           |  4 +-
 t/t5334-incremental-multi-pack-index.sh | 84 +++++++++++++++++++++++++
 8 files changed, 183 insertions(+), 44 deletions(-)

Comments

Jeff King March 18, 2025, 2:16 a.m. UTC | #1
On Fri, Mar 14, 2025 at 04:19:00PM -0400, Taylor Blau wrote:

> +write_midx_layer () {
> +	n=1
> +	if test -f $midx_chain
> +	then
> +		n="$(($(wc -l <$midx_chain) + 1))"
> +	fi
> +
> +	for i in 1 2
> +	do
> +		test_commit $n.$i &&
> +		git repack -d || return 1
> +	done &&
> +	git multi-pack-index write --bitmap --incremental
> +}
> +
> +test_expect_success 'write initial MIDX layer' '
> +	git repack -ad &&
> +	write_midx_layer
> +'
> +
> +test_expect_success 'read bitmap from first MIDX layer' '
> +	git rev-list --test-bitmap 1.2
> +'
> +
> +test_expect_success 'write another MIDX layer' '
> +	write_midx_layer
> +'
> +
> +test_expect_success 'midx verify with multiple layers' '
> +	git multi-pack-index verify
> +'

Perhaps a silly suggestion, but do you want to confirm in one of these
tests that there are in fact multiple layers of bitmaps? (I expect it to
be true, but just trying to cover all bases in the test).

I guess that happens somewhat here:

> +test_expect_success 'relink existing MIDX layer' '
> +	rm -fr "$midxdir" &&
> +
> +	GIT_TEST_MIDX_WRITE_REV=1 git multi-pack-index write --bitmap &&
> +
> +	midx_hash="$(test-tool read-midx --checksum $objdir)" &&
> +
> +	test_path_is_file "$packdir/multi-pack-index" &&
> +	test_path_is_file "$packdir/multi-pack-index-$midx_hash.bitmap" &&
> +	test_path_is_file "$packdir/multi-pack-index-$midx_hash.rev" &&
> +
> +	test_commit another &&
> +	git repack -d &&
> +	git multi-pack-index write --bitmap --incremental &&
> +
> +	test_path_is_missing "$packdir/multi-pack-index" &&
> +	test_path_is_missing "$packdir/multi-pack-index-$midx_hash.bitmap" &&
> +	test_path_is_missing "$packdir/multi-pack-index-$midx_hash.rev" &&
> +
> +	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.midx" &&
> +	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.bitmap" &&
> +	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.rev" &&
> +	test_line_count = 2 "$midx_chain"

where we check that we switched to $midxdir.

-Peff
Elijah Newren March 18, 2025, 5:13 p.m. UTC | #2
On Fri, Mar 14, 2025 at 1:19 PM Taylor Blau <me@ttaylorr.com> wrote:

[...]
> diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> index e92341b8fa..056c410efb 100644
> --- a/ewah/ewah_bitmap.c
> +++ b/ewah/ewah_bitmap.c
> @@ -399,7 +399,7 @@ int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it)
>         return ret;
>  }
>
> -void ewah_or_iterator_free(struct ewah_or_iterator *it)
> +void ewah_or_iterator_release(struct ewah_or_iterator *it)
>  {
>         free(it->its);
>  }
> diff --git a/ewah/ewok.h b/ewah/ewok.h
> index 4b70641045..c29d354236 100644
> --- a/ewah/ewok.h
> +++ b/ewah/ewok.h
> @@ -158,7 +158,7 @@ void ewah_or_iterator_init(struct ewah_or_iterator *it,
>
>  int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it);
>
> -void ewah_or_iterator_free(struct ewah_or_iterator *it);
> +void ewah_or_iterator_release(struct ewah_or_iterator *it);

Was the rename from these last two hunks squashed into the wrong
patch?  Since you're not changing its definition, I'm assuming the
updated name should have been applied to when it was introduced.
Taylor Blau March 20, 2025, 12:14 a.m. UTC | #3
On Mon, Mar 17, 2025 at 10:16:05PM -0400, Jeff King wrote:
> On Fri, Mar 14, 2025 at 04:19:00PM -0400, Taylor Blau wrote:
>
> > +write_midx_layer () {
> > +	n=1
> > +	if test -f $midx_chain
> > +	then
> > +		n="$(($(wc -l <$midx_chain) + 1))"
> > +	fi
> > +
> > +	for i in 1 2
> > +	do
> > +		test_commit $n.$i &&
> > +		git repack -d || return 1
> > +	done &&
> > +	git multi-pack-index write --bitmap --incremental
> > +}
> > +
> > +test_expect_success 'write initial MIDX layer' '
> > +	git repack -ad &&
> > +	write_midx_layer
> > +'
> > +
> > +test_expect_success 'read bitmap from first MIDX layer' '
> > +	git rev-list --test-bitmap 1.2
> > +'
> > +
> > +test_expect_success 'write another MIDX layer' '
> > +	write_midx_layer
> > +'
> > +
> > +test_expect_success 'midx verify with multiple layers' '
> > +	git multi-pack-index verify
> > +'
>
> Perhaps a silly suggestion, but do you want to confirm in one of these
> tests that there are in fact multiple layers of bitmaps? (I expect it to
> be true, but just trying to cover all bases in the test).

I don't think it's a silly suggestion. As you note, we do implicitly
check it further down, but doing something like the following

    test_path_is_dir "$midx_chain" &&
    test_line_count = 2 "$midx_chain" &&

explicitly before calling 'git multi-pack-index verify' would be nice to
have.

Thanks,
Taylor
Taylor Blau March 20, 2025, 12:16 a.m. UTC | #4
On Tue, Mar 18, 2025 at 10:13:03AM -0700, Elijah Newren wrote:
> On Fri, Mar 14, 2025 at 1:19 PM Taylor Blau <me@ttaylorr.com> wrote:
>
> [...]
> > diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> > index e92341b8fa..056c410efb 100644
> > --- a/ewah/ewah_bitmap.c
> > +++ b/ewah/ewah_bitmap.c
> > @@ -399,7 +399,7 @@ int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it)
> >         return ret;
> >  }
> >
> > -void ewah_or_iterator_free(struct ewah_or_iterator *it)
> > +void ewah_or_iterator_release(struct ewah_or_iterator *it)
> >  {
> >         free(it->its);
> >  }
> > diff --git a/ewah/ewok.h b/ewah/ewok.h
> > index 4b70641045..c29d354236 100644
> > --- a/ewah/ewok.h
> > +++ b/ewah/ewok.h
> > @@ -158,7 +158,7 @@ void ewah_or_iterator_init(struct ewah_or_iterator *it,
> >
> >  int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it);
> >
> > -void ewah_or_iterator_free(struct ewah_or_iterator *it);
> > +void ewah_or_iterator_release(struct ewah_or_iterator *it);
>
> Was the rename from these last two hunks squashed into the wrong
> patch?  Since you're not changing its definition, I'm assuming the
> updated name should have been applied to when it was introduced.

Hah! I knew that I made this change, so I was confused in

    https://lore.kernel.org/git/Z9oQ4moLVKh3+vul@nand.local/

when it didn't show up in that patch.

It got rebased out of my local copy of this patch automatically since I
had manually applied the rename to the earlier patch while responding to
that review comment.

Thanks,
Taylor
diff mbox series

Patch

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 58a9b16126..a7e4bb7904 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1397,7 +1397,8 @@  static void write_pack_file(void)
 
 			if (write_bitmap_index) {
 				bitmap_writer_init(&bitmap_writer,
-						   the_repository, &to_pack);
+						   the_repository, &to_pack,
+						   NULL);
 				bitmap_writer_set_checksum(&bitmap_writer, hash);
 				bitmap_writer_build_type_index(&bitmap_writer,
 							       written_list);
diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
index e92341b8fa..056c410efb 100644
--- a/ewah/ewah_bitmap.c
+++ b/ewah/ewah_bitmap.c
@@ -399,7 +399,7 @@  int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it)
 	return ret;
 }
 
-void ewah_or_iterator_free(struct ewah_or_iterator *it)
+void ewah_or_iterator_release(struct ewah_or_iterator *it)
 {
 	free(it->its);
 }
diff --git a/ewah/ewok.h b/ewah/ewok.h
index 4b70641045..c29d354236 100644
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -158,7 +158,7 @@  void ewah_or_iterator_init(struct ewah_or_iterator *it,
 
 int ewah_or_iterator_next(eword_t *next, struct ewah_or_iterator *it);
 
-void ewah_or_iterator_free(struct ewah_or_iterator *it);
+void ewah_or_iterator_release(struct ewah_or_iterator *it);
 
 void ewah_xor(
 	struct ewah_bitmap *ewah_i,
diff --git a/midx-write.c b/midx-write.c
index 48d6558253..0897cbd829 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -647,16 +647,22 @@  static uint32_t *midx_pack_order(struct write_midx_context *ctx)
 	return pack_order;
 }
 
-static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
-				     struct write_midx_context *ctx)
+static void write_midx_reverse_index(struct write_midx_context *ctx,
+				     const char *object_dir,
+				     unsigned char *midx_hash)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *tmp_file;
 
 	trace2_region_enter("midx", "write_midx_reverse_index", ctx->repo);
 
-	strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex_algop(midx_hash,
-								    ctx->repo->hash_algo));
+	if (ctx->incremental)
+		get_split_midx_filename_ext(ctx->repo->hash_algo, &buf,
+					    object_dir, midx_hash,
+					    MIDX_EXT_REV);
+	else
+		get_midx_filename_ext(ctx->repo->hash_algo, &buf, object_dir,
+				      midx_hash, MIDX_EXT_REV);
 
 	tmp_file = write_rev_file_order(ctx->repo->hash_algo, NULL, ctx->pack_order,
 					ctx->entries_nr, midx_hash, WRITE_REV);
@@ -829,22 +835,29 @@  static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr
 	return cb.commits;
 }
 
-static int write_midx_bitmap(struct repository *r, const char *midx_name,
+static int write_midx_bitmap(struct write_midx_context *ctx,
+			     const char *object_dir,
 			     const unsigned char *midx_hash,
 			     struct packing_data *pdata,
 			     struct commit **commits,
 			     uint32_t commits_nr,
-			     uint32_t *pack_order,
 			     unsigned flags)
 {
 	int ret, i;
 	uint16_t options = 0;
 	struct bitmap_writer writer;
 	struct pack_idx_entry **index;
-	char *bitmap_name = xstrfmt("%s-%s.bitmap", midx_name,
-				    hash_to_hex_algop(midx_hash, r->hash_algo));
+	struct strbuf bitmap_name = STRBUF_INIT;
 
-	trace2_region_enter("midx", "write_midx_bitmap", r);
+	trace2_region_enter("midx", "write_midx_bitmap", ctx->repo);
+
+	if (ctx->incremental)
+		get_split_midx_filename_ext(ctx->repo->hash_algo, &bitmap_name,
+					    object_dir, midx_hash,
+					    MIDX_EXT_BITMAP);
+	else
+		get_midx_filename_ext(ctx->repo->hash_algo, &bitmap_name,
+				      object_dir, midx_hash, MIDX_EXT_BITMAP);
 
 	if (flags & MIDX_WRITE_BITMAP_HASH_CACHE)
 		options |= BITMAP_OPT_HASH_CACHE;
@@ -861,7 +874,8 @@  static int write_midx_bitmap(struct repository *r, const char *midx_name,
 	for (i = 0; i < pdata->nr_objects; i++)
 		index[i] = &pdata->objects[i].idx;
 
-	bitmap_writer_init(&writer, r, pdata);
+	bitmap_writer_init(&writer, ctx->repo, pdata,
+			   ctx->incremental ? ctx->base_midx : NULL);
 	bitmap_writer_show_progress(&writer, flags & MIDX_PROGRESS);
 	bitmap_writer_build_type_index(&writer, index);
 
@@ -879,7 +893,7 @@  static int write_midx_bitmap(struct repository *r, const char *midx_name,
 	 * bitmap_writer_finish().
 	 */
 	for (i = 0; i < pdata->nr_objects; i++)
-		index[pack_order[i]] = &pdata->objects[i].idx;
+		index[ctx->pack_order[i]] = &pdata->objects[i].idx;
 
 	bitmap_writer_select_commits(&writer, commits, commits_nr);
 	ret = bitmap_writer_build(&writer);
@@ -887,14 +901,14 @@  static int write_midx_bitmap(struct repository *r, const char *midx_name,
 		goto cleanup;
 
 	bitmap_writer_set_checksum(&writer, midx_hash);
-	bitmap_writer_finish(&writer, index, bitmap_name, options);
+	bitmap_writer_finish(&writer, index, bitmap_name.buf, options);
 
 cleanup:
 	free(index);
-	free(bitmap_name);
+	strbuf_release(&bitmap_name);
 	bitmap_writer_free(&writer);
 
-	trace2_region_leave("midx", "write_midx_bitmap", r);
+	trace2_region_leave("midx", "write_midx_bitmap", ctx->repo);
 
 	return ret;
 }
@@ -1077,8 +1091,6 @@  static int write_midx_internal(struct repository *r, const char *object_dir,
 	ctx.repo = r;
 
 	ctx.incremental = !!(flags & MIDX_WRITE_INCREMENTAL);
-	if (ctx.incremental && (flags & MIDX_WRITE_BITMAP))
-		die(_("cannot write incremental MIDX with bitmap"));
 
 	if (ctx.incremental)
 		strbuf_addf(&midx_name,
@@ -1119,6 +1131,13 @@  static int write_midx_internal(struct repository *r, const char *object_dir,
 	if (ctx.incremental) {
 		struct multi_pack_index *m = ctx.base_midx;
 		while (m) {
+			if (flags & MIDX_WRITE_BITMAP && load_midx_revindex(m)) {
+				error(_("could not load reverse index for MIDX %s"),
+				      hash_to_hex_algop(get_midx_checksum(m),
+							m->repo->hash_algo));
+				result = 1;
+				goto cleanup;
+			}
 			ctx.num_multi_pack_indexes_before++;
 			m = m->base_midx;
 		}
@@ -1387,7 +1406,7 @@  static int write_midx_internal(struct repository *r, const char *object_dir,
 
 	if (flags & MIDX_WRITE_REV_INDEX &&
 	    git_env_bool("GIT_TEST_MIDX_WRITE_REV", 0))
-		write_midx_reverse_index(midx_name.buf, midx_hash, &ctx);
+		write_midx_reverse_index(&ctx, object_dir, midx_hash);
 
 	if (flags & MIDX_WRITE_BITMAP) {
 		struct packing_data pdata;
@@ -1410,8 +1429,8 @@  static int write_midx_internal(struct repository *r, const char *object_dir,
 		FREE_AND_NULL(ctx.entries);
 		ctx.entries_nr = 0;
 
-		if (write_midx_bitmap(r, midx_name.buf, midx_hash, &pdata,
-				      commits, commits_nr, ctx.pack_order,
+		if (write_midx_bitmap(&ctx, object_dir,
+				      midx_hash, &pdata, commits, commits_nr,
 				      flags) < 0) {
 			error(_("could not write multi-pack bitmap"));
 			result = 1;
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 34e86d4994..8a30853d2e 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -26,6 +26,8 @@ 
 #include "alloc.h"
 #include "refs.h"
 #include "strmap.h"
+#include "midx.h"
+#include "pack-revindex.h"
 
 struct bitmapped_commit {
 	struct commit *commit;
@@ -43,7 +45,8 @@  static inline int bitmap_writer_nr_selected_commits(struct bitmap_writer *writer
 }
 
 void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
-			struct packing_data *pdata)
+			struct packing_data *pdata,
+			struct multi_pack_index *midx)
 {
 	memset(writer, 0, sizeof(struct bitmap_writer));
 	if (writer->bitmaps)
@@ -51,6 +54,7 @@  void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
 	writer->bitmaps = kh_init_oid_map();
 	writer->pseudo_merge_commits = kh_init_oid_map();
 	writer->to_pack = pdata;
+	writer->midx = midx;
 
 	string_list_init_dup(&writer->pseudo_merge_groups);
 
@@ -113,6 +117,11 @@  void bitmap_writer_build_type_index(struct bitmap_writer *writer,
 				    struct pack_idx_entry **index)
 {
 	uint32_t i;
+	uint32_t base_objects = 0;
+
+	if (writer->midx)
+		base_objects = writer->midx->num_objects +
+			writer->midx->num_objects_in_base;
 
 	writer->commits = ewah_new();
 	writer->trees = ewah_new();
@@ -142,19 +151,19 @@  void bitmap_writer_build_type_index(struct bitmap_writer *writer,
 
 		switch (real_type) {
 		case OBJ_COMMIT:
-			ewah_set(writer->commits, i);
+			ewah_set(writer->commits, i + base_objects);
 			break;
 
 		case OBJ_TREE:
-			ewah_set(writer->trees, i);
+			ewah_set(writer->trees, i + base_objects);
 			break;
 
 		case OBJ_BLOB:
-			ewah_set(writer->blobs, i);
+			ewah_set(writer->blobs, i + base_objects);
 			break;
 
 		case OBJ_TAG:
-			ewah_set(writer->tags, i);
+			ewah_set(writer->tags, i + base_objects);
 			break;
 
 		default:
@@ -207,19 +216,37 @@  void bitmap_writer_push_commit(struct bitmap_writer *writer,
 static uint32_t find_object_pos(struct bitmap_writer *writer,
 				const struct object_id *oid, int *found)
 {
-	struct object_entry *entry = packlist_find(writer->to_pack, oid);
+	struct object_entry *entry;
+
+	entry = packlist_find(writer->to_pack, oid);
+	if (entry) {
+		uint32_t base_objects = 0;
+		if (writer->midx)
+			base_objects = writer->midx->num_objects +
+				writer->midx->num_objects_in_base;
+
+		if (found)
+			*found = 1;
+		return oe_in_pack_pos(writer->to_pack, entry) + base_objects;
+	} else if (writer->midx) {
+		uint32_t at, pos;
+
+		if (!bsearch_midx(oid, writer->midx, &at))
+			goto missing;
+		if (midx_to_pack_pos(writer->midx, at, &pos) < 0)
+			goto missing;
 
-	if (!entry) {
 		if (found)
-			*found = 0;
-		warning("Failed to write bitmap index. Packfile doesn't have full closure "
-			"(object %s is missing)", oid_to_hex(oid));
-		return 0;
+			*found = 1;
+		return pos;
 	}
 
+missing:
 	if (found)
-		*found = 1;
-	return oe_in_pack_pos(writer->to_pack, entry);
+		*found = 0;
+	warning("Failed to write bitmap index. Packfile doesn't have full closure "
+		"(object %s is missing)", oid_to_hex(oid));
+	return 0;
 }
 
 static void compute_xor_offsets(struct bitmap_writer *writer)
@@ -586,7 +613,7 @@  int bitmap_writer_build(struct bitmap_writer *writer)
 	struct prio_queue queue = { compare_commits_by_gen_then_commit_date };
 	struct prio_queue tree_queue = { NULL };
 	struct bitmap_index *old_bitmap;
-	uint32_t *mapping;
+	uint32_t *mapping = NULL;
 	int closed = 1; /* until proven otherwise */
 
 	if (writer->show_progress)
@@ -1021,7 +1048,7 @@  void bitmap_writer_finish(struct bitmap_writer *writer,
 	struct strbuf tmp_file = STRBUF_INIT;
 	struct hashfile *f;
 	off_t *offsets = NULL;
-	uint32_t i;
+	uint32_t i, base_objects;
 
 	struct bitmap_disk_header header;
 
@@ -1047,6 +1074,12 @@  void bitmap_writer_finish(struct bitmap_writer *writer,
 	if (options & BITMAP_OPT_LOOKUP_TABLE)
 		CALLOC_ARRAY(offsets, writer->to_pack->nr_objects);
 
+	if (writer->midx)
+		base_objects = writer->midx->num_objects +
+			writer->midx->num_objects_in_base;
+	else
+		base_objects = 0;
+
 	for (i = 0; i < bitmap_writer_nr_selected_commits(writer); i++) {
 		struct bitmapped_commit *stored = &writer->selected[i];
 		int commit_pos = oid_pos(&stored->commit->object.oid, index,
@@ -1055,7 +1088,7 @@  void bitmap_writer_finish(struct bitmap_writer *writer,
 
 		if (commit_pos < 0)
 			BUG(_("trying to write commit not in index"));
-		stored->commit_pos = commit_pos;
+		stored->commit_pos = commit_pos + base_objects;
 	}
 
 	write_selected_commits_v1(writer, f, offsets);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 5e6d4ace58..94d1e8474a 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1719,7 +1719,7 @@  static void show_objects_for_type(
 		}
 	}
 
-	ewah_or_iterator_free(&it);
+	ewah_or_iterator_release(&it);
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -1808,7 +1808,7 @@  static void filter_bitmap_exclude_type(struct bitmap_index *bitmap_git,
 			bitmap_unset(to_filter, pos);
 	}
 
-	ewah_or_iterator_free(&it);
+	ewah_or_iterator_release(&it);
 	bitmap_free(tips);
 }
 
@@ -1903,7 +1903,7 @@  static void filter_bitmap_blob_limit(struct bitmap_index *bitmap_git,
 			bitmap_unset(to_filter, pos);
 	}
 
-	ewah_or_iterator_free(&it);
+	ewah_or_iterator_release(&it);
 	bitmap_free(tips);
 }
 
@@ -2552,7 +2552,7 @@  static uint32_t count_object_type(struct bitmap_index *bitmap_git,
 			count++;
 	}
 
-	ewah_or_iterator_free(&it);
+	ewah_or_iterator_release(&it);
 
 	return count;
 }
@@ -3133,7 +3133,7 @@  static off_t get_disk_usage_for_type(struct bitmap_index *bitmap_git,
 		}
 	}
 
-	ewah_or_iterator_free(&it);
+	ewah_or_iterator_release(&it);
 
 	return total;
 }
diff --git a/pack-bitmap.h b/pack-bitmap.h
index d7f4b8b8e9..dd0951088f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -111,6 +111,7 @@  struct bitmap_writer {
 
 	kh_oid_map_t *bitmaps;
 	struct packing_data *to_pack;
+	struct multi_pack_index *midx; /* if appending to a MIDX chain */
 
 	struct bitmapped_commit *selected;
 	unsigned int selected_nr, selected_alloc;
@@ -125,7 +126,8 @@  struct bitmap_writer {
 };
 
 void bitmap_writer_init(struct bitmap_writer *writer, struct repository *r,
-			struct packing_data *pdata);
+			struct packing_data *pdata,
+			struct multi_pack_index *midx);
 void bitmap_writer_show_progress(struct bitmap_writer *writer, int show);
 void bitmap_writer_set_checksum(struct bitmap_writer *writer,
 				const unsigned char *sha1);
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 26257e5660..46d1f0b864 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -44,4 +44,88 @@  test_expect_success 'convert incremental to non-incremental' '
 
 compare_results_with_midx 'non-incremental MIDX conversion'
 
+write_midx_layer () {
+	n=1
+	if test -f $midx_chain
+	then
+		n="$(($(wc -l <$midx_chain) + 1))"
+	fi
+
+	for i in 1 2
+	do
+		test_commit $n.$i &&
+		git repack -d || return 1
+	done &&
+	git multi-pack-index write --bitmap --incremental
+}
+
+test_expect_success 'write initial MIDX layer' '
+	git repack -ad &&
+	write_midx_layer
+'
+
+test_expect_success 'read bitmap from first MIDX layer' '
+	git rev-list --test-bitmap 1.2
+'
+
+test_expect_success 'write another MIDX layer' '
+	write_midx_layer
+'
+
+test_expect_success 'midx verify with multiple layers' '
+	git multi-pack-index verify
+'
+
+test_expect_success 'read bitmap from second MIDX layer' '
+	git rev-list --test-bitmap 2.2
+'
+
+test_expect_success 'read earlier bitmap from second MIDX layer' '
+	git rev-list --test-bitmap 1.2
+'
+
+test_expect_success 'show object from first pack' '
+	git cat-file -p 1.1
+'
+
+test_expect_success 'show object from second pack' '
+	git cat-file -p 2.2
+'
+
+for reuse in false single multi
+do
+	test_expect_success "full clone (pack.allowPackReuse=$reuse)" '
+		rm -fr clone.git &&
+
+		git config pack.allowPackReuse $reuse &&
+		git clone --no-local --bare . clone.git
+	'
+done
+
+test_expect_success 'relink existing MIDX layer' '
+	rm -fr "$midxdir" &&
+
+	GIT_TEST_MIDX_WRITE_REV=1 git multi-pack-index write --bitmap &&
+
+	midx_hash="$(test-tool read-midx --checksum $objdir)" &&
+
+	test_path_is_file "$packdir/multi-pack-index" &&
+	test_path_is_file "$packdir/multi-pack-index-$midx_hash.bitmap" &&
+	test_path_is_file "$packdir/multi-pack-index-$midx_hash.rev" &&
+
+	test_commit another &&
+	git repack -d &&
+	git multi-pack-index write --bitmap --incremental &&
+
+	test_path_is_missing "$packdir/multi-pack-index" &&
+	test_path_is_missing "$packdir/multi-pack-index-$midx_hash.bitmap" &&
+	test_path_is_missing "$packdir/multi-pack-index-$midx_hash.rev" &&
+
+	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.midx" &&
+	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.bitmap" &&
+	test_path_is_file "$midxdir/multi-pack-index-$midx_hash.rev" &&
+	test_line_count = 2 "$midx_chain"
+
+'
+
 test_done