Re: [PATCH v3] ref-filter: restore prefix-scoped iteration

>
> 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.

>
> > 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.