Re: [PATCH v2 5/5] merge-ort: add prefetching for content merges

[Junio C Hamano <gitster@pobox.com>]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +		/* Ignore clean entries */
> +		if (ci->merged.clean)
> +			continue;
> +
> +		/* Ignore entries that don't need a content merge */
> +		if (ci->match_mask || ci->filemask < 6 ||
> +		    !S_ISREG(ci->stages[1].mode) ||
> +		    !S_ISREG(ci->stages[2].mode) ||
> +		    oideq(&ci->stages[1].oid, &ci->stages[2].oid))
> +			continue;
> +
> +		/* Also don't need content merge if base matches either side */
> +		if (ci->filemask == 7 &&
> +		    S_ISREG(ci->stages[0].mode) &&
> +		    (oideq(&ci->stages[0].oid, &ci->stages[1].oid) ||
> +		     oideq(&ci->stages[0].oid, &ci->stages[2].oid)))
> +			continue;

Even though this is unlikely to change, it is unsatisfactory that we
reproduce the knowledge on the situations when a merge will
trivially resolve and when it will need to go content level.

One obvious way to solve it would be to fold this logic into the
main code that actually merges a list of "ci"s by making it a two
pass process (the first pass does essentially the same as this new
function, the second pass does the tree-level merge where the above
says "continue", fills mmfiles with the loop below, and calls into
ll_merge() after the loop to merge), but the logic duplication is
not too big and it may not be worth such a code churn.

> +		for (i = 0; i < 3; i++) {
> +			unsigned side_mask = (1 << i);
> +			struct version_info *vi = &ci->stages[i];
> +
> +			if ((ci->filemask & side_mask) &&
> +			    S_ISREG(vi->mode) &&
> +			    oid_object_info_extended(opt->repo, &vi->oid, NULL,
> +						     OBJECT_INFO_FOR_PREFETCH))
> +				oid_array_append(&to_fetch, &vi->oid);
> +		}
> +	}
> +
> +	promisor_remote_get_direct(opt->repo, to_fetch.oid, to_fetch.nr);
> +	oid_array_clear(&to_fetch);
> +}
> +