| Message ID | 650b8c8c21b7e8a6e4f65d9b47185a875f0022bb.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:13PM -0400, Taylor Blau wrote: > In a similar fashion as in previous commits, teach the function > `nth_bitmapped_pack()` about incremental MIDXs by translating the given > `pack_int_id` from the concatenated lexical order to a MIDX-local > lexical position. > > When accessing the containing MIDX's array of packs, use the local pack > ID. Likewise, when reading the 'BTMP' chunk, use the MIDX-local offset > when accessing the data within that chunk. OK, makes sense. > int nth_bitmapped_pack(struct repository *r, struct multi_pack_index *m, > struct bitmapped_pack *bp, uint32_t pack_int_id) > { > + uint32_t local_pack_int_id = midx_for_pack(&m, pack_int_id); > + Heh, after the last one reused the "n" variable, now we are back to a separate local variable. Not wrong, but curious to go back and forth. -Peff
On Thu, Aug 01, 2024 at 05:39:47AM -0400, Jeff King wrote: > > int nth_bitmapped_pack(struct repository *r, struct multi_pack_index *m, > > struct bitmapped_pack *bp, uint32_t pack_int_id) > > { > > + uint32_t local_pack_int_id = midx_for_pack(&m, pack_int_id); > > + > > Heh, after the last one reused the "n" variable, now we are back to a > separate local variable. Not wrong, but curious to go back and forth. This one we care about having both, for a couple of reasons: prepare_midx_pack() still expects us to have the global pack_int_id, and just as well for bp->pack_int_id. We could write this as: pack_int_id = midx_for_pack(&m, pack_int_id); if (prepare_midx_pack(r, m, pack_int_id + m->num_packs_in_base)) return -1; But I found it easier to have a separate local_-prefixed variable for when referring to the MIDX-local pack identifier. I'll add a short note in the commit message explaining why we took this approach in this commit. Thanks, Taylor
diff --git a/midx.c b/midx.c index d470a88755..b6c3cd3e59 100644 --- a/midx.c +++ b/midx.c @@ -310,17 +310,19 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, int nth_bitmapped_pack(struct repository *r, struct multi_pack_index *m, struct bitmapped_pack *bp, uint32_t pack_int_id) { + uint32_t local_pack_int_id = midx_for_pack(&m, pack_int_id); + if (!m->chunk_bitmapped_packs) return error(_("MIDX does not contain the BTMP chunk")); if (prepare_midx_pack(r, m, pack_int_id)) return error(_("could not load bitmapped pack %"PRIu32), pack_int_id); - bp->p = m->packs[pack_int_id]; + bp->p = m->packs[local_pack_int_id]; bp->bitmap_pos = get_be32((char *)m->chunk_bitmapped_packs + - MIDX_CHUNK_BITMAPPED_PACKS_WIDTH * pack_int_id); + MIDX_CHUNK_BITMAPPED_PACKS_WIDTH * local_pack_int_id); bp->bitmap_nr = get_be32((char *)m->chunk_bitmapped_packs + - MIDX_CHUNK_BITMAPPED_PACKS_WIDTH * pack_int_id + + MIDX_CHUNK_BITMAPPED_PACKS_WIDTH * local_pack_int_id + sizeof(uint32_t)); bp->pack_int_id = pack_int_id;
In a similar fashion as in previous commits, teach the function `nth_bitmapped_pack()` about incremental MIDXs by translating the given `pack_int_id` from the concatenated lexical order to a MIDX-local lexical position. When accessing the containing MIDX's array of packs, use the local pack ID. Likewise, when reading the 'BTMP' chunk, use the MIDX-local offset when accessing the data within that chunk. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- midx.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)