From: Tamir Duberstein Date: Fri, 12 Jun 2026 21:24:49 GMT Subject: Re: [PATCH v3] ref-filter: restore prefix-scoped iteration Message-ID: In-Reply-To: On Fri, Jun 12, 2026 at 7:48 AM Patrick Steinhardt wrote: > > On Wed, Jun 10, 2026 at 05:29:49AM -0700, Tamir Duberstein wrote: > > 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. > > > > 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 these single-kind filters, construct the iterator with the namespace > > prefix when start_after is not set. Keep the existing unscoped > > construction for start_after, whose seek position may differ from the > > namespace prefix. > > > > With 10,000 unrelated loose refs, 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 > > > > Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/ > > Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/ > > Link: https://lore.kernel.org/r/CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com > > I honestly have no idea what you want to say with these links, as they > seem to just link to random reviews mails when the above mentioned > commit was reviewed. In general, we typically try to embed references > like this into the explanation, like: > > In [1], we discussed... and this is relevant because of ... > > [1]: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/ > > Just dropping the links as-is without much of an explanation isn't > helpful. Will be numbered references in next spin. > > > 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); > > The patch itself seems sensible to me. > > > diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh > > index fa7289c752..ed9c1c6a19 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 REFFILES 'setup many unrelated loose 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 > > +' > > I've already called this out before on other patches, but the REFFILES > prerequisite just doesn't make any sense here as this test logic is > generic. You're right. Removed in v4.