Subject: Re: [PATCH v2 1/5] sparse-checkout: refactor skip worktree retry logic

From: Junio C Hamano <[email-masked]>
Date: Thu, 27 Jun 2024 13:59:32 -0700

"Derrick Stolee via GitGitGadget" <[email-masked]> writes:

> -void clear_skip_worktree_from_present_files(struct index_state *istate)
> +static int clear_skip_worktree_from_present_files_sparse(struct index_state *istate)
>  {
>  	const char *last_dirname = NULL;
>  	size_t dir_len = 0;
>  	int dir_found = 1;
>  
> -	int i;
> -	int path_count[2] = {0, 0};
> -	int restarted = 0;
> +	int path_count = 0;
> +	int to_restart = 0;
>  
> -	if (!core_apply_sparse_checkout ||
> -	    sparse_expect_files_outside_of_patterns)
> -		return;
> -
> -	trace2_region_enter("index", "clear_skip_worktree_from_present_files",
> +	trace2_region_enter("index", "clear_skip_worktree_from_present_files_sparse",
>  			    istate->repo);
> -restart:
> -	for (i = 0; i < istate->cache_nr; i++) {
> +	for (int i = 0; i < istate->cache_nr; i++) {
>  		struct cache_entry *ce = istate->cache[i];
>  
>  		if (ce_skip_worktree(ce)) {
> -			path_count[restarted]++;
> +			path_count++;
>  			if (path_found(ce->name, &last_dirname, &dir_len, &dir_found)) {
>  				if (S_ISSPARSEDIR(ce->ce_mode)) {
> -					if (restarted)
> -						BUG("ensure-full-index did not fully flatten?");
> -					ensure_full_index(istate);
> -					restarted = 1;
> -					goto restart;
> +					to_restart = 1;
> +					break;
>  				}
>  				ce->ce_flags &= ~CE_SKIP_WORKTREE;
>  			}
>  		}
>  	}

Both original and the rewritten code shares one trait, which is that
it goes from the beginning to check all paths that are marked with
SKIP_WORKTREE bit to clear CE_SKIP_WORKTREE bits from them, until
they find a S_ISSPARSEDIR entry and lazily call ensure_full_index(),
but then when retrying after calling ensure_full_index(), it
restarts from the beginning.  I wonder if it would help, especially
if the S_ISSPARSEDIR entry comes very late in the index (e.g., by
returning "here is where we have already checked during the first
run, until we realized that we first need to do ensure_full_index()"
to the caller from here, and then the caller tells the second phase
to restart from there), to reduce the number of calls to path_found()?