On Wed, Mar 30, 2022 at 10:52 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "Neeraj Singh via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > + /*
> > + * Allow the object layer to optimize adding multiple objects in
> > + * a batch.
> > + */
> > + begin_odb_transaction();
> > while (ctx.argc) {
> > if (parseopt_state != PARSE_OPT_DONE)
> > parseopt_state = parse_options_step(&ctx, options,
> > @@ -1167,6 +1174,17 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
> > the_index.version = preferred_index_format;
> > }
> >
> > + /*
> > + * It is possible, though unlikely, that a caller could use the verbose
> > + * output to synchronize with addition of objects to the object
> > + * database. The current implementation of ODB transactions leaves
> > + * objects invisible while a transaction is active, so end the
> > + * transaction here if verbose output is enabled.
> > + */
> > +
> > + if (verbose)
> > + end_odb_transaction();
> > +
> > if (read_from_stdin) {
> > struct strbuf buf = STRBUF_INIT;
> > struct strbuf unquoted = STRBUF_INIT;
> > @@ -1190,6 +1208,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
> > strbuf_release(&buf);
> > }
> >
> > + /*
> > + * By now we have added all of the new objects
> > + */
> > + if (!verbose)
> > + end_odb_transaction();
>
> If we had "flush" in addition to "begin" and "end", then we could,
> instead of the above
>
> begin_transaction
> do things
> if condition:
> end_transaction
> loop:
> do thing
> if !condition:
> end_transaction
>
>
> which is somewhat hard to follow and maintain, consider using a
> different flow, which is
>
> begin_transaction
> do things
> loop:
> do thing
> if condition:
> flush
> end_transaction
>
> and that might make it easier to follow and maintain. I am not 100%
> sure if it is worth it, but I am leaning to say it would be.
>
> Thanks.
I thought about this, but I was somewhat worried about the extra cost
of "flushing" versus just making things visible immediately if the
top-level update-index command knows it's going to need objects to be
visible. I'll go ahead and implement flush in the next iteration.
