On Tue, Nov 12, 2024 at 03:41:48PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> writes:
> > - ret = read_ref_without_reload(refs, stack, refname, oid, referent, type);
> > + ret = read_ref_without_reload(refs, be->stack, refname, oid, referent, type);
>
>
> The following bit is curious.
>
> > + ret = backend_for(&be, refs, update->refname, NULL, 0);
> > + if (ret)
> > + return ret;
> > +
>
> We locate one without reloading, and ...
>
> > /*
> > * Search for a preexisting stack update. If there is one then we add
> > * the update to it, otherwise we set up a new stack update.
> > */
> > for (i = 0; !arg && i < tx_data->args_nr; i++)
> > - if (tx_data->args[i].stack == stack)
> > + if (tx_data->args[i].be == be)
> > arg = &tx_data->args[i];
> > if (!arg) {
>
> ... only when we cannot reuse preexisting one, ...
>
> > struct reftable_addition *addition;
> >
> > - ret = reftable_stack_reload(stack);
> > + ret = backend_for(&be, refs, update->refname, NULL, 1);
> > if (ret)
> > return ret;
>
> ... instead of directly doing reload on the instance we already
> have, we do another _for() to locate one, this time reload set to 1.
>
> That looks like doing some redundant work? I am confused.
It indeed is redundant work, yes. And in fact it is redundant work that
isn't really required anymore. My first iteration didn't yet have the
`reftable_write_options::on_reload()` callback, and I instead tried to
catch reloads via `backend_for()`, so it was required to reload via that
function.
But now that we do have the callback that isn't needed anymore, and thus
we don't have to call `backend_for()` a second time here. I'll adapt
this accordingly.
Patrick
