From: Karthik Nayak Date: Tue, 16 Sep 2025 09:07:56 GMT Subject: Re: [PATCH v2 2/6] builtin/update-index: end ODB transaction when --verbose is specified Message-ID: In-Reply-To: <20250915202956.3784935-3-jltobler@gmail.com> Justin Tobler writes: > With 23a3a303 (update-index: use the bulk-checkin infrastructure, > 2022-04-04), object database transactions were added to > git-update-index(1) to facilitate writing objects in bulk. With > transactions, newly added objects are instead written to a temporary > object directory and migrated to the primary object database upon > transaction commit. > > When the --verbose option is specified, each of the following objects is > explicitly flushed via flush_odb_transaction() prior to reporting the > update. Flushing the object database transaction migrates pending > objects to the primary object database without marking the transaction > as complete. This is done so objects are immediately visible to > git-update-index(1) callers using the --verbose option and that rely on > parsing verbose output to know when objects are written. > > Due to how git-update-index(1) parses options, each filename argument is > evaluated with only the set of options that precede it. Therefore, it is > possible for an initial set of objects to be written in a transaction > before a --verbose option is encountered. > > As soon as the --verbose option is parsed in git-update-index(1), all > subsequent object writes are flushed prior to being reported and thus no > longer benefit from being transactional. Furthermore, the mechanism to > flush a transaction without committing is rather awkward. Drop the call > to flush_odb_transaction() in favor of ending the transaction early when > the --verbose flag is encountered. > > Signed-off-by: Justin Tobler > --- > builtin/update-index.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/builtin/update-index.c b/builtin/update-index.c > index 2ba2d29c95..d36bc55752 100644 > --- a/builtin/update-index.c > +++ b/builtin/update-index.c > @@ -70,14 +70,6 @@ static void report(const char *fmt, ...) > if (!verbose) > return; > > - /* > - * 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 flush the > - * transaction here before reporting a change made by update-index. > - */ > - flush_odb_transaction(the_repository->objects->transaction); > va_start(vp, fmt); > vprintf(fmt, vp); > putchar('\n'); > @@ -1150,6 +1142,21 @@ int cmd_update_index(int argc, > const char *path = ctx.argv[0]; > char *p; > > + /* > + * 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 early before processing the next > + * update. All further updates are performed outside of > + * a transaction. > + */ > + if (transaction && verbose) { > + end_odb_transaction(transaction); > + transaction = NULL; > + } > + So with this change, we now have all objects updated before the `--verbose` flag updated via a single transaction. Updates after the `--verbose` flag will no longer use a transaction. The older version would flush the transaction on every report, is there is any benefits to the new flow with regards to performance? > setup_work_tree(); > p = prefix_path(prefix, prefix_length, path); > update_one(p); > -- > 2.51.0.193.g4975ec3473b