Derrick Stolee <stolee@xxxxxxxxx> writes: >> +int convert_to_sparse(struct index_state *istate, int flags) >> +{ >> + /* >> + * If the index is already sparse, empty, or otherwise >> + * cannot be converted to sparse, do not convert. >> + */ >> + if (istate->sparse_index || !istate->cache_nr || >> + !is_sparse_index_allowed(istate, flags)) >> + return 0; Shouldn't we also at least do this? Blindly blowing away the entire cache-tree and rebuilding it from scratch may be hiding a latent bug somewhere else, but is never supposed to be needed, and is a huge waste of computational resources. I say "at least" here, because a cache tree that is partially valid should be safely salvageable---at least that was the intention back when I designed the subsystem. sparse-index.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git c/sparse-index.c w/sparse-index.c index bc3ee358c6..a95c3386f3 100644 --- c/sparse-index.c +++ w/sparse-index.c @@ -188,17 +188,19 @@ int convert_to_sparse(struct index_state *istate, int flags) if (index_has_unmerged_entries(istate)) return 0; - /* Clear and recompute the cache-tree */ - cache_tree_free(&istate->cache_tree); - /* - * Silently return if there is a problem with the cache tree update, - * which might just be due to a conflict state in some entry. - * - * This might create new tree objects, so be sure to use - * WRITE_TREE_MISSING_OK. - */ - if (cache_tree_update(istate, WRITE_TREE_MISSING_OK)) - return 0; + if (!cache_tree_fully_valid(&istate->cache_tree)) { + /* Clear and recompute the cache-tree */ + cache_tree_free(&istate->cache_tree); + /* + * Silently return if there is a problem with the cache tree update, + * which might just be due to a conflict state in some entry. + * + * This might create new tree objects, so be sure to use + * WRITE_TREE_MISSING_OK. + */ + if (cache_tree_update(istate, WRITE_TREE_MISSING_OK)) + return 0; + } remove_fsmonitor(istate);