{"thread":{"id":"65767","subject":"[PATCH] describe: limit default ref iteration to tags","startedAt":"2026-06-07T20:56:58Z","lastAt":"2026-06-08T15:54:32Z","messageCount":5,"participants":["Tamir Duberstein","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"544857","messageId":"20260607-describe-tag-ref-scope-v1-1-653d232b86b5@gmail.com","threadId":"65767","inReplyTo":null,"subject":"[PATCH] describe: limit default ref iteration to tags","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-07T20:51:53Z","receivedAt":"2026-06-07T20:56:58Z","isPatch":true,"body":"Unless --all is given, get_name() rejects every ref outside refs/tags/.\nThe rejection happens only after the ref backend has enumerated the ref,\nso repositories with many other refs spend most of a simple describe\ninvocation visiting refs which cannot affect its result.\n\nCommit 8a5a1884e9 (Avoid accessing non-tag refs in git-describe unless\n--all is requested, 2008-02-24) moved this rejection before object\nlookup, but left iteration unscoped. Pass the existing refs/tags/\nrestriction to the iterator unless --all is given so the backend can\navoid unrelated refs.\n\nOn a checkout with 124,357 refs, of which 330 were tags, I ran the\nfollowing command with the parent and patched binaries:\n\n hyperfine --warmup 3 --runs 15 \\\n 'git describe --always --long --abbrev=40 HEAD'\n\nThe results were:\n\n parent this commit\n elapsed 196.2 ms 63.3 ms\n user 69.5 ms 48.0 ms\n system 123.0 ms 12.0 ms\n\nThe wall-time standard deviations were 13.2 ms and 2.6 ms, respectively,\nfor a 3.10x speedup.\n\nBoth revisions were built with -O3, -mcpu=native, and ThinLTO using\nApple clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro\n(Mac16,6) with a 16-core Apple M4 Max (12 performance and four\nefficiency cores) and 128 GB RAM.\n\nSigned-off-by: Tamir Duberstein \n---\n builtin/describe.c | 3 +++\n t/perf/p6100-describe.sh | 20 ++++++++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 1c47d7c0b7..3532c8ff22 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -740,6 +740,9 @@ int cmd_describe(int argc,\n \t\treturn ret;\n \t}\n \n+\tif (!all)\n+\t\tfor_each_ref_opts.prefix = \"refs/tags/\";\n+\n \thashmap_init(&names, commit_name_neq, NULL, 0);\n \trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n \t\t\t get_name, NULL, &for_each_ref_opts);\ndiff --git a/t/perf/p6100-describe.sh b/t/perf/p6100-describe.sh\nindex 069f91ce49..dfcaf59e90 100755\n--- a/t/perf/p6100-describe.sh\n+++ b/t/perf/p6100-describe.sh\n@@ -5,6 +5,12 @@ test_description='performance of git-describe'\n \n test_perf_default_repo\n \n+test_lazy_prereq PERF_REFFILES '\n+\ttest \"$(git rev-parse --show-ref-format)\" = files\n+'\n+\n+ref_count=10000\n+\n # clear out old tags and give us a known state\n test_expect_success 'set up tags' '\n \tgit for-each-ref --format=\"delete %(refname)\" refs/tags >to-delete &&\n@@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '\n \tgit describe --match=new HEAD\n '\n \n+test_expect_success PERF_REFFILES 'set up many unrelated refs' '\n+\tgit tag -m tip tip HEAD &&\n+\tfor i in $(test_seq $ref_count)\n+\tdo\n+\t\tprintf \"create refs/heads/describe-perf/%05d HEAD\\n\" $i ||\n+\t\treturn 1\n+\tdone >instructions &&\n+\tgit update-ref --stdin \n\n"},{"id":"544873","messageId":"aiZoYE8koq1UKlWq@pks.im","threadId":"65767","inReplyTo":"20260607-describe-tag-ref-scope-v1-1-653d232b86b5@gmail.com","subject":"Re: [PATCH] describe: limit default ref iteration to tags","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-06-08T06:59:44Z","receivedAt":"2026-06-08T06:59:50Z","isPatch":true,"body":"On Sun, Jun 07, 2026 at 04:51:53PM -0400, Tamir Duberstein wrote:\n> Unless --all is given, get_name() rejects every ref outside refs/tags/.\n> The rejection happens only after the ref backend has enumerated the ref,\n> so repositories with many other refs spend most of a simple describe\n> invocation visiting refs which cannot affect its result.\n\nRight. The relevant block is this one:\n\n\tif (skip_prefix(ref->name, \"refs/tags/\", &path_to_match)) {\n\t\tis_tag = 1;\n\t} else if (all) {\n\t\tif ((exclude_patterns.nr || patterns.nr) &&\n\t\t !skip_prefix(ref->name, \"refs/heads/\", &path_to_match) &&\n\t\t !skip_prefix(ref->name, \"refs/remotes/\", &path_to_match)) {\n\t\t\t/* Only accept reference of known type if there are match/exclude patterns */\n\t\t\treturn 0;\n\t\t}\n\t} else {\n\t\t/* Reject anything outside refs/tags/ unless --all */\n\t\treturn 0;\n\t}\n\nSo we really only use tags unless \"--all\" is given.\n\n> Commit 8a5a1884e9 (Avoid accessing non-tag refs in git-describe unless\n> --all is requested, 2008-02-24) moved this rejection before object\n> lookup, but left iteration unscoped. Pass the existing refs/tags/\n> restriction to the iterator unless --all is given so the backend can\n> avoid unrelated refs.\n> \n> On a checkout with 124,357 refs, of which 330 were tags, I ran the\n> following command with the parent and patched binaries:\n> \n> hyperfine --warmup 3 --runs 15 \\\n> 'git describe --always --long --abbrev=40 HEAD'\n> \n> The results were:\n> \n> parent this commit\n> elapsed 196.2 ms 63.3 ms\n> user 69.5 ms 48.0 ms\n> system 123.0 ms 12.0 ms\n\nIt's a bit curious that you don't post the hyperfine(1) results as-is\nhere.\n\n> The wall-time standard deviations were 13.2 ms and 2.6 ms, respectively,\n> for a 3.10x speedup.\n\nMakes sense that this would result in a sizeable speedup, depending of\ncourse on the shape of the existing refs in the repository.\n\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index 1c47d7c0b7..3532c8ff22 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -740,6 +740,9 @@ int cmd_describe(int argc,\n> \t\treturn ret;\n> \t}\n> \n> +\tif (!all)\n> +\t\tfor_each_ref_opts.prefix = \"refs/tags/\";\n> +\n> \thashmap_init(&names, commit_name_neq, NULL, 0);\n> \trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n> \t\t\t get_name, NULL, &for_each_ref_opts);\n\nAnother performance optimization that we could do here is to wire up the\nexclude patterns via `for_each_ref_opts.exclude_patterns`. But that's\noutside the scope of this patch series, and also much less likely to\nhelp many use cases out there.\n\n> diff --git a/t/perf/p6100-describe.sh b/t/perf/p6100-describe.sh\n> index 069f91ce49..dfcaf59e90 100755\n> --- a/t/perf/p6100-describe.sh\n> +++ b/t/perf/p6100-describe.sh\n> @@ -5,6 +5,12 @@ test_description='performance of git-describe'\n> \n> test_perf_default_repo\n> \n> +test_lazy_prereq PERF_REFFILES '\n> +\ttest \"$(git rev-parse --show-ref-format)\" = files\n> +'\n> +\n> +ref_count=10000\n\nLet's not declare this variable outside of tests.\n\n> @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '\n> \tgit describe --match=new HEAD\n> '\n> \n> +test_expect_success PERF_REFFILES 'set up many unrelated refs' '\n> +\tgit tag -m tip tip HEAD &&\n> +\tfor i in $(test_seq $ref_count)\n> +\tdo\n> +\t\tprintf \"create refs/heads/describe-perf/%05d HEAD\\n\" $i ||\n> +\t\treturn 1\n> +\tdone >instructions &&\n> +\tgit update-ref --stdin +'\n\nWhy is this limited to the \"files\" backend, only? The logic should work\nfor both backends as-is.\n\nThanks!\n\nPatrick\n"},{"id":"544915","messageId":"xmqqecihyzse.fsf@gitster.g","threadId":"65767","inReplyTo":"20260607-describe-tag-ref-scope-v1-1-653d232b86b5@gmail.com","subject":"Re: [PATCH] describe: limit default ref iteration to tags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-06-08T12:36:33Z","receivedAt":"2026-06-08T12:36:35Z","isPatch":true,"body":"Tamir Duberstein writes:\n\n[jc: Removing Shawn from CC who passed away quite a while ago, RIP].\n\n> Unless --all is given, get_name() rejects every ref outside refs/tags/.\n> The rejection happens only after the ref backend has enumerated the ref,\n> so repositories with many other refs spend most of a simple describe\n> invocation visiting refs which cannot affect its result.\n> ...\n> Both revisions were built with -O3, -mcpu=native, and ThinLTO using\n> Apple clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro\n> (Mac16,6) with a 16-core Apple M4 Max (12 performance and four\n> efficiency cores) and 128 GB RAM.\n>\n> Signed-off-by: Tamir Duberstein \n> ---\n> builtin/describe.c | 3 +++\n> t/perf/p6100-describe.sh | 20 ++++++++++++++++++++\n> 2 files changed, 23 insertions(+)\n\nInteresting. How would this relate to and work well with\n<20260601233727.43558-1-jacob.e.keller@intel.com>?\n\n> +test_lazy_prereq PERF_REFFILES '\n> +\ttest \"$(git rev-parse --show-ref-format)\" = files\n> +'\n> +\n> +ref_count=10000\n> +\n> # clear out old tags and give us a known state\n> test_expect_success 'set up tags' '\n> \tgit for-each-ref --format=\"delete %(refname)\" refs/tags >to-delete &&\n> @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '\n> \tgit describe --match=new HEAD\n> '\n> \n> +test_expect_success PERF_REFFILES 'set up many unrelated refs' '\n> +\tgit tag -m tip tip HEAD &&\n> +\tfor i in $(test_seq $ref_count)\n> +\tdo\n> +\t\tprintf \"create refs/heads/describe-perf/%05d HEAD\\n\" $i ||\n> +\t\treturn 1\n> +\tdone >instructions &&\n> +\tgit update-ref --stdin +'\n> +\n> +test_perf 'describe exact tag with many loose refs' --prereq PERF_REFFILES '\n> +\tgit describe --exact-match HEAD\n> +'\n> +\n\nIs there a strong reason to guard this new test behind\n`PERF_REFFILES`?\n\nEven though the penalty of enumerating 10,000 unrelated loose\nreferences may be most pronounced in the `files` backend, skipping\nunnecessary reference enumeration is an architectural win for other\nbackends (like `reftable` or a fully packed repository) as well.\n\nIf we drop `PERF_REFFILES` and retitle the test to \"describe exact\ntag with many unrelated refs\", we could run it unconditionally to\nbenchmark the improvement across all storage formats.\n"},{"id":"544940","messageId":"CAJ-ks9nPJVM0ik=yua9f2TSKkQWUWEUkZHZBQcdRq3P+3aA3iA@mail.gmail.com","threadId":"65767","inReplyTo":"aiZoYE8koq1UKlWq@pks.im","subject":"Re: [PATCH] describe: limit default ref iteration to tags","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-08T15:46:33Z","receivedAt":"2026-06-08T15:47:13Z","isPatch":true,"body":"On Sun, Jun 7, 2026 at 11:59 PM Patrick Steinhardt wrote:\n>\n> On Sun, Jun 07, 2026 at 04:51:53PM -0400, Tamir Duberstein wrote:\n> > Unless --all is given, get_name() rejects every ref outside refs/tags/.\n> > The rejection happens only after the ref backend has enumerated the ref,\n> > so repositories with many other refs spend most of a simple describe\n> > invocation visiting refs which cannot affect its result.\n>\n> Right. The relevant block is this one:\n>\n> if (skip_prefix(ref->name, \"refs/tags/\", &path_to_match)) {\n> is_tag = 1;\n> } else if (all) {\n> if ((exclude_patterns.nr || patterns.nr) &&\n> !skip_prefix(ref->name, \"refs/heads/\", &path_to_match) &&\n> !skip_prefix(ref->name, \"refs/remotes/\", &path_to_match)) {\n> /* Only accept reference of known type if there are match/exclude patterns */\n> return 0;\n> }\n> } else {\n> /* Reject anything outside refs/tags/ unless --all */\n> return 0;\n> }\n>\n> So we really only use tags unless \"--all\" is given.\n>\n> > Commit 8a5a1884e9 (Avoid accessing non-tag refs in git-describe unless\n> > --all is requested, 2008-02-24) moved this rejection before object\n> > lookup, but left iteration unscoped. Pass the existing refs/tags/\n> > restriction to the iterator unless --all is given so the backend can\n> > avoid unrelated refs.\n> >\n> > On a checkout with 124,357 refs, of which 330 were tags, I ran the\n> > following command with the parent and patched binaries:\n> >\n> > hyperfine --warmup 3 --runs 15 \\\n> > 'git describe --always --long --abbrev=40 HEAD'\n> >\n> > The results were:\n> >\n> > parent this commit\n> > elapsed 196.2 ms 63.3 ms\n> > user 69.5 ms 48.0 ms\n> > system 123.0 ms 12.0 ms\n>\n> It's a bit curious that you don't post the hyperfine(1) results as-is\n> here.\n\nAgreed, will include that in v2. For reference:\n\n Benchmark 1: parent\n Time (mean ± σ): 171.7 ms ± 18.5 ms [User: 23.9 ms,\nSystem: 133.6 ms]\n Range (min … max): 142.3 ms … 198.3 ms 15 runs\n\n Benchmark 2: this commit\n Time (mean ± σ): 9.9 ms ± 1.1 ms [User: 3.3 ms,\nSystem: 4.7 ms]\n Range (min … max): 8.8 ms … 13.1 ms 15 runs\n\n>\n> > The wall-time standard deviations were 13.2 ms and 2.6 ms, respectively,\n> > for a 3.10x speedup.\n>\n> Makes sense that this would result in a sizeable speedup, depending of\n> course on the shape of the existing refs in the repository.\n>\n> > diff --git a/builtin/describe.c b/builtin/describe.c\n> > index 1c47d7c0b7..3532c8ff22 100644\n> > --- a/builtin/describe.c\n> > +++ b/builtin/describe.c\n> > @@ -740,6 +740,9 @@ int cmd_describe(int argc,\n> > return ret;\n> > }\n> >\n> > + if (!all)\n> > + for_each_ref_opts.prefix = \"refs/tags/\";\n> > +\n> > hashmap_init(&names, commit_name_neq, NULL, 0);\n> > refs_for_each_ref_ext(get_main_ref_store(the_repository),\n> > get_name, NULL, &for_each_ref_opts);\n>\n> Another performance optimization that we could do here is to wire up the\n> exclude patterns via `for_each_ref_opts.exclude_patterns`. But that's\n> outside the scope of this patch series, and also much less likely to\n> help many use cases out there.\n\nI tried this and have a separate patch prepared.\n\nThe patterns cannot be passed through verbatim: `git describe\n--exclude=foo` excludes the exact name `foo`, while the refs API would\ntreat `foo` as a directory prefix and also skip `foo/*`. The patch\ntherefore passes only patterns consisting of a literal prefix followed\nby trailing asterisks, adds back the applicable ref namespace, and\nretains the existing callback filtering.\n\nWith 30,000 packed remote-tracking refs under an excluded prefix, the\nperf test invokes `git describe` ten times per run:\n\n```\n master patched\ndescribe excluding many refs 0.16(0.07+0.05) 0.12(0.04+0.05)\n```\n\nThat is a 25% wall-time reduction, with user CPU falling from 0.07 to\n0.04 seconds.\n\nI also tested a larger checkout with 62,170 refs under\n`refs/remotes/origin/`:\n\n```\ngit describe --all --exact-match --exclude='origin/*' HEAD\n```\n\nThis improved from 176.7 ms to 161.3 ms, or about 10%. Startup work\nunrelated to ref iteration dominates more of that repository's runtime.\n\n>\n> > diff --git a/t/perf/p6100-describe.sh b/t/perf/p6100-describe.sh\n> > index 069f91ce49..dfcaf59e90 100755\n> > --- a/t/perf/p6100-describe.sh\n> > +++ b/t/perf/p6100-describe.sh\n> > @@ -5,6 +5,12 @@ test_description='performance of git-describe'\n> >\n> > test_perf_default_repo\n> >\n> > +test_lazy_prereq PERF_REFFILES '\n> > + test \"$(git rev-parse --show-ref-format)\" = files\n> > +'\n> > +\n> > +ref_count=10000\n>\n> Let's not declare this variable outside of tests.\n\nDone in v2.\n\n>\n> > @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '\n> > git describe --match=new HEAD\n> > '\n> >\n> > +test_expect_success PERF_REFFILES 'set up many unrelated refs' '\n> > + git tag -m tip tip HEAD &&\n> > + for i in $(test_seq $ref_count)\n> > + do\n> > + printf \"create refs/heads/describe-perf/%05d HEAD\\n\" $i ||\n> > + return 1\n> > + done >instructions &&\n> > + git update-ref --stdin > +'\n>\n> Why is this limited to the \"files\" backend, only? The logic should work\n> for both backends as-is.\n\nYou're right, fixed in v2.\n\n>\n> Thanks!\n\nThanks for the quick review!\n"},{"id":"544941","messageId":"CAJ-ks9mdzXQsFpLRgC2zKRABX6aKyTcj1RF2nRb_U8jCj6iVZw@mail.gmail.com","threadId":"65767","inReplyTo":"xmqqecihyzse.fsf@gitster.g","subject":"Re: [PATCH] describe: limit default ref iteration to tags","fromName":"Tamir Duberstein","fromEmail":"tamird@gmail.com","sentAt":"2026-06-08T15:53:53Z","receivedAt":"2026-06-08T15:54:32Z","isPatch":true,"body":"On Mon, Jun 8, 2026 at 5:36 AM Junio C Hamano wrote:\n>\n> Tamir Duberstein writes:\n>\n> [jc: Removing Shawn from CC who passed away quite a while ago, RIP].\n>\n> > Unless --all is given, get_name() rejects every ref outside refs/tags/.\n> > The rejection happens only after the ref backend has enumerated the ref,\n> > so repositories with many other refs spend most of a simple describe\n> > invocation visiting refs which cannot affect its result.\n> > ...\n> > Both revisions were built with -O3, -mcpu=native, and ThinLTO using\n> > Apple clang 21.0.0 on macOS 26.5. The machine was a MacBook Pro\n> > (Mac16,6) with a 16-core Apple M4 Max (12 performance and four\n> > efficiency cores) and 128 GB RAM.\n> >\n> > Signed-off-by: Tamir Duberstein \n> > ---\n> > builtin/describe.c | 3 +++\n> > t/perf/p6100-describe.sh | 20 ++++++++++++++++++++\n> > 2 files changed, 23 insertions(+)\n>\n> Interesting. How would this relate to and work well with\n> <20260601233727.43558-1-jacob.e.keller@intel.com>?\n\nThey are orthogonal. That patch changes the argument construction\ninside the `contains` block, which invokes `cmd_name_rev()` and\nreturns. This patch changes the ref iterator used after that block, so\nit only affects the ordinary, non-`--contains` path.\n\n>\n> > +test_lazy_prereq PERF_REFFILES '\n> > + test \"$(git rev-parse --show-ref-format)\" = files\n> > +'\n> > +\n> > +ref_count=10000\n> > +\n> > # clear out old tags and give us a known state\n> > test_expect_success 'set up tags' '\n> > git for-each-ref --format=\"delete %(refname)\" refs/tags >to-delete &&\n> > @@ -27,4 +33,18 @@ test_perf 'describe HEAD with one tag' '\n> > git describe --match=new HEAD\n> > '\n> >\n> > +test_expect_success PERF_REFFILES 'set up many unrelated refs' '\n> > + git tag -m tip tip HEAD &&\n> > + for i in $(test_seq $ref_count)\n> > + do\n> > + printf \"create refs/heads/describe-perf/%05d HEAD\\n\" $i ||\n> > + return 1\n> > + done >instructions &&\n> > + git update-ref --stdin > +'\n> > +\n> > +test_perf 'describe exact tag with many loose refs' --prereq PERF_REFFILES '\n> > + git describe --exact-match HEAD\n> > +'\n> > +\n>\n> Is there a strong reason to guard this new test behind\n> `PERF_REFFILES`?\n>\n> Even though the penalty of enumerating 10,000 unrelated loose\n> references may be most pronounced in the `files` backend, skipping\n> unnecessary reference enumeration is an architectural win for other\n> backends (like `reftable` or a fully packed repository) as well.\n>\n> If we drop `PERF_REFFILES` and retitle the test to \"describe exact\n> tag with many unrelated refs\", we could run it unconditionally to\n> benchmark the improvement across all storage formats.\n\nYeah, there's no good reason - and Patrick made the same observation.\nIn v2 I will remove the prerequisite and rename the case to refer to\nunrelated rather than loose refs.\n"}]}