Re: [PATCH v2 12/16] refs/iterator: implement seeking for reftable iterators
- From
shejialuo <shejialuo@gmail.com>
- Date
- Feb 24, 2025, 14:00 UTC
- Message-ID
- <Z7x7fDiW0iN7qGIg@ArchLinux>
- In-Reply-To
- <20250219-pks-update-ref-optimization-v2-12-e696e7220b22@pks.im>
On Wed, Feb 19, 2025 at 02:23:39PM +0100, Patrick Steinhardt wrote:
Show 41 quoted lines
> Implement seeking of reftable iterators. As the low-level reftable
> iterators already support seeking this change is straight-forward. Two
> notes though:
>
> - We do not support seeking on reflog iterators.
>
> - We start to check whether `reftable_stack_init_ref_iterator()` is
> successful.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> refs/reftable-backend.c | 35 ++++++++++++++++++++++++++++++-----
> 1 file changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 06543f79c64..b0c09f34433 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -547,7 +547,7 @@ struct reftable_ref_iterator {
> struct reftable_ref_record ref;
> struct object_id oid;
>
> - const char *prefix;
> + char *prefix;
> size_t prefix_len;
> char **exclude_patterns;
> size_t exclude_patterns_index;
> @@ -718,6 +718,20 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
> return ITER_OK;
> }
>
> +static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator,
> + const char *prefix)
> +{
> + struct reftable_ref_iterator *iter =
> + (struct reftable_ref_iterator *)ref_iterator;
> +
> + free(iter->prefix);
> + iter->prefix = xstrdup_or_null(prefix);
> + iter->prefix_len = prefix ? strlen(prefix) : 0;
> + iter->err = reftable_iterator_seek_ref(&iter->iter, prefix);Should we rename this function `reftable_iterator_seek_ref` by the way? It is a little strange that we have two functions which are so similar:
1. reftable_ref_iterator_seek 2. reftable_iterator_seek_ref
However, don't worth a reroll.
Show 26 quoted lines
> +
> + return iter->err;
> +}
> +
> static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator,
> struct object_id *peeled)
> {
> @@ -744,10 +758,12 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator)
> free(iter->exclude_patterns[i]);
> free(iter->exclude_patterns);
> }
> + free(iter->prefix);
> }
>
> static struct ref_iterator_vtable reftable_ref_iterator_vtable = {
> .advance = reftable_ref_iterator_advance,
> + .seek = reftable_ref_iterator_seek,
> .peel = reftable_ref_iterator_peel,
> .release = reftable_ref_iterator_release,
> };
> @@ -806,8 +822,6 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
>
> iter = xcalloc(1, sizeof(*iter));
> base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable);
> - iter->prefix = prefix;
> - iter->prefix_len = prefix ? strlen(prefix) : 0;We don't assign `iter->prefix` and `iter->prefix_len` here. This is because we want to use the new defined function `reftable_ref_iterator_seek`. In the fist glance, I am worried that "iter->prefix" might not be `NULL`. However, because we use `xcalloc`, "iter->prefix" would be `NULL` by default.
Thanks, Jialuo