Re: [PATCH v2 10/19] midx: teach `fill_midx_entry()` about incremental MIDXs

From: Jeff King <peff@peff.net>
Date: Aug 1, 2024, 10:12 UTC
Message-ID: <20240801101215.GJ1159276@coredump.intra.peff.net>
In-Reply-To: <2b335c45ae7832b886ef9adccc2530e4ca53d267.1721250704.git.me@ttaylorr.com>

On Wed, Jul 17, 2024 at 05:12:25PM -0400, Taylor Blau wrote:

Show 12 quoted lines

> After that point, we only need to make two special considerations within
> this function:
> 
>   - First, the pack_int_id returned to us by `nth_midxed_pack_int_id()`
>     is a position in the concatenated lexical order of packs, so we must
>     ensure that we subtract `m->num_packs_in_base` before accessing the
>     MIDX-local `packs` array.
> 
>   - Second, we must avoid translating the `pos` back to a MIDX-local
>     index, since we use it as an argument to `nth_midxed_offset()` which
>     expects a position relative to the concatenated lexical order of
>     objects.

OK. I think this is correct, but this would be another place where we could use an nth_midxed_offset_one() function if we had one.

My thinking was that we'd avoid walking back over the midx chain again. But I guess we don't actually do that, because our midx_for_object() will have overwritten our "m" variable, as well. So inside nth_midxed_offset_one() we'll immediately realize that the global position is inside the midx we passed in. A little extra arithmetic, but there's no pointer chasing.

-Peff