Re: [PATCH 09/13] reftable/generic: move seeking of records into the iterator

> Reftable iterators are created by seeking on the parent structure of a
> corresponding record. For example, to create an iterator for the merged
> table you would call `reftable_merged_table_seek_ref()`. Most notably,
> it is not posible to create an iterator and then seek it afterwards.
> 
> While this may be a bit easier to reason about, it comes with two
> significant downsides. The first downside is that the logic to find
> records is split up between the parent data structure and the iterator
> itself. Conceptually, it is more straight forward if all that logic was
> contained in a single place, which should be the iterator.
> 
> The second and more significant downside is that it is impossible to
> reuse iterators for multiple seeks. Whenever you want to look up a
> record, you need to re-create the whole infrastructure again, which is
> quite a waste of time. Furthermore, it is impossible to for example
> optimize seeks, for example when seeking the same record multiple times.

> 
> To address this, we essentially split up the concerns properly such that
> the parent data structure is responsible for setting up the iterator via
> a new `init_iter()` callback, whereas the iterator handles seeks via a
> new `seek()` callback. This will eventually allow us to call `seek()` on
> the iterator multiple times, where every iterator can potentially
> optimize for certain cases.
> 
> Note that at this point in time we are not yet ready to reuse the
> iterators. This will be left for a future patch series.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  reftable/generic.c |  47 +++++++++++++----
>  reftable/generic.h |   9 +++-
>  reftable/iter.c    |  15 ++++++
>  reftable/merged.c  |  97 +++++++++++++++++-----------------
>  reftable/reader.c  | 126 +++++++++++++++++++++++++--------------------
>  5 files changed, 177 insertions(+), 117 deletions(-)
> 
> diff --git a/reftable/generic.c b/reftable/generic.c
> index b9f1c7c18a..1cf68fe124 100644
> --- a/reftable/generic.c
> +++ b/reftable/generic.c
> @@ -12,25 +12,39 @@ license that can be found in the LICENSE file or at
>  #include "reftable-iterator.h"
>  #include "reftable-generic.h"
>  
> +void table_init_iter(struct reftable_table *tab,

> +		     struct reftable_iterator *it,
> +		     uint8_t typ)
> +{
> +
> +	tab->ops->init_iter(tab->table_arg, it, typ);
> +}
> +
>  int reftable_table_seek_ref(struct reftable_table *tab,
>  			    struct reftable_iterator *it, const char *name)
>  {
> -	struct reftable_record rec = { .type = BLOCK_TYPE_REF,
> -				       .u.ref = {
> -					       .refname = (char *)name,
> -				       } };
> -	return tab->ops->seek_record(tab->table_arg, it, &rec);
> +	struct reftable_record want = {
> +		.type = BLOCK_TYPE_REF,
> +		.u.ref = {
> +			.refname = (char *)name,
> +		},
> +	};
> +	table_init_iter(tab, it, BLOCK_TYPE_REF);
> +	return it->ops->seek(it->iter_arg, &want);
>  }
>  
>  int reftable_table_seek_log(struct reftable_table *tab,
>  			    struct reftable_iterator *it, const char *name)
>  {
> -	struct reftable_record rec = { .type = BLOCK_TYPE_LOG,
> -				       .u.log = {
> -					       .refname = (char *)name,
> -					       .update_index = ~((uint64_t)0),
> -				       } };
> -	return tab->ops->seek_record(tab->table_arg, it, &rec);
> +	struct reftable_record want = {
> +		.type = BLOCK_TYPE_LOG,
> +		.u.log = {
> +			.refname = (char *)name,
> +			.update_index = ~((uint64_t)0),
> +		},
> +	};
> +	table_init_iter(tab, it, BLOCK_TYPE_LOG);
> +	return it->ops->seek(it->iter_arg, &want);
>  }
>  
>  int reftable_table_read_ref(struct reftable_table *tab, const char *name,

> @@ -23,6 +23,13 @@ static void filtering_ref_iterator_close(void *iter_arg)
>  	reftable_iterator_destroy(&fri->it);
>  }
>  
> +static int filtering_ref_iterator_seek(void *iter_arg,
> +				       struct reftable_record *want)
> +{
> +	struct filtering_ref_iterator *fri = iter_arg;
> +	return iterator_seek(&fri->it, want);
> +}