From 89baa52da612dde6da031acfa2cb957d4297d544 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:11 +0200 Subject: refs: introduce `.ref` field for the base iterator The base iterator has a couple of fields that tracks the name, target, object ID and flags for the current reference. Due to this design we have to create a new `struct reference` whenever we want to hand over that reference to the callback function, which is tedious and not very efficient. Convert the structure to instead contain a `struct reference` as member. This member is expected to be populated by the implementations of the iterator and is handed over to the callback directly. While at it, simplify `should_pack_ref()` to take a `struct reference` directly instead of passing its respective fields. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/refs-internal.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'refs/refs-internal.h') diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 4ef3bd75c6..ed749d1657 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -249,10 +249,7 @@ const char *find_descendant_ref(const char *dirname, */ struct ref_iterator { struct ref_iterator_vtable *vtable; - const char *refname; - const char *referent; - const struct object_id *oid; - unsigned int flags; + struct reference ref; }; /* -- cgit v1.3 From 5a5c7359f77ecd1bc4b0e172563161d602f131d3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:18 +0200 Subject: refs: drop `current_ref_iter` hack In preceding commits we have refactored all callers of `peel_iterated_oid()` to instead use `reference_get_peeled_oid()`. This allows us to thus get rid of the former function. Getting rid of that function is nice, but even nicer is that this also allows us to get rid of the `current_ref_iter` hack. This global variable tracked the currently-active ref iterator so that we can use it to peel an object ID. Now that the peeled object ID is propagated via `struct reference` though we don't have to depend on this hack anymore, which makes for a more robust and easier-to-understand infrastructure. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 10 ---------- refs/iterator.c | 5 ----- refs/refs-internal.h | 13 ------------- 3 files changed, 28 deletions(-) (limited to 'refs/refs-internal.h') diff --git a/refs.c b/refs.c index 1b1551f981..9d8f0a9ca4 100644 --- a/refs.c +++ b/refs.c @@ -2324,16 +2324,6 @@ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) return refs->be->optimize(refs, opts); } -int peel_iterated_oid(struct repository *r, const struct object_id *base, struct object_id *peeled) -{ - if (current_ref_iter && - (current_ref_iter->ref.oid == base || - oideq(current_ref_iter->ref.oid, base))) - return ref_iterator_peel(current_ref_iter, peeled); - - return peel_object(r, base, peeled) ? -1 : 0; -} - int reference_get_peeled_oid(struct repository *repo, const struct reference *ref, struct object_id *peeled_oid) diff --git a/refs/iterator.c b/refs/iterator.c index fe5980e1b6..072c6aacdb 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -458,15 +458,11 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, return ref_iterator; } -struct ref_iterator *current_ref_iter = NULL; - int do_for_each_ref_iterator(struct ref_iterator *iter, each_ref_fn fn, void *cb_data) { int retval = 0, ok; - struct ref_iterator *old_ref_iter = current_ref_iter; - current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { retval = fn(&iter->ref, cb_data); if (retval) @@ -474,7 +470,6 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, } out: - current_ref_iter = old_ref_iter; if (ok == ITER_ERROR) retval = -1; ref_iterator_free(iter); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index ed749d1657..f4f845bbea 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -376,19 +376,6 @@ struct ref_iterator_vtable { ref_iterator_release_fn *release; }; -/* - * current_ref_iter is a performance hack: when iterating over - * references using the for_each_ref*() functions, current_ref_iter is - * set to the reference iterator before calling the callback function. - * If the callback function calls peel_ref(), then peel_ref() first - * checks whether the reference to be peeled is the one referred to by - * the iterator (it usually is) and if so, asks the iterator for the - * peeled version of the reference if it is available. This avoids a - * refname lookup in a common case. current_ref_iter is set to NULL - * when the iteration is over. - */ -extern struct ref_iterator *current_ref_iter; - struct ref_store; /* refs backends */ -- cgit v1.3 From 705114772e0a0741c3288329bd9ac4e11e38db9a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:19 +0200 Subject: refs: drop infrastructure to peel via iterators Now that the peeled object ID gets propagated via the `struct reference` there is no need anymore to call into the reference iterator itself to dereference an object. Remove this infrastructure. Most of the changes are straight-forward deletions of code. There is one exception though in `refs/packed-backend.c::write_with_updates()`. Here we stop peeling the iterator and instead just pass the peeled object ID of that iterator directly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 14 -------------- refs/debug.c | 11 ----------- refs/files-backend.c | 17 ----------------- refs/iterator.c | 36 ------------------------------------ refs/packed-backend.c | 24 +----------------------- refs/ref-cache.c | 9 --------- refs/refs-internal.h | 7 ------- refs/reftable-backend.c | 24 ------------------------ 8 files changed, 1 insertion(+), 141 deletions(-) (limited to 'refs/refs-internal.h') diff --git a/refs.h b/refs.h index 886ed2c0f4..2dd7ac1a16 100644 --- a/refs.h +++ b/refs.h @@ -1289,10 +1289,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * to the next entry, ref_iterator_advance() aborts the iteration, * frees the ref_iterator, and returns ITER_ERROR. * - * The reference currently being looked at can be peeled by calling - * ref_iterator_peel(). This function is often faster than peel_ref(), - * so it should be preferred when iterating over references. - * * Putting it all together, a typical iteration looks like this: * * int ok; @@ -1307,9 +1303,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * // Access information about the current reference: * if (!(iter->flags & REF_ISSYMREF)) * printf("%s is %s\n", iter->refname, oid_to_hex(iter->oid)); - * - * // If you need to peel the reference: - * ref_iterator_peel(iter, &oid); * } * * if (ok != ITER_DONE) @@ -1400,13 +1393,6 @@ enum ref_iterator_seek_flag { int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * If possible, peel the reference currently being viewed by the - * iterator. Return 0 on success. - */ -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* Free the reference iterator and any associated resources. */ void ref_iterator_free(struct ref_iterator *ref_iterator); diff --git a/refs/debug.c b/refs/debug.c index 67718bd1f4..01499b9033 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -177,16 +177,6 @@ static int debug_ref_iterator_seek(struct ref_iterator *ref_iterator, return res; } -static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct debug_ref_iterator *diter = - (struct debug_ref_iterator *)ref_iterator; - int res = diter->iter->vtable->peel(diter->iter, peeled); - trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->ref.name, res); - return res; -} - static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) { struct debug_ref_iterator *diter = @@ -198,7 +188,6 @@ static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable debug_ref_iterator_vtable = { .advance = debug_ref_iterator_advance, .seek = debug_ref_iterator_seek, - .peel = debug_ref_iterator_peel, .release = debug_ref_iterator_release, }; diff --git a/refs/files-backend.c b/refs/files-backend.c index fac53fa052..5aeb454fb4 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -993,15 +993,6 @@ static int files_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int files_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct files_ref_iterator *iter = - (struct files_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void files_ref_iterator_release(struct ref_iterator *ref_iterator) { struct files_ref_iterator *iter = @@ -1012,7 +1003,6 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_ref_iterator_vtable = { .advance = files_ref_iterator_advance, .seek = files_ref_iterator_seek, - .peel = files_ref_iterator_peel, .release = files_ref_iterator_release, }; @@ -2388,12 +2378,6 @@ static int files_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSED, BUG("ref_iterator_seek() called for reflog_iterator"); } -static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("ref_iterator_peel() called for reflog_iterator"); -} - static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct files_reflog_iterator *iter = @@ -2404,7 +2388,6 @@ static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_reflog_iterator_vtable = { .advance = files_reflog_iterator_advance, .seek = files_reflog_iterator_seek, - .peel = files_reflog_iterator_peel, .release = files_reflog_iterator_release, }; diff --git a/refs/iterator.c b/refs/iterator.c index 072c6aacdb..d79aa5ec82 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -21,12 +21,6 @@ int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, return ref_iterator->vtable->seek(ref_iterator, refname, flags); } -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - return ref_iterator->vtable->peel(ref_iterator, peeled); -} - void ref_iterator_free(struct ref_iterator *ref_iterator) { if (ref_iterator) { @@ -60,12 +54,6 @@ static int empty_ref_iterator_seek(struct ref_iterator *ref_iterator UNUSED, return 0; } -static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("peel called for empty iterator"); -} - static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) { } @@ -73,7 +61,6 @@ static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) static struct ref_iterator_vtable empty_ref_iterator_vtable = { .advance = empty_ref_iterator_advance, .seek = empty_ref_iterator_seek, - .peel = empty_ref_iterator_peel, .release = empty_ref_iterator_release, }; @@ -240,18 +227,6 @@ static int merge_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct merge_ref_iterator *iter = - (struct merge_ref_iterator *)ref_iterator; - - if (!iter->current) { - BUG("peel called before advance for merge iterator"); - } - return ref_iterator_peel(*iter->current, peeled); -} - static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) { struct merge_ref_iterator *iter = @@ -263,7 +238,6 @@ static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable merge_ref_iterator_vtable = { .advance = merge_ref_iterator_advance, .seek = merge_ref_iterator_seek, - .peel = merge_ref_iterator_peel, .release = merge_ref_iterator_release, }; @@ -412,15 +386,6 @@ static int prefix_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct prefix_ref_iterator *iter = - (struct prefix_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) { struct prefix_ref_iterator *iter = @@ -432,7 +397,6 @@ static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable prefix_ref_iterator_vtable = { .advance = prefix_ref_iterator_advance, .seek = prefix_ref_iterator_seek, - .peel = prefix_ref_iterator_peel, .release = prefix_ref_iterator_release, }; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1fefefd54e..6fa229edd0 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1030,22 +1030,6 @@ static int packed_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct packed_ref_iterator *iter = - (struct packed_ref_iterator *)ref_iterator; - - if ((iter->base.ref.flags & REF_KNOWS_PEELED)) { - oidcpy(peeled, &iter->peeled); - return is_null_oid(&iter->peeled) ? -1 : 0; - } else if ((iter->base.ref.flags & (REF_ISBROKEN | REF_ISSYMREF))) { - return -1; - } else { - return peel_object(iter->repo, &iter->oid, peeled) ? -1 : 0; - } -} - static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) { struct packed_ref_iterator *iter = @@ -1059,7 +1043,6 @@ static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable packed_ref_iterator_vtable = { .advance = packed_ref_iterator_advance, .seek = packed_ref_iterator_seek, - .peel = packed_ref_iterator_peel, .release = packed_ref_iterator_release, }; @@ -1525,13 +1508,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re if (cmp < 0) { /* Pass the old reference through. */ - - struct object_id peeled; - int peel_error = ref_iterator_peel(iter, &peeled); - if (write_packed_entry(out, iter->ref.name, - iter->ref.oid, - peel_error ? NULL : &peeled)) + iter->ref.oid, iter->ref.peeled_oid)) goto write_error; if ((ok = ref_iterator_advance(iter)) != ITER_OK) { diff --git a/refs/ref-cache.c b/refs/ref-cache.c index e427848879..ffef01a597 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -546,14 +546,6 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct cache_ref_iterator *iter = - (struct cache_ref_iterator *)ref_iterator; - return peel_object(iter->repo, ref_iterator->ref.oid, peeled) ? -1 : 0; -} - static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) { struct cache_ref_iterator *iter = @@ -565,7 +557,6 @@ static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable cache_ref_iterator_vtable = { .advance = cache_ref_iterator_advance, .seek = cache_ref_iterator_seek, - .peel = cache_ref_iterator_peel, .release = cache_ref_iterator_release, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f4f845bbea..4671517dad 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -357,12 +357,6 @@ typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator); typedef int ref_iterator_seek_fn(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * Peels the current ref, returning 0 for success or -1 for failure. - */ -typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* * Implementations of this function should free any resources specific * to the derived class. @@ -372,7 +366,6 @@ typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator); struct ref_iterator_vtable { ref_iterator_advance_fn *advance; ref_iterator_seek_fn *seek; - ref_iterator_peel_fn *peel; ref_iterator_release_fn *release; }; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e214e120d7..e329d4a423 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -744,21 +744,6 @@ static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator, return iter->err; } -static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct reftable_ref_iterator *iter = - (struct reftable_ref_iterator *)ref_iterator; - - if (iter->ref.value_type == REFTABLE_REF_VAL2) { - oidread(peeled, iter->ref.value.val2.target_value, - iter->refs->base.repo->hash_algo); - return 0; - } - - return -1; -} - static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_ref_iterator *iter = @@ -776,7 +761,6 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) 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, }; @@ -2098,13 +2082,6 @@ static int reftable_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSE return -1; } -static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("reftable reflog iterator cannot be peeled"); - return -1; -} - static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_reflog_iterator *iter = @@ -2117,7 +2094,6 @@ static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_reflog_iterator_vtable = { .advance = reftable_reflog_iterator_advance, .seek = reftable_reflog_iterator_seek, - .peel = reftable_reflog_iterator_peel, .release = reftable_reflog_iterator_release, }; -- cgit v1.3