Re: [PATCH v3 2/2] merge: remember conflict labels

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 10/10/2026 01:24, Junio C Hamano wrote:
> 
> This function is called once for each conflicted path, which means
> we would read the "merge.renormalize" configuration variable and the
> MERGE_LABELS file, both of which will stay constant during a single
> conflicted "checkout -m".  The issue is shared with the original,
> but looking up the same configuration variable repeatedly would be
> helped with in-core configset cache.  Compared to that, the overhead
> added by this patch is to open the same unchanging file, read & parse,
> allocate and deallocate.
> 
> Perhaps we want to have another preliminary [PATCH 1.5/2] before
> this step to allow setting these "per invocation constants" once to
> be reused?  Then step [PATCH 2/2] can read labels in the prepare
> phace just once, use it from the data structure in
> checkout_merged(), and free them in release phase when we are done.

That's a good idea, I'll re-roll using the diff below (I assume you're 
happy for me to add your sign off?)

Thanks

Phillip

> 
> 
>   builtin/checkout.c | 37 +++++++++++++++++++++++++++++++------
>   1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git i/builtin/checkout.c w/builtin/checkout.c
> index c0f0d2c700..a02cbacf1d 100644
> --- i/builtin/checkout.c
> +++ w/builtin/checkout.c
> @@ -310,9 +310,27 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos,
>   		return error(_("path '%s' does not have their version"), ce->name);
>   }
>   
> +struct checkout_merged_data {
> +	int conflict_style;
> +	int renormalize;
> +	/* we will add more later */
> +};
> +
> +static void checkout_merged_release(struct checkout_merged_data *data)
> +{
> +	; /* nothing to free (yet) */
> +}
> +
> +static void checkout_merged_prepare(struct checkout_merged_data *data)
> +{
> +	int renormalize = 0;
> +	repo_config_get_bool(the_repository, "merge.renormalize", &renormalize);
> +	data->renormalize = renormalize;
> +}
> +
>   static int checkout_merged(int pos, const struct checkout *state,
>   			   int *nr_checkouts, struct mem_pool *ce_mem_pool,
> -			   int conflict_style)
> +			   struct checkout_merged_data *data)
>   {
>   	struct cache_entry *ce = the_repository->index->cache[pos];
>   	const char *path = ce->name;
> @@ -324,7 +342,6 @@ static int checkout_merged(int pos, const struct checkout *state,
>   	struct object_id threeway[3];
>   	unsigned mode = 0;
>   	struct ll_merge_options ll_opts = LL_MERGE_OPTIONS_INIT;
> -	int renormalize = 0;
>   
>   	memset(threeway, 0, sizeof(threeway));
>   	while (pos < the_repository->index->cache_nr) {
> @@ -345,9 +362,8 @@ static int checkout_merged(int pos, const struct checkout *state,
>   	read_mmblob(&ours, the_repository->objects, &threeway[1]);
>   	read_mmblob(&theirs, the_repository->objects, &threeway[2]);
>   
> -	repo_config_get_bool(the_repository, "merge.renormalize", &renormalize);
> -	ll_opts.renormalize = renormalize;
> -	ll_opts.conflict_style = conflict_style;
> +	ll_opts.renormalize = data->renormalize;
> +	ll_opts.conflict_style = data->conflict_style;
>   	merge_status = ll_merge(&result_buf, path, &ancestor, "base",
>   				&ours, "ours", &theirs, "theirs",
>   				state->istate, &ll_opts);
> @@ -446,6 +462,7 @@ static int checkout_worktree(const struct checkout_opts *opts,
>   	int pos;
>   	int pc_workers, pc_threshold;
>   	struct mem_pool ce_mem_pool;
> +	struct checkout_merged_data checkout_merged_data = {0};
>   
>   	state.force = 1;
>   	state.refresh_cache = 1;
> @@ -462,6 +479,10 @@ static int checkout_worktree(const struct checkout_opts *opts,
>   	if (pc_workers > 1)
>   		init_parallel_checkout();
>   
> +	if (opts->merge) {
> +		checkout_merged_prepare(&checkout_merged_data);
> +		checkout_merged_data.conflict_style = opts->conflict_style;
> +	}
>   	for (pos = 0; pos < the_repository->index->cache_nr; pos++) {
>   		struct cache_entry *ce = the_repository->index->cache[pos];
>   		if (ce->ce_flags & CE_MATCHED) {
> @@ -479,10 +500,14 @@ static int checkout_worktree(const struct checkout_opts *opts,
>   				errs |= checkout_merged(pos, &state,
>   							&nr_unmerged,
>   							&ce_mem_pool,
> -							opts->conflict_style);
> +							&checkout_merged_data);
>   			pos = skip_same_name(ce, pos) - 1;
>   		}
>   	}
> +
> +	if (opts->merge)
> +		checkout_merged_release(&checkout_merged_data);
> +
>   	if (pc_workers > 1)
>   		errs |= run_parallel_checkout(&state, pc_workers, pc_threshold,
>   					      NULL, NULL);