On Thu, Mar 06, 2025 at 04:08:36PM +0100, Patrick Steinhardt wrote: > Refactor the "reftable" backend to batch the availability check for > refnames. This does not yet have an effect on performance as > `refs_verify_refnames_available()` effectively still performs the > availability check for each refname individually. But this will be > optimized in subsequent commits, where we learn to optimize some parts > of the logic when checking multiple refnames for availability. > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > refs/reftable-backend.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > 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 > @@ -1069,6 +1069,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, > reftable_be_downcast(ref_store, REF_STORE_WRITE|REF_STORE_MAIN, "ref_transaction_prepare"); > struct strbuf referent = STRBUF_INIT, head_referent = STRBUF_INIT; > struct string_list affected_refnames = STRING_LIST_INIT_NODUP; > + struct string_list refnames_to_check = STRING_LIST_INIT_NODUP; > struct reftable_transaction_data *tx_data = NULL; > struct reftable_backend *be; > struct object_id head_oid; > @@ -1224,12 +1225,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, > * can output a proper error message instead of failing > * at a later point. > */ > - ret = refs_verify_refname_available(ref_store, u->refname, > - &affected_refnames, NULL, > - transaction->flags & REF_TRANSACTION_FLAG_INITIAL, > - err); > - if (ret < 0) > - goto done; > + string_list_append(&refnames_to_check, u->refname); > Instead of checking in `prepare`, we will just append the refname to the list. > /* > * There is no need to write the reference deletion > @@ -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. > + ret = refs_verify_refnames_available(ref_store, &refnames_to_check, &affected_refnames, NULL, > + transaction->flags & REF_TRANSACTION_FLAG_INITIAL, > + err); > + if (ret < 0) > + goto done; > + > transaction->backend_data = tx_data; > transaction->state = REF_TRANSACTION_PREPARED; > > @@ -1394,6 +1397,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, > string_list_clear(&affected_refnames, 0); > strbuf_release(&referent); > strbuf_release(&head_referent); > + string_list_clear(&refnames_to_check, 0); > > return ret; > }
