[PATCH v2 10/16] reftable/block: store block pointer in the block iterator
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Apr 7, 2025, 13:16 UTC
- Message-ID
- <20250407-pks-reftable-polishing-v2-10-316c4ff10571@pks.im>
- In-Reply-To
- <20250407-pks-reftable-polishing-v2-0-316c4ff10571@pks.im>
The block iterator requires access to a bunch of data from the underlying `reftable_block` that it is iterating over. This data is stored by copying over relevant data into a separate set of variables. This has multiple downsides:
- We require more storage space than necessary. This is more of a
theoretical issue as we shouldn't ever have many blocks. - We have to perform more bookkeeping, and the variable names are
inconsistent across the two data structures. This can lead to some
confusion. - The lifetime of the block iterator is tied to the block anyway, but
we hide that a bit by only storing pointers pointing into the block.There isn't really any good reason why we rip out parts of the block instead of storing a pointer to the block itself.
Refactor the code to do so. Despite being simpler, it also allows us to decouple the lifetime of the block iterator from seeking in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> --- reftable/block.c | 22 ++++++++-------------- reftable/block.h | 4 +--- 2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/reftable/block.c b/reftable/block.c index 844da397833..a425dd7abf3 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -381,13 +381,11 @@ static uint32_t block_restart_offset(const struct reftable_block *b, size_t idx) return reftable_get_be24(b->block_data.data + b->restart_off + 3 * idx); } -void block_iter_seek_start(struct block_iter *it, const struct reftable_block *b) +void block_iter_seek_start(struct block_iter *it, const struct reftable_block *block) { - it->block = b->block_data.data; - it->block_len = b->restart_off; - it->hash_size = b->hash_size; + it->block = block; reftable_buf_reset(&it->last_key); - it->next_off = b->header_off + 4; + it->next_off = block->header_off + 4; } struct restart_needle_less_args { @@ -435,14 +433,14 @@ static int restart_needle_less(size_t idx, void *_args) int block_iter_next(struct block_iter *it, struct reftable_record *rec) { struct string_view in = { - .buf = (unsigned char *) it->block + it->next_off, - .len = it->block_len - it->next_off, + .buf = (unsigned char *) it->block->block_data.data + it->next_off, + .len = it->block->restart_off - it->next_off, }; struct string_view start = in; uint8_t extra = 0; int n = 0; - if (it->next_off >= it->block_len) + if (it->next_off >= it->block->restart_off) return 1; n = reftable_decode_key(&it->last_key, &extra, in); @@ -452,7 +450,7 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec) return REFTABLE_FORMAT_ERROR; string_view_consume(&in, n); - n = reftable_record_decode(rec, it->last_key, extra, in, it->hash_size, + n = reftable_record_decode(rec, it->last_key, extra, in, it->block->hash_size, &it->scratch); if (n < 0) return -1; @@ -467,8 +465,6 @@ void block_iter_reset(struct block_iter *it) reftable_buf_reset(&it->last_key); it->next_off = 0; it->block = NULL; - it->block_len = 0; - it->hash_size = 0; } void block_iter_close(struct block_iter *it) @@ -528,9 +524,7 @@ int block_iter_seek_key(struct block_iter *it, const struct reftable_block *bloc it->next_off = block_restart_offset(block, i - 1); else it->next_off = block->header_off + 4; - it->block = block->block_data.data; - it->block_len = block->restart_off; - it->hash_size = block->hash_size; + it->block = block; err = reftable_record_init(&rec, reftable_block_type(block)); if (err < 0) diff --git a/reftable/block.h b/reftable/block.h index 4f7f29028c4..268d5a1e005 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -67,9 +67,7 @@ void block_writer_release(struct block_writer *bw); struct block_iter { /* offset within the block of the next entry to read. */ uint32_t next_off; - const unsigned char *block; - size_t block_len; - uint32_t hash_size; + const struct reftable_block *block; /* key for last entry we read. */ struct reftable_buf last_key;
-- 2.49.0.604.gff1f9ca942.dirty