| Message ID | 594386da10bc3f3d6364916201438bf453b70098.1721250704.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | midx: incremental multi-pack indexes, part one | expand |
On Wed, Jul 17, 2024 at 05:12:38PM -0400, Taylor Blau wrote: > The function `midx_fanout_add_midx_fanout()` is used to help construct > the fanout table when generating a MIDX by reusing data from an existing > MIDX. I'm not sure I understand the original function enough to know if we're doing the right thing. But I notice that after your series, we can only get into midx_fanout_add_midx_fanout() if !ctx->incremental. So is this code even used for an incremental midx? Or is it used if we are writing a non-incremental midx, but trying to reuse data from a chained one? -Peff
On Thu, Aug 01, 2024 at 06:29:06AM -0400, Jeff King wrote: > On Wed, Jul 17, 2024 at 05:12:38PM -0400, Taylor Blau wrote: > > > The function `midx_fanout_add_midx_fanout()` is used to help construct > > the fanout table when generating a MIDX by reusing data from an existing > > MIDX. > > I'm not sure I understand the original function enough to know if we're > doing the right thing. But I notice that after your series, we can only > get into midx_fanout_add_midx_fanout() if !ctx->incremental. So is this > code even used for an incremental midx? Originally it was, but after 0c5a62f14b (midx-write.c: do not read existing MIDX with `packs_to_include`, 2024-06-11) we no longer use this function in that path. But... > Or is it used if we are writing a non-incremental midx, but trying to > reuse data from a chained one? ...we would use it in this one, so I think the patch stands. I added a note at the end of the commit message to make sure that we don't forget which paths do and don't reach this function. Thanks, Taylor
diff --git a/midx-write.c b/midx-write.c index 478b42e720..d5275d719b 100644 --- a/midx-write.c +++ b/midx-write.c @@ -196,7 +196,7 @@ static int nth_midxed_pack_midx_entry(struct multi_pack_index *m, struct pack_midx_entry *e, uint32_t pos) { - if (pos >= m->num_objects) + if (pos >= m->num_objects + m->num_objects_in_base) return 1; nth_midxed_object_oid(&e->oid, m, pos); @@ -247,12 +247,16 @@ static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout, uint32_t cur_fanout, int preferred_pack) { - uint32_t start = 0, end; + uint32_t start = m->num_objects_in_base, end; uint32_t cur_object; + if (m->base_midx) + midx_fanout_add_midx_fanout(fanout, m->base_midx, cur_fanout, + preferred_pack); + if (cur_fanout) - start = ntohl(m->chunk_oid_fanout[cur_fanout - 1]); - end = ntohl(m->chunk_oid_fanout[cur_fanout]); + start += ntohl(m->chunk_oid_fanout[cur_fanout - 1]); + end = m->num_objects_in_base + ntohl(m->chunk_oid_fanout[cur_fanout]); for (cur_object = start; cur_object < end; cur_object++) { if ((preferred_pack > -1) &&
The function `midx_fanout_add_midx_fanout()` is used to help construct the fanout table when generating a MIDX by reusing data from an existing MIDX. Prepare this function to work with incremental MIDXs by making a few changes: - The bounds checks need to be adjusted to start object lookups taking into account the number of objects in the previous MIDX layer (i.e., by starting the lookups at position `m->num_objects_in_base` instead of position 0). - Likewise, the bounds checks need to end at `m->num_objects_in_base` objects after `m->num_objects`. - Finally, `midx_fanout_add_midx_fanout()` needs to recur on earlier MIDX layers when dealing with an incremental MIDX chain by calling itself when given a MIDX with a non-NULL `base_midx`. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- midx-write.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)