There's still a long tail of situations where Git expands a sparse index in-memory in order to operate on blob path entries instead of intelligently handling trees. I was recently alerted to one such case with git restore --staged -- ..
The basic idea here is that the pathspec . signals that all paths matter, but what we want to do across those pathspecs will ignore the expanded blob paths with the SKIP_WORKTREE bit, so we should avoid expanding the tree when we can.
This series has two patches: first a test to demonstrate the baseline behavior of git restore across different sparsity cases as well as demonstrate that the index is currently expanded. The second patch includes the fix and maintains the same end-to-end behavior with the only change being the performance improvement from not expanding the sparse index.
Update in v2 ============
The logic around handling a tree entry is extracted to a helper method, making the diff easier to read.
Thanks, -Stolee
Derrick Stolee (2):
t1092: test 'git restore' with sparse index
restore: avoid sparse index expansion
builtin/checkout.c | 65 +++++++++++++++++++++---
t/t1092-sparse-checkout-compatibility.sh | 50 ++++++++++++++++++
2 files changed, 109 insertions(+), 6 deletions(-)
base-commit: aec3f587505a472db67e9462d0702e7d463a449d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2121%2Fderrickstolee%2Frestore-sparse-index-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2121/derrickstolee/restore-sparse-index-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2121
Range-diff vs v1:
1: 7c56d03830 = 1: 7c56d03830 t1092: test 'git restore' with sparse index
2: 47542cbd42 ! 2: 88f5d26a33 restore: avoid sparse index expansion
@@ builtin/checkout.c
#include "submodule.h"
#include "symlinks.h"
@@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm
+ return run_hooks_opt(the_repository, "post-checkout", &opt);
}
++/*
++ * Handle a tree object and determine if we need to recurse into the
++ * tree (READ_TREE_RECURSIVE) or skip it (0).
++ */
++static int try_update_sparse_directory(const struct object_id *oid,
++ struct strbuf *base,
++ const char *pathname,
++ int overlay_mode)
++{
++ struct strbuf dirpath = STRBUF_INIT;
++ struct cache_entry *old;
++ int pos, result = READ_TREE_RECURSIVE;
++
++ if (!the_repository->index->sparse_index)
++ return result;
++
++ strbuf_addbuf(&dirpath, base);
++ strbuf_addstr(&dirpath, pathname);
++ strbuf_addch(&dirpath, '/');
++
++ pos = index_name_pos_sparse(the_repository->index,
++ dirpath.buf, dirpath.len);
++ if (pos < 0)
++ goto cleanup;
++
++ old = the_repository->index->cache[pos];
++ if (!S_ISSPARSEDIR(old->ce_mode))
++ goto cleanup;
++
++ if (oideq(oid, &old->oid)) {
++ /* Tree content already matches; no need to descend. */
++ result = 0;
++ } else if (!overlay_mode) {
++ /*
++ * In non-overlay mode (e.g., restore --staged), replace the
++ * sparse directory OID directly since files not present in
++ * the source tree should be removed anyway.
++ */
++ oidcpy(&old->oid, oid);
++ old->ce_flags |= CE_UPDATE;
++ result = 0;
++ }
++
++cleanup:
++ strbuf_release(&dirpath);
++ return result;
++}
++
static int update_some(const struct object_id *oid, struct strbuf *base,
- const char *pathname, unsigned mode, void *context UNUSED)
+ const char *pathname, unsigned mode, void *context)
@@ builtin/checkout.c: static int post_checkout_hook(struct commit *old_commit, str
int pos;
+ int overlay_mode = context ? *((int *)context) : 1;
-- if (S_ISDIR(mode))
-+ if (S_ISDIR(mode)) {
-+ /*
-+ * If this directory exists as a sparse directory entry in
-+ * the index, we can handle it at the tree level without
-+ * descending into individual files.
-+ */
-+ if (the_repository->index->sparse_index) {
-+ struct strbuf dirpath = STRBUF_INIT;
-+
-+ strbuf_addbuf(&dirpath, base);
-+ strbuf_addstr(&dirpath, pathname);
-+ strbuf_addch(&dirpath, '/');
-+
-+ pos = index_name_pos_sparse(the_repository->index,
-+ dirpath.buf, dirpath.len);
-+ if (pos >= 0) {
-+ struct cache_entry *old =
-+ the_repository->index->cache[pos];
-+ if (S_ISSPARSEDIR(old->ce_mode)) {
-+ if (oideq(oid, &old->oid)) {
-+ strbuf_release(&dirpath);
-+ return 0;
-+ }
-+ if (!overlay_mode) {
-+ /*
-+ * In non-overlay mode (e.g.,
-+ * restore --staged), we can
-+ * replace the sparse dir OID
-+ * directly since files not in
-+ * the source tree should be
-+ * removed anyway.
-+ */
-+ oidcpy(&old->oid, oid);
-+ old->ce_flags |= CE_UPDATE;
-+ strbuf_release(&dirpath);
-+ return 0;
-+ }
-+ }
-+ }
-+ strbuf_release(&dirpath);
-+ }
- return READ_TREE_RECURSIVE;
-+ }
+ if (S_ISDIR(mode))
+- return READ_TREE_RECURSIVE;
++ return try_update_sparse_directory(oid, base, pathname,
++ overlay_mode);
len = base->len + strlen(pathname);
ce = make_empty_cache_entry(the_repository->index, len);