Subject: Re: [PATCH v5 05/14] cache-tree: use ODB transaction around writing a tree

From: Junio C Hamano <[email-masked]>
Date: Wed, 30 Mar 2022 10:46:59 -0700

"Neeraj Singh via GitGitGadget" <[email-masked]> writes:

> From: Neeraj Singh <[email-masked]>
>
> Take advantage of the odb transaction infrastructure around writing the
> cached tree to the object database.
>
> Signed-off-by: Neeraj Singh <[email-masked]>
> ---
>  cache-tree.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/cache-tree.c b/cache-tree.c
> index 6752f69d515..8c5e8822716 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -3,6 +3,7 @@
>  #include "tree.h"
>  #include "tree-walk.h"
>  #include "cache-tree.h"
> +#include "bulk-checkin.h"
>  #include "object-store.h"
>  #include "replace-object.h"
>  #include "promisor-remote.h"
> @@ -474,8 +475,10 @@ int cache_tree_update(struct index_state *istate, int flags)
>  
>  	trace_performance_enter();
>  	trace2_region_enter("cache_tree", "update", the_repository);

There is no I/O in update_one() when the WRITE_TREE_DRY_RUN bit is
set, so we _could_ optimize the begin/end away with

	if (!(flags & WRITE_TREE_DRY_RUN))
		begin_odb_transaction()

> +	begin_odb_transaction();
>  	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
>  		       "", 0, &skip, flags);
> +	end_odb_transaction();
>  	trace2_region_leave("cache_tree", "update", the_repository);
>  	trace_performance_leave("cache_tree_update");
>  	if (i < 0)

I do not know if that is worth it.  If we do not do any object
creation inside begin/end, we don't even create the temporary object
directory and there is nothing we need to do when we "unplug".  So
this would be fine as-is, but I may be overlooking something, so I
thought I'd mention it for completeness.

Thanks.