From b9fd73a234db1a272f6cbfb528bae0ead9e07bde Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 31 Jul 2025 16:56:49 +0200 Subject: refs: pass refname when invoking reflog entry callback With `refs_for_each_reflog_ent()` callers can iterate through all the reflog entries for a given reference. The callback that is being invoked for each such entry does not receive the name of the reference that we are currently iterating through. This isn't really a limiting factor, as callers can simply pass the name via the callback data. But this layout sometimes does make for a bit of an awkward calling pattern. One example: when iterating through all reflogs, and for each reflog we iterate through all refnames, we have to do some extra book keeping to track which reference name we are currently yielding reflog entries for. Change the signature of the callback function so that the reference name of the reflog gets passed through to it. Adapt callers accordingly and start using the new parameter in trivial cases. The next commit will refactor the reference migration logic to make use of this parameter so that we can simplify its logic a bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'refs.h') diff --git a/refs.h b/refs.h index 99b58d0b73..0bf50ce25c 100644 --- a/refs.h +++ b/refs.h @@ -558,10 +558,13 @@ int refs_delete_reflog(struct ref_store *refs, const char *refname); * The cb_data is a caller-supplied pointer given to the iterator * functions. */ -typedef int each_reflog_ent_fn( - struct object_id *old_oid, struct object_id *new_oid, - const char *committer, timestamp_t timestamp, - int tz, const char *msg, void *cb_data); +typedef int each_reflog_ent_fn(const char *refname, + struct object_id *old_oid, + struct object_id *new_oid, + const char *committer, + timestamp_t timestamp, + int tz, const char *msg, + void *cb_data); /* Iterate over reflog entries in the log for `refname`. */ -- cgit v1.3 From 16c4fa26b99e6f6c24dc93575ffa884c13b1fe5f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 31 Jul 2025 16:56:54 +0200 Subject: builtin/remote: only iterate through refs that are to be renamed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When renaming a remote we also need to rename all references accordingly. But while we only need to rename references that are contained in the "refs/remotes/$OLDNAME/" namespace, we end up using `refs_for_each_rawref()` that iterates through _all_ references. We know to exit early in the callback in case we see an irrelevant reference, but ultimately this is still a waste of compute as we knowingly iterate through references that we won't ever care about. Improve this by using `refs_for_each_rawref_in()`, which knows to only iterate through (potentially broken) references in a given prefix. The following benchmark renames a remote with a single reference in a repository that has 100k unrelated references. This shows a sizeable improvement with the "files" backend: Benchmark 1: rename remote (refformat = files, revision = HEAD~) Time (mean ± σ): 42.6 ms ± 0.9 ms [User: 29.1 ms, System: 8.4 ms] Range (min … max): 40.1 ms … 43.3 ms 10 runs Benchmark 2: rename remote (refformat = files, revision = HEAD) Time (mean ± σ): 31.7 ms ± 4.0 ms [User: 19.6 ms, System: 6.9 ms] Range (min … max): 27.1 ms … 36.0 ms 10 runs Summary rename remote (refformat = files, revision = HEAD) ran 1.35 ± 0.17 times faster than rename remote (refformat = files, revision = HEAD~) The "reftable" backend shows roughly the same absolute improvement, but given that it's already significantly faster than the "files" backend this translates to a much larger relative improvement: Benchmark 1: rename remote (refformat = reftable, revision = HEAD~) Time (mean ± σ): 18.2 ms ± 0.5 ms [User: 12.7 ms, System: 3.0 ms] Range (min … max): 17.3 ms … 21.4 ms 110 runs Benchmark 2: rename remote (refformat = reftable, revision = HEAD) Time (mean ± σ): 8.8 ms ± 0.5 ms [User: 3.8 ms, System: 2.9 ms] Range (min … max): 7.5 ms … 9.9 ms 167 runs Summary rename remote (refformat = reftable, revision = HEAD) ran 2.07 ± 0.12 times faster than rename remote (refformat = reftable, revision = HEAD~) Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/remote.c | 13 ++++--------- refs.c | 8 +++++++- refs.h | 2 ++ 3 files changed, 13 insertions(+), 10 deletions(-) (limited to 'refs.h') diff --git a/builtin/remote.c b/builtin/remote.c index db481f39bc..60e67f1b74 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -720,16 +720,8 @@ static int rename_one_ref(const char *old_refname, const char *referent, struct strbuf new_referent = STRBUF_INIT; struct strbuf new_refname = STRBUF_INIT; struct rename_info *rename = cb_data; - const char *ptr = old_refname; int error; - if (!skip_prefix(ptr, "refs/remotes/", &ptr) || - !skip_prefix(ptr, rename->old_name, &ptr) || - !skip_prefix(ptr, "/", &ptr)) { - error = 0; - goto out; - } - compute_renamed_ref(rename, old_refname, &new_refname); if (flags & REF_ISSYMREF) { @@ -932,7 +924,10 @@ static int mv(int argc, const char **argv, const char *prefix, rename.progress = start_delayed_progress(the_repository, _("Renaming remote references"), 0); - result = refs_for_each_rawref(get_main_ref_store(the_repository), + strbuf_reset(&buf); + strbuf_addf(&buf, "refs/remotes/%s/", rename.old_name); + + result = refs_for_each_rawref_in(get_main_ref_store(the_repository), buf.buf, rename_one_ref, &rename); if (result < 0) die(_("queueing remote ref renames failed: %s"), rename.err->buf); diff --git a/refs.c b/refs.c index 04c9ace793..7e2f02dddf 100644 --- a/refs.c +++ b/refs.c @@ -1839,7 +1839,13 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data) { - return do_for_each_ref(refs, "", NULL, fn, 0, + return refs_for_each_rawref_in(refs, "", fn, cb_data); +} + +int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, + each_ref_fn fn, void *cb_data) +{ + return do_for_each_ref(refs, prefix, NULL, fn, 0, DO_FOR_EACH_INCLUDE_BROKEN, cb_data); } diff --git a/refs.h b/refs.h index 0bf50ce25c..19fb1d924a 100644 --- a/refs.h +++ b/refs.h @@ -428,6 +428,8 @@ int refs_for_each_namespaced_ref(struct ref_store *refs, /* can be used to learn about broken ref and symref */ int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data); +int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix, + each_ref_fn fn, void *cb_data); /* * Iterates over all refs including root refs, i.e. pseudorefs and HEAD. -- cgit v1.3