From: Taylor Blau <me@ttaylorr.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>,
Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about incremental MIDXs
Date: Fri, 28 Feb 2025 19:19:57 -0500 [thread overview]
Message-ID: <Z8JSreTnEFlocYQ9@nand.local> (raw)
In-Reply-To: <Z8GJdv5FwDP_k1et@pks.im>
On Fri, Feb 28, 2025 at 11:01:26AM +0100, Patrick Steinhardt wrote:
> On Tue, Nov 19, 2024 at 05:07:38PM -0500, Taylor Blau wrote:
> > diff --git a/pack-bitmap.c b/pack-bitmap.c
> > index 1dddb242434..02864a0e1f7 100644
> > --- a/pack-bitmap.c
> > +++ b/pack-bitmap.c
> > @@ -2564,13 +2584,57 @@ static void test_show_commit(struct commit *commit, void *data)
> > display_progress(tdata->prg, ++tdata->seen);
> > }
> >
> > +static uint32_t bitmap_total_entry_count(struct bitmap_index *bitmap_git)
> > +{
> > + uint32_t total = 0;
> > + do {
> > + total = st_add(total, bitmap_git->entry_count);
> > + bitmap_git = bitmap_git->base;
> > + } while (bitmap_git);
> > +
> > + return total;
> > +}
> > +
> > +static void prepare_bitmap_test_data(struct bitmap_test_data *tdata,
> > + struct bitmap_index *bitmap_git)
>
> Nit: according to our style guide this should be called
> `bitmap_test_data_prepare()`.
Ah, I didn't know about that part of the style guide. It looks like it
comes from your 10f0723c8d (Documentation: document idiomatic function
names, 2024-07-30), which is semi-recent ;-).
> > +{
> > + memset(tdata, 0, sizeof(struct bitmap_test_data));
> > +
> > + tdata->bitmap_git = bitmap_git;
> > + tdata->base = bitmap_new();
> > + tdata->commits = ewah_to_bitmap(bitmap_git->commits);
> > + tdata->trees = ewah_to_bitmap(bitmap_git->trees);
> > + tdata->blobs = ewah_to_bitmap(bitmap_git->blobs);
> > + tdata->tags = ewah_to_bitmap(bitmap_git->tags);
> > +
> > + if (bitmap_git->base) {
> > + CALLOC_ARRAY(tdata->base_tdata, 1);
> > + prepare_bitmap_test_data(tdata->base_tdata, bitmap_git->base);
> > + }
> > +}
> > +
> > +static void free_bitmap_test_data(struct bitmap_test_data *tdata)
>
> Same nit here, this should be called `bitmap_test_data_free()`. In fact,
> it should be called `bitmap_test_data_release()`, because we don't free
> `tadata` itself.
Noted, and adjusted!
> > @@ -2579,17 +2643,26 @@ void test_bitmap_walk(struct rev_info *revs)
> > if (revs->pending.nr != 1)
> > die(_("you must specify exactly one commit to test"));
> >
> > - fprintf_ln(stderr, "Bitmap v%d test (%d entries%s)",
> > + fprintf_ln(stderr, "Bitmap v%d test (%d entries%s, %d total)",
> > bitmap_git->version,
> > bitmap_git->entry_count,
> > - bitmap_git->table_lookup ? "" : " loaded");
> > + bitmap_git->table_lookup ? "" : " loaded",
> > + bitmap_total_entry_count(bitmap_git));
> >
> > root = revs->pending.objects[0].item;
> > - bm = bitmap_for_commit(bitmap_git, (struct commit *)root);
> > + bm = find_bitmap_for_commit(bitmap_git, (struct commit *)root, &found);
> >
> > if (bm) {
> > fprintf_ln(stderr, "Found bitmap for '%s'. %d bits / %08x checksum",
> > - oid_to_hex(&root->oid), (int)bm->bit_size, ewah_checksum(bm));
> > + oid_to_hex(&root->oid),
> > + (int)bm->bit_size, ewah_checksum(bm));
> > +
> > + if (bitmap_is_midx(found))
> > + fprintf_ln(stderr, "Located via MIDX '%s'.",
> > + hash_to_hex(get_midx_checksum(found->midx)));
> > + else
> > + fprintf_ln(stderr, "Located via pack '%s'.",
> > + hash_to_hex(found->pack->hash));
> >
> > result = ewah_to_bitmap(bm);
> > }
>
> I'm a bit surprised that this doesn't result in any changes required in
> our tests.
That's intentional, the aim of this commit is really just to get us
ready for the eventuality of having incremental MIDXs. In the meantime,
we don't disrupt the behavior of --test-bitmap for single-layer MIDX
bitmaps.
Thanks,
Taylor
next prev parent reply other threads:[~2025-03-01 0:19 UTC|newest]
Thread overview: 136+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-15 21:01 [PATCH 00/13] midx: incremental multi-pack indexes, part two Taylor Blau
2024-08-15 21:01 ` [PATCH 01/13] Documentation: describe incremental MIDX bitmaps Taylor Blau
2024-08-15 21:01 ` [PATCH 02/13] pack-revindex: prepare for " Taylor Blau
2024-08-15 21:01 ` [PATCH 03/13] pack-bitmap.c: open and store incremental bitmap layers Taylor Blau
2024-08-15 21:01 ` [PATCH 04/13] pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs Taylor Blau
2024-08-15 21:01 ` [PATCH 05/13] pack-bitmap.c: teach `show_objects_for_type()` " Taylor Blau
2024-08-15 21:01 ` [PATCH 06/13] pack-bitmap.c: support bitmap pack-reuse with " Taylor Blau
2024-08-15 21:01 ` [PATCH 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about " Taylor Blau
2024-08-15 21:01 ` [PATCH 08/13] pack-bitmap.c: compute disk-usage with " Taylor Blau
2024-08-15 21:01 ` [PATCH 09/13] pack-bitmap.c: apply pseudo-merge commits " Taylor Blau
2024-08-15 21:01 ` [PATCH 10/13] ewah: implement `struct ewah_or_iterator` Taylor Blau
2024-08-15 21:01 ` [PATCH 11/13] pack-bitmap.c: keep track of each layer's type bitmaps Taylor Blau
2024-08-15 21:01 ` [PATCH 12/13] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators Taylor Blau
2024-08-15 21:01 ` [PATCH 13/13] midx: implement writing incremental MIDX bitmaps Taylor Blau
2024-08-15 22:28 ` [PATCH v2 00/13] midx: incremental multi-pack indexes, part two Taylor Blau
2024-08-15 22:28 ` [PATCH v2 01/13] Documentation: describe incremental MIDX bitmaps Taylor Blau
2024-08-15 22:28 ` [PATCH v2 02/13] pack-revindex: prepare for " Taylor Blau
2024-08-15 22:28 ` [PATCH v2 03/13] pack-bitmap.c: open and store incremental bitmap layers Taylor Blau
2024-08-15 22:29 ` [PATCH v2 04/13] pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs Taylor Blau
2024-08-15 22:29 ` [PATCH v2 05/13] pack-bitmap.c: teach `show_objects_for_type()` " Taylor Blau
2024-08-15 22:29 ` [PATCH v2 06/13] pack-bitmap.c: support bitmap pack-reuse with " Taylor Blau
2024-08-15 22:29 ` [PATCH v2 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about " Taylor Blau
2024-08-15 22:29 ` [PATCH v2 08/13] pack-bitmap.c: compute disk-usage with " Taylor Blau
2024-08-15 22:29 ` [PATCH v2 09/13] pack-bitmap.c: apply pseudo-merge commits " Taylor Blau
2024-08-15 22:29 ` [PATCH v2 10/13] ewah: implement `struct ewah_or_iterator` Taylor Blau
2024-08-15 22:29 ` [PATCH v2 11/13] pack-bitmap.c: keep track of each layer's type bitmaps Taylor Blau
2024-08-15 22:29 ` [PATCH v2 12/13] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators Taylor Blau
2024-08-15 22:29 ` [PATCH v2 13/13] midx: implement writing incremental MIDX bitmaps Taylor Blau
2024-08-28 17:55 ` [PATCH] fixup! " Junio C Hamano
2024-08-28 18:33 ` Jeff King
2024-08-29 18:57 ` Taylor Blau
2024-08-29 19:27 ` Jeff King
2024-11-19 20:56 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 00/13] midx: incremental multi-pack indexes, part two Taylor Blau
2024-11-19 22:07 ` [PATCH v3 01/13] Documentation: describe incremental MIDX bitmaps Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-02-28 23:26 ` Taylor Blau
2025-03-03 10:54 ` Patrick Steinhardt
2024-11-19 22:07 ` [PATCH v3 02/13] pack-revindex: prepare for " Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-02-28 23:39 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 03/13] pack-bitmap.c: open and store incremental bitmap layers Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-02-28 23:49 ` Taylor Blau
2025-03-03 10:55 ` Patrick Steinhardt
2024-11-19 22:07 ` [PATCH v3 04/13] pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:12 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 05/13] pack-bitmap.c: teach `show_objects_for_type()` " Taylor Blau
2024-11-19 22:07 ` [PATCH v3 06/13] pack-bitmap.c: support bitmap pack-reuse with " Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:16 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about " Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:19 ` Taylor Blau [this message]
2024-11-19 22:07 ` [PATCH v3 08/13] pack-bitmap.c: compute disk-usage with " Taylor Blau
2024-11-19 22:07 ` [PATCH v3 09/13] pack-bitmap.c: apply pseudo-merge commits " Taylor Blau
2024-11-19 22:07 ` [PATCH v3 10/13] ewah: implement `struct ewah_or_iterator` Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:22 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 11/13] pack-bitmap.c: keep track of each layer's type bitmaps Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:26 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 12/13] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:28 ` Taylor Blau
2024-11-19 22:07 ` [PATCH v3 13/13] midx: implement writing incremental MIDX bitmaps Taylor Blau
2025-02-28 10:01 ` Patrick Steinhardt
2025-03-01 0:31 ` Taylor Blau
2024-11-20 8:49 ` [PATCH v3 00/13] midx: incremental multi-pack indexes, part two Junio C Hamano
2025-03-14 20:18 ` [PATCH v4 " Taylor Blau
2025-03-14 20:18 ` [PATCH v4 01/13] Documentation: describe incremental MIDX bitmaps Taylor Blau
2025-03-18 1:16 ` Jeff King
2025-03-18 23:11 ` Taylor Blau
2025-03-18 2:42 ` Elijah Newren
2025-03-18 23:19 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 02/13] pack-revindex: prepare for " Taylor Blau
2025-03-18 1:27 ` Jeff King
2025-03-19 0:02 ` Taylor Blau
2025-03-19 0:07 ` Taylor Blau
2025-03-26 18:08 ` Jeff King
2025-03-18 2:43 ` Elijah Newren
2025-03-19 0:03 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 03/13] pack-bitmap.c: open and store incremental bitmap layers Taylor Blau
2025-03-18 4:13 ` Elijah Newren
2025-03-19 0:08 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 04/13] pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs Taylor Blau
2025-03-18 1:38 ` Jeff King
2025-03-19 0:13 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 05/13] pack-bitmap.c: teach `show_objects_for_type()` " Taylor Blau
2025-03-14 20:18 ` [PATCH v4 06/13] pack-bitmap.c: support bitmap pack-reuse with " Taylor Blau
2025-03-18 4:13 ` Elijah Newren
2025-03-19 0:17 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about " Taylor Blau
2025-03-18 5:31 ` Elijah Newren
2025-03-19 0:30 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 08/13] pack-bitmap.c: compute disk-usage with " Taylor Blau
2025-03-18 1:41 ` Jeff King
2025-03-19 0:30 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 09/13] pack-bitmap.c: apply pseudo-merge commits " Taylor Blau
2025-03-14 20:18 ` [PATCH v4 10/13] ewah: implement `struct ewah_or_iterator` Taylor Blau
2025-03-18 1:44 ` Jeff King
2025-03-19 0:33 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 11/13] pack-bitmap.c: keep track of each layer's type bitmaps Taylor Blau
2025-03-18 2:01 ` Jeff King
2025-03-19 0:38 ` Taylor Blau
2025-03-18 6:43 ` Elijah Newren
2025-03-19 0:39 ` Taylor Blau
2025-03-14 20:18 ` [PATCH v4 12/13] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators Taylor Blau
2025-03-18 2:05 ` Jeff King
2025-03-19 23:02 ` Taylor Blau
2025-03-14 20:19 ` [PATCH v4 13/13] midx: implement writing incremental MIDX bitmaps Taylor Blau
2025-03-18 2:16 ` Jeff King
2025-03-20 0:14 ` Taylor Blau
2025-03-18 17:13 ` Elijah Newren
2025-03-20 0:16 ` Taylor Blau
2025-03-18 2:21 ` [PATCH v4 00/13] midx: incremental multi-pack indexes, part two Jeff King
2025-03-20 0:18 ` Taylor Blau
2025-03-20 17:56 ` [PATCH v5 00/14] " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 01/14] Documentation: remove a "future work" item from the MIDX docs Taylor Blau
2025-03-20 17:56 ` [PATCH v5 02/14] Documentation: describe incremental MIDX bitmaps Taylor Blau
2025-03-20 17:56 ` [PATCH v5 03/14] pack-revindex: prepare for " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 04/14] pack-bitmap.c: open and store incremental bitmap layers Taylor Blau
2025-03-20 17:56 ` [PATCH v5 05/14] pack-bitmap.c: teach `bitmap_for_commit()` about incremental MIDXs Taylor Blau
2025-03-20 17:56 ` [PATCH v5 06/14] pack-bitmap.c: teach `show_objects_for_type()` " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 07/14] pack-bitmap.c: support bitmap pack-reuse with " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 08/14] pack-bitmap.c: teach `rev-list --test-bitmap` about " Taylor Blau
2025-03-20 17:56 ` Taylor Blau
2025-03-20 17:58 ` Taylor Blau
2025-03-20 17:56 ` [PATCH v5 09/14] pack-bitmap.c: compute disk-usage with " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 10/14] pack-bitmap.c: apply pseudo-merge commits " Taylor Blau
2025-03-20 17:56 ` [PATCH v5 11/14] ewah: implement `struct ewah_or_iterator` Taylor Blau
2025-03-20 17:57 ` [PATCH v5 12/14] pack-bitmap.c: keep track of each layer's type bitmaps Taylor Blau
2025-03-20 17:57 ` [PATCH v5 13/14] pack-bitmap.c: use `ewah_or_iterator` for type bitmap iterators Taylor Blau
2025-03-20 17:57 ` [PATCH v5 14/14] midx: implement writing incremental MIDX bitmaps Taylor Blau
2025-03-20 20:00 ` [PATCH v5 00/14] midx: incremental multi-pack indexes, part two Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z8JSreTnEFlocYQ9@nand.local \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=newren@gmail.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).