Re: [PATCH 2/5] add: allow operating on a sparse-only index
From: Elijah Newren <hidden>
Date: 2021-07-23 17:46:05
On Wed, Jul 21, 2021 at 2:07 PM Derrick Stolee via GitGitGadget [off-list ref] wrote:
From: Derrick Stolee <redacted> Disable command_requires_full_index for 'git add'. This does not require any additional removals of ensure_full_index(). The main reason is that 'git add' discovers changes based on the pathspec and the worktree itself. These are then inserted into the index directly, and calls to index_name_pos() or index_file_exists() already call expand_to_path() at the appropriate time to support a sparse-index.
Nice.
Add a test to check that 'git add -A' and 'git add <file>' does not expand the index at all, as long as <file> is not within a sparse directory. This does not help the global 'git add .' case.
Good idea.
We can measure the improvement using p2000-sparse-operations.sh with these results: Test HEAD~1 HEAD ------------------------------------------------------------------------------ 2000.6: git add -A (full-index-v3) 0.35(0.30+0.05) 0.37(0.29+0.06) +5.7% 2000.7: git add -A (full-index-v4) 0.31(0.26+0.06) 0.33(0.27+0.06) +6.5% 2000.8: git add -A (sparse-index-v3) 0.57(0.53+0.07) 0.05(0.04+0.08) -91.2% 2000.9: git add -A (sparse-index-v4) 0.58(0.55+0.06) 0.05(0.05+0.06) -91.4% While the 91% improvement seems impressive, it's important to recognize that previously we had significant overhead for expanding the sparse-index. Comparing to the full index case, 'git add -A' goes from 0.37s to 0.05s, which is "only" an 86% improvement.
Hehe. Yep, it's so "disappointing" to "only" have the code be 7x faster. :-) Out of curiosity, IIRC any operation involving the index took ~10s on some of the Microsoft repos. What does the speedup look like over there for these changes to git-add?
quoted hunk ↗ jump to hunk
Signed-off-by: Derrick Stolee <redacted> --- builtin/add.c | 3 +++ t/t1092-sparse-checkout-compatibility.sh | 14 ++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-)diff --git a/builtin/add.c b/builtin/add.c index b773b5a4993..c76e6ddd359 100644 --- a/builtin/add.c +++ b/builtin/add.c@@ -528,6 +528,9 @@ int cmd_add(int argc, const char **argv, const char *prefix) add_new_files = !take_worktree_changes && !refresh_only && !add_renormalize; require_pathspec = !(take_worktree_changes || (0 < addremove_explicit)); + prepare_repo_settings(the_repository); + the_repository->settings.command_requires_full_index = 0; + hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR); /*diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index a3c01d588d8..a11d9d7f35d 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh@@ -340,13 +340,6 @@ test_expect_success 'status/add: outside sparse cone' ' test_sparse_match git status --porcelain=v2 && - # This "git add folder1/a" fails with a warning - # in the sparse repos, differing from the full - # repo. This is intentional. - test_sparse_match test_must_fail git add folder1/a && - test_sparse_match test_must_fail git add --refresh folder1/a && - test_all_match git status --porcelain=v2 && -
Why was this chunk removed? Nothing in the commit message mentions this, and it's not clear to me the reason for it. I tried adding it back in at the end of the series and it still works (and further I can't change test_sparse_match to test_all_match and have the test work).
quoted hunk ↗ jump to hunk
test_all_match git add . && test_all_match git status --porcelain=v2 && test_all_match git commit -m folder1/new &&@@ -635,7 +628,12 @@ test_expect_success 'sparse-index is not expanded' ' git -C sparse-index reset --hard && ensure_not_expanded checkout rename-out-to-out -- deep/deeper1 && git -C sparse-index reset --hard && - ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 + ensure_not_expanded restore -s rename-out-to-out -- deep/deeper1 && + + echo >>sparse-index/README.md && + ensure_not_expanded add -A && + echo >>sparse-index/extra.txt && + ensure_not_expanded add extra.txt
...and here's the extra test you mentioned in the commit message. Looks good.
' # NEEDSWORK: a sparse-checkout behaves differently from a full checkout -- gitgitgadget