Re: [PATCH v4 01/13] Documentation: describe incremental MIDX bitmaps

[Taylor Blau <me@ttaylorr.com>]

On Mon, Mar 17, 2025 at 07:42:54PM -0700, Elijah Newren wrote:
> > +In the incremental MIDX design, we extend this definition to include
> > +objects from multiple layers of the MIDX chain. The pseudo-pack order
> > +for incremental MIDXs is determined by concatenating the pseudo-pack
> > +ordering for each layer of the MIDX chain in order. Formally two objects
> > +`o1` and `o2` are compared as follows:
> > +
> > +1. If `o1` appears in an earlier layer of the MIDX chain than `o2`, then
> > +  `o1` is considered less than `o2`.
>
> For sorting order, 'less than' doesn't tell us if you are sorting
> smallest to greatest or greatest to smallest.  Maybe "less than (so
> its order is earlier than) `o2'" ?

Oh, good suggestion. I found the alternative a little verbose, but went
with "sorts ahead of" instead of "less than".

> > +
> > +2. Otherwise, if `o1` and `o2` appear in the same MIDX layer, and that
> > +   MIDX layer has no base, then if one of `pack(o1)` and `pack(o2)` is
> > +   preferred and the other is not, then the preferred one sorts first. If
> > +   there is a base layer (i.e. the MIDX layer is not the first layer in
> > +   the chain), then if `pack(o1)` appears earlier in that MIDX layer's
> > +   pack order, than `o1` is less than `o2`. Likewise if `pack(o2)`
>
> s/than/then/
>
> > +   appears earlier, than the opposite is true.
>
> s/than/then/

Good catch on both accounts ;-).

> > +The structure of a `*.bitmap` file belonging to an incremental MIDX
> > +chain is identical to that of a non-incremental MIDX bitmap, or a
> > +classic single-pack bitmap. Since objects are added to the end of the
> > +incremental MIDX's pseudo-pack order (see: above), it is possible to
>
> drop the colon?

Yep, dropped.

> > +extend a bitmap when appending to the end of a MIDX chain.
> > +
> > +(Note: it is possible likewise to compress a contiguous sequence of MIDX
> > +incremental layers, and their `*.bitmap`(s) into a single layer and
> > +`*.bitmap`, but this is not yet implemented.)
>
> "`*.bitmap`(s)" feels slightly awkward and only saves 2 characters.
> Maybe just "`*.bitmap` files"?

Fair suggestion, sure!

> >  Future Work
> >  -----------
>
> Should the patch also remove the first item from Future Work, since
> this series is implementing it?

Hah, that was quite satisfying to do. I moved that to its own commit,
though, since this series doesn't implement incremental MIDXs, but
bitmap support for them. Incremental MIDXs were "done" as of b9497848df
(Merge branch 'tb/incremental-midx-part-1', 2024-08-19).

Thanks,
Taylor