From cc7dc407fe4c153195e5ce38d0109f3c2c35ceaf Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:13 -0700 Subject: [PATCH 1/3] fetch-prune: optimize dangling-ref reporting When pruning during `git fetch` we check each pruned ref against the ref_store one at a time to decide whether to report it as dangling. This causes every local ref to be scanned for each ref being pruned. If there are N refs in the repo and M refs being pruned, this code is O(M*N). However, `git remote prune` uses a very similar function that is only O(N*log(M)). Remove the wasteful ref scanning for each pruned ref and use the faster version already available in refs_warn_dangling_symrefs. Change the message to include the original refname since the message is no longer printed immediately after the line that did just print the refname. In a repo with 126,000 refs, where I was pruning 28,000 refs, this code made about 3.6 billion calls to strcmp and consumed 410 seconds of CPU. (Invariably in that time, my remote would timeout and the fetch would fail anyway.) After this change, the same operation completes in under a second. Signed-off-by: Phil Hord Reviewed-by: Jacob Keller Signed-off-by: Junio C Hamano --- builtin/fetch.c | 20 ++++++++++---------- builtin/remote.c | 4 ++-- refs.c | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index fe2b26c74a..f05530b62e 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1384,9 +1384,13 @@ static int prune_refs(struct display_state *display_state, int result = 0; struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; + struct string_list refnames = STRING_LIST_INIT_NODUP; const char *dangling_msg = dry_run - ? _(" (%s will become dangling)") - : _(" (%s has become dangling)"); + ? _(" %s will become dangling after %s is deleted") + : _(" %s has become dangling after %s was deleted"); + + for (ref = stale_refs; ref; ref = ref->next) + string_list_append(&refnames, ref->name); if (!dry_run) { if (transaction) { @@ -1397,15 +1401,9 @@ static int prune_refs(struct display_state *display_state, goto cleanup; } } else { - struct string_list refnames = STRING_LIST_INIT_NODUP; - - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); - result = refs_delete_refs(get_main_ref_store(the_repository), "fetch: prune", &refnames, 0); - string_list_clear(&refnames, 0); } } @@ -1417,12 +1415,14 @@ static int prune_refs(struct display_state *display_state, _("(none)"), ref->name, &ref->new_oid, &ref->old_oid, summary_width); - refs_warn_dangling_symref(get_main_ref_store(the_repository), - stderr, dangling_msg, ref->name); } + string_list_sort(&refnames); + refs_warn_dangling_symrefs(get_main_ref_store(the_repository), + stderr, dangling_msg, &refnames); } cleanup: + string_list_clear(&refnames, 0); strbuf_release(&err); free_refs(stale_refs); return result; diff --git a/builtin/remote.c b/builtin/remote.c index 0435963286..30cf4100d4 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1516,8 +1516,8 @@ static int prune_remote(const char *remote, int dry_run) struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; struct string_list_item *item; const char *dangling_msg = dry_run - ? _(" %s will become dangling!") - : _(" %s has become dangling!"); + ? _(" %s will become dangling after %s is deleted!") + : _(" %s has become dangling after %s was deleted!"); get_remote_ref_states(remote, &states, GET_REF_STATES); diff --git a/refs.c b/refs.c index 0f41b2fd4a..def147b685 100644 --- a/refs.c +++ b/refs.c @@ -461,7 +461,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } - fprintf(d->fp, d->msg_fmt, refname); + fprintf(d->fp, d->msg_fmt, refname, resolves_to); fputc('\n', d->fp); return 0; } From 0f846954999a332735c52dc26451be674ea617ba Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:14 -0700 Subject: [PATCH 2/3] refs: remove old refs_warn_dangling_symref The dangling warning function that takes a single ref to search for is no longer used. Remove it. Signed-off-by: Phil Hord Signed-off-by: Junio C Hamano --- refs.c | 17 +---------------- refs.h | 2 -- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/refs.c b/refs.c index def147b685..b0e67077a6 100644 --- a/refs.c +++ b/refs.c @@ -438,7 +438,6 @@ static int for_each_filter_refs(const char *refname, const char *referent, struct warn_if_dangling_data { struct ref_store *refs; FILE *fp; - const char *refname; const struct string_list *refnames; const char *msg_fmt; }; @@ -455,9 +454,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU resolves_to = refs_resolve_ref_unsafe(d->refs, refname, 0, NULL, NULL); if (!resolves_to - || (d->refname - ? strcmp(resolves_to, d->refname) - : !string_list_has_string(d->refnames, resolves_to))) { + || !string_list_has_string(d->refnames, resolves_to)) { return 0; } @@ -466,18 +463,6 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } -void refs_warn_dangling_symref(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const char *refname) -{ - struct warn_if_dangling_data data = { - .refs = refs, - .fp = fp, - .refname = refname, - .msg_fmt = msg_fmt, - }; - refs_for_each_rawref(refs, warn_if_dangling_symref, &data); -} - void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const char *msg_fmt, const struct string_list *refnames) { diff --git a/refs.h b/refs.h index a0cdd99250..60642bc24a 100644 --- a/refs.h +++ b/refs.h @@ -435,8 +435,6 @@ static inline const char *has_glob_specials(const char *pattern) return strpbrk(pattern, "?*["); } -void refs_warn_dangling_symref(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const char *refname); void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const char *msg_fmt, const struct string_list *refnames); From 87d8d8c5d09b1ee52cdf472b53b370020a7cb41c Mon Sep 17 00:00:00 2001 From: Phil Hord Date: Tue, 1 Jul 2025 18:12:15 -0700 Subject: [PATCH 3/3] clean up interface for refs_warn_dangling_symrefs The refs_warn_dangling_symrefs interface is a bit fragile as it passes in printf-formatting strings with expectations about the number of arguments. This patch series made it worse by adding a 2nd positional argument. But there are only two call sites, and they both use almost identical display options. Make this safer by moving the format strings into the function that uses them to make it easier to see when the arguments don't match. Pass a prefix string and a dry_run flag so the decision logic can be handled where needed. Signed-off-by: Phil Hord Signed-off-by: Junio C Hamano --- builtin/fetch.c | 5 +---- builtin/remote.c | 5 +---- refs.c | 17 +++++++++++------ refs.h | 3 ++- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index f05530b62e..98d89d6f65 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1385,9 +1385,6 @@ static int prune_refs(struct display_state *display_state, struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); struct strbuf err = STRBUF_INIT; struct string_list refnames = STRING_LIST_INIT_NODUP; - const char *dangling_msg = dry_run - ? _(" %s will become dangling after %s is deleted") - : _(" %s has become dangling after %s was deleted"); for (ref = stale_refs; ref; ref = ref->next) string_list_append(&refnames, ref->name); @@ -1418,7 +1415,7 @@ static int prune_refs(struct display_state *display_state, } string_list_sort(&refnames); refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stderr, dangling_msg, &refnames); + stderr, " ", dry_run, &refnames); } cleanup: diff --git a/builtin/remote.c b/builtin/remote.c index 30cf4100d4..81912fed55 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1515,9 +1515,6 @@ static int prune_remote(const char *remote, int dry_run) struct ref_states states = REF_STATES_INIT; struct string_list refs_to_prune = STRING_LIST_INIT_NODUP; struct string_list_item *item; - const char *dangling_msg = dry_run - ? _(" %s will become dangling after %s is deleted!") - : _(" %s has become dangling after %s was deleted!"); get_remote_ref_states(remote, &states, GET_REF_STATES); @@ -1549,7 +1546,7 @@ static int prune_remote(const char *remote, int dry_run) } refs_warn_dangling_symrefs(get_main_ref_store(the_repository), - stdout, dangling_msg, &refs_to_prune); + stdout, " ", dry_run, &refs_to_prune); string_list_clear(&refs_to_prune, 0); free_remote_ref_states(&states); diff --git a/refs.c b/refs.c index b0e67077a6..8573310a06 100644 --- a/refs.c +++ b/refs.c @@ -439,7 +439,8 @@ struct warn_if_dangling_data { struct ref_store *refs; FILE *fp; const struct string_list *refnames; - const char *msg_fmt; + const char *indent; + int dry_run; }; static int warn_if_dangling_symref(const char *refname, const char *referent UNUSED, @@ -447,7 +448,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU int flags, void *cb_data) { struct warn_if_dangling_data *d = cb_data; - const char *resolves_to; + const char *resolves_to, *msg; if (!(flags & REF_ISSYMREF)) return 0; @@ -458,19 +459,23 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU return 0; } - fprintf(d->fp, d->msg_fmt, refname, resolves_to); - fputc('\n', d->fp); + msg = d->dry_run + ? _("%s%s will become dangling after %s is deleted\n") + : _("%s%s has become dangling after %s was deleted\n"); + fprintf(d->fp, msg, d->indent, refname, resolves_to); return 0; } void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const struct string_list *refnames) + const char *indent, int dry_run, + const struct string_list *refnames) { struct warn_if_dangling_data data = { .refs = refs, .fp = fp, .refnames = refnames, - .msg_fmt = msg_fmt, + .indent = indent, + .dry_run = dry_run, }; refs_for_each_rawref(refs, warn_if_dangling_symref, &data); } diff --git a/refs.h b/refs.h index 60642bc24a..6b9c6d5764 100644 --- a/refs.h +++ b/refs.h @@ -436,7 +436,8 @@ static inline const char *has_glob_specials(const char *pattern) } void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, - const char *msg_fmt, const struct string_list *refnames); + const char *indent, int dry_run, + const struct string_list *refnames); /* * Flags for controlling behaviour of pack_refs()