Re: [PATCH v5 05/16] refs/reftable: batch refname availability checks

[Patrick Steinhardt <ps@pks.im>]

On Wed, Mar 12, 2025 at 08:54:18PM +0800, shejialuo wrote:
> On Thu, Mar 06, 2025 at 04:08:36PM +0100, Patrick Steinhardt wrote:
> > diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> > index d39a14c5a46..2a90e7cb391 100644
> > --- a/refs/reftable-backend.c
> > +++ b/refs/reftable-backend.c
> > @@ -1379,6 +1375,13 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
> >  		}
> >  	}
> >  
> > +	string_list_sort(&refnames_to_check);
> 
> I am curious why we need to sort the refnames here. I think at current,
> we don't optimize the `refs_verify_refnames_available` function. No
> matter whether the `refnames_to_check` is sorted, it should not
> change the result of `refs_verify_refnames_available` function, right? I
> guess this statement may be related to optimization part. If so, I think
> we should delete this line and add in the later commit.
> 
> However, I am not sure.

You're right, sorting shouldn't be necessary. It was in a previous
version of my patch series, but now that it's not we can drop this.

We may at one point introduce an optimization that does depend on refs
being sorted. But until there is a need we should skip unnecessary work.

Patrick