Re: [PATCH] builtin/fetch: iterate symrefs instead of all when checking dangling refs

[Taylor Blau <me@ttaylorr.com>]

On Tue, Oct 15, 2024 at 03:27:58AM +0000, Liu Zhongbo via GitGitGadget wrote:
> From: Liu Zhongbo <liuzhongbo.6666@bytedance.com>
>
> refs_warn_dangling_symref() traverse all references to check if there are
> any dangling symbolic references. The complexity is
> O(number of deleted references * total number of references).
> It will take a lot of time if there are tens of thousands of branches in
> monorepo.
>
> So I first identified all the symbolic references, and then only traverse
> in these references. The complexity is
> O (number of deleted references * number of symbolic references).
>
> Due to the infrequent use of symbolic references, there will be significant
> performance improvements here. In my case, the prune_refs() time has been
> reduced from 20 seconds to 4 seconds.
>
> Signed-off-by: Liu Zhongbo <liuzhongbo.6666@bytedance.com>
> ---
>     builtin/fetch: iterate symrefs instead of all refs
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1812%2Flzb6666%2Fspeed_up_prune_refs-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1812/lzb6666/speed_up_prune_refs-v1
> Pull-Request: https://github.com/git/git/pull/1812
>
>  builtin/fetch.c |  7 +++++--
>  refs.c          | 35 ++++++++++++++++++++++++++---------
>  refs.h          |  4 +++-
>  3 files changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 80a64d0d269..ec4be60cfeb 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1412,15 +1412,18 @@ static int prune_refs(struct display_state *display_state,
>
>  	if (verbosity >= 0) {
>  		int summary_width = transport_summary_width(stale_refs);
> +	    struct string_list symrefs = STRING_LIST_INIT_NODUP;
> +	    refs_get_symrefs(get_main_ref_store(the_repository), &symrefs);

Without reading the contents of the patch below, there appear to be some
style issues here. It seems that the indentation here is incorrect, as
well as below throughout the patch.

Please make sure that the indentation is consistent with the rest of the
project's conventions which can be found in
Documentation/CodingGuidelines before re-submitting.

Thanks,
Taylor