Re: [PATCH 10/16] reftable/block: store block pointer in the block iterator
On 25/03/31 10:41AM, Patrick Steinhardt wrote:
Show 14 quoted lines
> 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 into the block.
Show 13 quoted lines
> 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(-)
>
Show 12 quoted lines
> 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;This is much simpler. Nice!
Show 7 quoted lines
> /* key for last entry we read. */
> struct reftable_buf last_key;
>
> --
> 2.49.0.604.gff1f9ca942.dirty
>
>
Justin Tobler <jltobler@gmail.com>