[PATCH v4] ref-filter: restore prefix-scoped iteration
- From
Tamir Duberstein <tamird@gmail.com>
- Date
- Jun 12, 2026, 21:27 UTC
- Message-ID
- <20260612-fix-git-branch-regression-v4-1-f150038c02f4@gmail.com>
- In-Reply-To
- <20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com>
dabecb9db2 (for-each-ref: introduce a '--start-after' option, 2025-07-15) changed branch, remote-tracking branch, and tag enumeration from constructing an iterator with the namespace prefix to constructing an unscoped iterator and seeking to the prefix.
Review of --start-after noted that the construction prefix and seek position represent different state and are easy to conflate [1]. It also noted that future branch or tag support would need to retain the namespace prefix while moving the cursor [2].
The files backend constructs its loose-ref iterator with cache priming enabled. cache_ref_iterator_begin() immediately applies the construction prefix through cache_ref_iterator_set_prefix(), reading loose refs beneath it before packed refs are opened. An empty prefix therefore reads every loose ref, and a later seek cannot undo that I/O.
For the current single-kind filters, construct the iterator with the namespace prefix when start_after is not set. Leave the existing start_after path unchanged; no current command combines it with these filters, and future support must carry the prefix separately from the cursor.
With 10,000 unrelated loose refs in the files backend, the p6300 tests improve as follows:
before after branch 2.74 s 0.11 s branch --remotes 2.81 s 0.12 s tag 3.01 s 0.11 s
[1] https://lore.kernel.org/r/aGZidwwlToWThkn8@pks.im/ [2] https://lore.kernel.org/r/xmqqikjq7s16.fsf@gitster.g/
Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
Suggested-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
The series is based on a89346e34a (maint) because the regression has
been present in released versions since Git 2.51.0.
---
Changes in v4:
- Explain the historical references in the commit message.
- Run the new performance cases with both ref backends.
- Drop the Assisted-by trailer.
- Link to v3: https://patch.msgid.link/20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.comChanges in v3: - Construct the iterator directly with the namespace prefix. - Explain when the files backend primes its loose-ref cache. - Condense the commit message and performance results. - Link to v2: https://patch.msgid.link/20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com
Changes in v2: - Extract local variable `store`. - Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com --- ref-filter.c | 13 ++++++------- t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c index 1da4c0e60d..9b04e3af85 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for if (prefix) { struct ref_iterator *iter; + struct ref_store *store = get_main_ref_store(the_repository); - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository), - "", NULL, 0, 0); - - if (filter->start_after) + if (filter->start_after) { + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0); ret = start_ref_iterator_after(iter, filter->start_after); - else - ret = ref_iterator_seek(iter, prefix, - REF_ITERATOR_SEEK_SET_PREFIX); + } else { + iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0); + } if (!ret) ret = do_for_each_ref_iterator(iter, fn, cb_data); diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh index fa7289c752..25ffa5e84c 100755 --- a/t/perf/p6300-for-each-ref.sh +++ b/t/perf/p6300-for-each-ref.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='performance of for-each-ref' +test_description='performance of ref-filter users' . ./perf-lib.sh test_perf_fresh_repo @@ -84,4 +84,41 @@ test_expect_success 'pack refs' ' ' run_tests "packed" +test_expect_success 'setup many unrelated refs' ' + git init scoped && + test_commit -C scoped --no-tag base && + test_seq $ref_count_per_type | + sed "s,.*,update refs/custom/unrelated_& HEAD," | + git -C scoped update-ref --stdin && + git -C scoped update-ref refs/remotes/origin/main HEAD && + git -C scoped update-ref refs/tags/only HEAD +' + +test_perf "branch (many unrelated refs)" " + ( + cd scoped && + for i in \$(test_seq $test_iteration_count); do + git branch --format='%(refname)' >/dev/null + done + ) +" + +test_perf "branch --remotes (many unrelated refs)" " + ( + cd scoped && + for i in \$(test_seq $test_iteration_count); do + git branch --remotes --format='%(refname)' >/dev/null + done + ) +" + +test_perf "tag (many unrelated refs)" " + ( + cd scoped && + for i in \$(test_seq $test_iteration_count); do + git tag --format='%(refname)' >/dev/null + done + ) +" + test_done --- base-commit: a89346e34a937f001e5d397ee62224e3e9852040 change-id: 20260605-fix-git-branch-regression-9e4236f18091 Best regards, -- Tamir Duberstein <tamird@gmail.com>