Re: [PATCH v4 2/4] sparse-index: avoid unnecessary cache tree clearing

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 29, 2021, 18:45 UTC
Message-ID: <xmqqfssjllen.fsf@gitster.g>
In-Reply-To: <91351ac4bded2c19264cd2009e90ee71fcf67b81.1635515487.git.gitgitgadget@gmail.com>

"Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> From: Victoria Dye <vdye@github.com>
>
> When converting a full index to sparse, clear and recreate the cache tree
> only if the cache tree is not fully valid. The convert_to_sparse operation
> should exit silently if a cache tree update cannot be successfully completed
> (e.g., due to a conflicted entry state). However, because this failure
> scenario only occurs when at least a portion of the cache tree is invalid,
> we can save ourselves the cost of clearing and recreating the cache tree by
> skipping the check when the cache tree is fully valid.

I see in cache-tree.c::update_one() this snippet of code.

	/*
	 * If the first entry of this region is a sparse directory
	 * entry corresponding exactly to 'base', then this cache_tree
	 * struct is a "leaf" in the data structure, pointing to the
	 * tree OID specified in the entry.
	 */
	if (entries > 0) {
		const struct cache_entry *ce = cache[0];

		if (S_ISSPARSEDIR(ce->ce_mode) &&
		    ce->ce_namelen == baselen &&
		    !strncmp(ce->name, base, baselen)) {
			it->entry_count = 1;
			oidcpy(&it->oid, &ce->oid);
			return 1;
		}
	}

Sorry for not noticing it earlier, but does this mean that the content of a cache-tree changes shape when sparse-ness of the index changes? Is a cache-tree that knows about all of the subdirectories, even the ones that are descendants of a directory that is represented as a tree-ish entry in the main index array, still valid in a sparse index?

If not, then I do not think of a quick and sure way to ensure that the cache-tree is valid when the sparse-ness changes.

The earlier suggestion was based on my assumption that even when the main index array becomes sparse, the cache tree is still populated and valid, so that after writing a tree and writing an on-disk index, and then reading the on-disk index back (possibly in another process), would not have to incur the recomputation cost of the full tree when the reading codepath needs to flip the sparseness.

But the above code snippet makes me worried a lot. A cache-tree that used to be valid when the corresponding in-core index array was not sparse will become invalid immediately when we decide to make it sparse, right?