Skip to content

Commit ee60e6b

Browse files
pks-tgitster
authored andcommitted
builtin/remote: only iterate through refs that are to be renamed
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 introducing `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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d6ce7c2 commit ee60e6b

File tree

3 files changed

+14
-11
lines changed

3 files changed

+14
-11
lines changed

builtin/remote.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -710,16 +710,8 @@ static int rename_one_ref(const char *old_refname, const char *referent,
710710
{
711711
struct rename_info *rename = cb_data;
712712
struct strbuf *new_referent = rename->buf1;
713-
const char *ptr = old_refname;
714713
int error;
715714

716-
if (!skip_prefix(ptr, "refs/remotes/", &ptr) ||
717-
!skip_prefix(ptr, rename->old_name, &ptr) ||
718-
!skip_prefix(ptr, "/", &ptr)) {
719-
error = 0;
720-
goto out;
721-
}
722-
723715
renamed_refname(rename, old_refname, rename->new_refname);
724716

725717
if (flags & REF_ISSYMREF) {
@@ -976,8 +968,11 @@ static int mv(int argc, const char **argv, const char *prefix,
976968
rename.progress = start_delayed_progress(the_repository,
977969
_("Renaming remote references"), 0);
978970

979-
result = refs_for_each_rawref(get_main_ref_store(the_repository),
980-
rename_one_ref, &rename);
971+
strbuf_reset(&buf);
972+
strbuf_addf(&buf, "refs/remotes/%s/", rename.old_name);
973+
974+
result = refs_for_each_rawref_in(get_main_ref_store(the_repository), buf.buf,
975+
rename_one_ref, &rename);
981976
if (result < 0)
982977
die(_("renaming references failed: %s"), rename.err->buf);
983978

refs.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1840,7 +1840,13 @@ int refs_for_each_namespaced_ref(struct ref_store *refs,
18401840

18411841
int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
18421842
{
1843-
return do_for_each_ref(refs, "", NULL, fn, 0,
1843+
return refs_for_each_rawref_in(refs, "", fn, cb_data);
1844+
}
1845+
1846+
int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix,
1847+
each_ref_fn fn, void *cb_data)
1848+
{
1849+
return do_for_each_ref(refs, prefix, NULL, fn, 0,
18441850
DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
18451851
}
18461852

refs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,8 @@ int refs_for_each_namespaced_ref(struct ref_store *refs,
428428

429429
/* can be used to learn about broken ref and symref */
430430
int refs_for_each_rawref(struct ref_store *refs, each_ref_fn fn, void *cb_data);
431+
int refs_for_each_rawref_in(struct ref_store *refs, const char *prefix,
432+
each_ref_fn fn, void *cb_data);
431433

432434
/*
433435
* Iterates over all refs including root refs, i.e. pseudorefs and HEAD.

0 commit comments

Comments
 (0)