On Mon, May 06, 2024 at 02:48:10PM -0400, Taylor Blau wrote:
> On Mon, May 06, 2024 at 01:52:56PM +0200, Patrick Steinhardt wrote:
> > On Mon, Apr 29, 2024 at 04:43:15PM -0400, Taylor Blau wrote:
> > [snip]
> > > @@ -46,6 +48,11 @@ struct bitmap_writer {
> > >
> > > static struct bitmap_writer writer;
> > >
> > > +static inline int bitmap_writer_selected_nr(void)
> > > +{
> > > + return writer.selected_nr - writer.pseudo_merges_nr;
> > > +}
> >
> > This function may use a comment to explain what its meaning actually is.
> > Like, `bitmap_writer_selected_nr()` is obviously not the same as the
> > `selected_nr` of the `bitmap_writer`, which is quite confusing. So why
> > do we subtract values and why are there two different `selected_nr`s?
>
> selected_nr is the total number of bitmaps we are writing (including
> pseudo-merges), and writer.pseudo_merges_nr is the number of those
> bitmaps which are pseudo-merges.
>
> I renamed this function to bitmap_writer_nr_selected_commits() which
> should clarify things, let me know if that works!
Yup, that's clearer, thanks.
> > [snip]
> > > diff --git a/pack-bitmap.h b/pack-bitmap.h
> > > index dae2d68a338..ca9acd2f735 100644
> > > --- a/pack-bitmap.h
> > > +++ b/pack-bitmap.h
> > > @@ -21,6 +21,7 @@ struct bitmap_disk_header {
> > > unsigned char checksum[GIT_MAX_RAWSZ];
> > > };
> > >
> > > +#define BITMAP_PSEUDO_MERGE (1u<<21)
> > > #define NEEDS_BITMAP (1u<<22)
> >
> > This flag is already used by "builtin/pack-objects.c", which may be fine.
> > But in any case, shouldn't we update "object.h" with both of these flags?
>
> I can't see where in builtin/pack-objects.c this flag is used. The table
> in object.h says that bit 21 is used in:
Yeah, no idea. I must've been seeing ghosts here.
> - list-objects-filter.c
> - builtin/index-pack.c
> - builtin/unpack-objects.c
>
> But I think those are all fine. We don't call unpack-objects from the
> bitmap writing paths, and the same is true of index-pack (since we're
> writing the pack out directly).
>
> list-objects-filter.c should also be OK, since I am 99% sure that these
> two code paths do not collide, but even if they do, that field is only
> set on tree objects from the list-objects-filter.c code path, and the
> new bits in pack-bitmap.h are only set on commit objects.
Okay, thanks for the explanation!
Patrick
Attachment:
signature.asc
Description: PGP signature
