git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] ls-refs.c: traverse longest common ref prefix

From
Jacob Vosmaer <jacob@gitlab.com>
Date
Jan 20, 2021, 11:00 UTC
Message-ID
<CADMWQoO+163mJBXctb+GHFebGCbKQRPAtMJ676_GwfEt-BedAw@mail.gmail.com>
In-Reply-To
<fb8681d12864d724108c524834f9498d91e270e6.1611080326.git.me@ttaylorr.com>
On Tue, Jan 19, 2021 at 7:19 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 64 quoted lines
>
> ls-refs performs a single revision walk over the whole ref namespace,
> and sends ones that match with one of the given ref prefixes down to the
> user.
>
> This can be expensive if there are many refs overall, but the portion of
> them covered by the given prefixes is small by comparison.
>
> To attempt to reduce the difference between the number of refs
> traversed, and the number of refs sent, only traverse references which
> are in the longest common prefix of the given prefixes. This is very
> reminiscent of the approach taken in b31e2680c4 (ref-filter.c: find
> disjoint pattern prefixes, 2019-06-26) which does an analogous thing for
> multi-patterned 'git for-each-ref' invocations.
>
> The only difference here is that we are operating on ref prefixes, which
> do not necessarily point to a single reference. That is just fine, since
> all we care about is finding the longest common prefix among prefixes
> which can be thought of as refspecs for our purposes here.
>
> Similarly, for_each_fullref_in_prefixes may return more results than the
> caller asked for (since the longest common prefix might match something
> that a longer prefix in the same set wouldn't match) but
> ls-refs.c:send_ref() discards such results.
>
> The code introduced in b31e2680c4 is resilient to stop early (and
> return a shorter prefix) when it encounters a metacharacter (as
> mentioned in that patch, there is some opportunity to improve this, but
> nobody has done it).
>
> There are two remaining small items in this patch:
>
>   - If no prefixes were provided, then implicitly add the empty string
>     (which will match all references).
>
>   - Since we are manually munging the prefixes, make sure that we
>     initialize it ourselves (previously this wasn't necessary since the
>     first strvec_push would do so).
>
> Original-patch-by: Jacob Vosmaer <jacob@gitlab.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  ls-refs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/ls-refs.c b/ls-refs.c
> index a1e0b473e4..eaaa36d0df 100644
> --- a/ls-refs.c
> +++ b/ls-refs.c
> @@ -90,6 +90,7 @@ int ls_refs(struct repository *r, struct strvec *keys,
>         struct ls_refs_data data;
>
>         memset(&data, 0, sizeof(data));
> +       strvec_init(&data.prefixes);
>
>         git_config(ls_refs_config, NULL);
>
> @@ -109,7 +110,10 @@ int ls_refs(struct repository *r, struct strvec *keys,
>                 die(_("expected flush after ls-refs arguments"));
>
>         head_ref_namespaced(send_ref, &data);
> -       for_each_namespaced_ref(send_ref, &data);
> +       if (!data.prefixes.nr)
> +               strvec_push(&data.prefixes, "");

The old code, with for_each_namespaced_ref, would walk "${NAMESPACE}refs/". The new code would walk "${NAMESPACE}" because we're pushing "" onto data.prefixes. So if there is anything in the namespace that does not start with "refs/" it will get yielded.

Does that matter?
Show 7 quoted lines
> +       for_each_fullref_in_prefixes(get_git_namespace(), data.prefixes.v,
> +                                    send_ref, &data, 0);
>         packet_flush(1);
>         strvec_clear(&data.prefixes);
>         return 0;
> --
> 2.30.0.138.g6d7191ea01
Previous: Jeff KingNext: Taylor Blau
Message 7 of 27 in “Re: [PATCH 1/1] ls-refs.c: minimize number of refs visited”
  1. Jacob VosmaerJan 19, 2021
  2. 0/2 ls-refs: only traverse through longest common ref prefixTaylor Blau, Jan 19, 2021
  3. 2/2 ls-refs.c: traverse longest common ref prefixTaylor Blau, Jan 19, 2021
  4. Jeff KingJan 19, 2021
  5. Taylor BlauJan 19, 2021
  6. Jeff KingJan 20, 2021
  7. Jacob VosmaerJan 20, 2021
  8. 1/2 refs: expose 'for_each_fullref_in_prefixes'Taylor Blau, Jan 19, 2021
  9. 0/3 ls-refs: traverse prefixes of disjoint "ref-prefix" setsTaylor Blau, Jan 20, 2021
  10. 2/3 ls-refs.c: initialize 'prefixes' before using itTaylor Blau, Jan 20, 2021
  11. Jeff KingJan 20, 2021
  12. Taylor BlauJan 20, 2021
  13. Jacob VosmaerJan 20, 2021
  14. 3/3 ls-refs.c: traverse prefixes of disjoint "ref-prefix" setsTaylor Blau, Jan 20, 2021
  15. 1/3 refs: expose 'for_each_fullref_in_prefixes'Taylor Blau, Jan 20, 2021
  16. Jeff KingJan 20, 2021
  17. Taylor BlauJan 20, 2021
  18. Junio C HamanoJan 23, 2021
  19. Taylor BlauJan 25, 2021
  20. Junio C HamanoJan 23, 2021
  21. Taylor BlauJan 19, 2021
  22. Jeff KingJan 19, 2021
  23. Jeff KingJan 19, 2021
  24. Taylor BlauJan 19, 2021
  25. Jeff KingJan 19, 2021
  26. Jeff KingJan 19, 2021
  27. Taylor BlauJan 19, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.