--- v6
+++ v3
@@ -1,114 +1,113 @@
From: Derrick Stolee <dstolee@microsoft.com>
-By testing 'git -c core.fsmonitor= status -uno', we can check for the
-simplest index operations that can be made sparse-aware. The necessary
-implementation details are already integrated with sparse-checkout, so
-modify command_requires_full_index to be zero for cmd_status().
+If we need to expand a sparse-index into a full one, then the FS Monitor
+bitmap is going to be incorrect. Ensure that we start fresh at such an
+event.
-In refresh_index(), we loop through the index entries to refresh their
-stat() information. However, sparse directories have no stat()
-information to populate. Ignore these entries.
+While this is currently a performance drawback, the eventual hope of the
+sparse-index feature is that these expansions will be rare and hence we
+will be able to keep the FS Monitor data accurate across multiple Git
+commands.
-This allows 'git status' to no longer expand a sparse index to a full
-one. This is further tested by dropping the "-uno" option and adding an
-untracked file into the worktree.
-
-The performance test p2000-sparse-checkout-operations.sh demonstrates
-these improvements:
-
-Test HEAD~1 HEAD
------------------------------------------------------------------------------
-2000.2: git status (full-index-v3) 0.31(0.30+0.05) 0.31(0.29+0.06) +0.0%
-2000.3: git status (full-index-v4) 0.31(0.29+0.07) 0.34(0.30+0.08) +9.7%
-2000.4: git status (sparse-index-v3) 2.35(2.28+0.10) 0.04(0.04+0.05) -98.3%
-2000.5: git status (sparse-index-v4) 2.35(2.24+0.15) 0.05(0.04+0.06) -97.9%
-
-Note that since HEAD~1 was expanding the sparse index by parsing trees,
-it was artificially slower than the full index case. Thus, the 98%
-improvement is misleading, and instead we should celebrate the 0.34s to
-0.05s improvement of 85%. This is more indicative of the peformance
-gains we are expecting by using a sparse index.
-
-Note: we are dropping the assignment of core.fsmonitor here. This is not
-necessary for the test script as we are not altering the config any
-other way. Correct integration with FS Monitor will be validated in
-later changes.
+These tests are added to demonstrate that the behavior is the same
+across a full index and a sparse index, but also that file modifications
+to a tracked directory outside of the sparse cone will trigger
+ensure_full_index().
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
- builtin/commit.c | 3 +++
- read-cache.c | 10 ++++++++--
- t/t1092-sparse-checkout-compatibility.sh | 13 +++++++++----
- 3 files changed, 20 insertions(+), 6 deletions(-)
+ sparse-index.c | 7 ++++++
+ t/t7519-status-fsmonitor.sh | 48 +++++++++++++++++++++++++++++++++++++
+ 2 files changed, 55 insertions(+)
-diff --git a/builtin/commit.c b/builtin/commit.c
-index 190d215d43b..12f51db158a 100644
---- a/builtin/commit.c
-+++ b/builtin/commit.c
-@@ -1510,6 +1510,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
- if (argc == 2 && !strcmp(argv[1], "-h"))
- usage_with_options(builtin_status_usage, builtin_status_options);
+diff --git a/sparse-index.c b/sparse-index.c
+index b2b3fbd75050..32ba0d17ef7c 100644
+--- a/sparse-index.c
++++ b/sparse-index.c
+@@ -195,6 +195,10 @@ int convert_to_sparse(struct index_state *istate)
+ cache_tree_free(&istate->cache_tree);
+ cache_tree_update(istate, 0);
-+ prepare_repo_settings(the_repository);
-+ the_repository->settings.command_requires_full_index = 0;
++ istate->fsmonitor_has_run_once = 0;
++ FREE_AND_NULL(istate->fsmonitor_dirty);
++ FREE_AND_NULL(istate->fsmonitor_last_update);
+
- status_init_config(&s, git_status_config);
- argc = parse_options(argc, argv, prefix,
- builtin_status_options,
-diff --git a/read-cache.c b/read-cache.c
-index 1b3c2eb408b..277c2970a03 100644
---- a/read-cache.c
-+++ b/read-cache.c
-@@ -1584,8 +1584,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,
- */
- preload_index(istate, pathspec, 0);
- trace2_region_enter("index", "refresh", NULL);
-- /* TODO: audit for interaction with sparse-index. */
-- ensure_full_index(istate);
-+
- for (i = 0; i < istate->cache_nr; i++) {
- struct cache_entry *ce, *new_entry;
- int cache_errno = 0;
-@@ -1600,6 +1599,13 @@ int refresh_index(struct index_state *istate, unsigned int flags,
- if (ignore_skip_worktree && ce_skip_worktree(ce))
- continue;
+ istate->sparse_index = 1;
+ trace2_region_leave("index", "convert_to_sparse", istate->repo);
+ return 0;
+@@ -291,6 +295,9 @@ void ensure_full_index(struct index_state *istate)
+ istate->cache = full->cache;
+ istate->cache_nr = full->cache_nr;
+ istate->cache_alloc = full->cache_alloc;
++ istate->fsmonitor_has_run_once = 0;
++ FREE_AND_NULL(istate->fsmonitor_dirty);
++ FREE_AND_NULL(istate->fsmonitor_last_update);
-+ /*
-+ * If this entry is a sparse directory, then there isn't
-+ * any stat() information to update. Ignore the entry.
-+ */
-+ if (S_ISSPARSEDIR(ce->ce_mode))
-+ continue;
-+
- if (pathspec && !ce_path_match(istate, ce, pathspec, seen))
- filtered = 1;
-
-diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
-index 9035adcb7db..1e9737cb4b7 100755
---- a/t/t1092-sparse-checkout-compatibility.sh
-+++ b/t/t1092-sparse-checkout-compatibility.sh
-@@ -508,12 +508,17 @@ test_expect_success 'sparse-index is expanded and converted back' '
- GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
- git -C sparse-index -c core.fsmonitor="" reset --hard &&
- test_region index convert_to_sparse trace2.txt &&
-- test_region index ensure_full_index trace2.txt &&
-+ test_region index ensure_full_index trace2.txt
-+'
-
-- rm trace2.txt &&
-+test_expect_success 'sparse-index is not expanded' '
-+ init_repos &&
-+
-+ rm -f trace2.txt &&
-+ echo >>sparse-index/untracked.txt &&
- GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-- git -C sparse-index -c core.fsmonitor="" status -uno &&
-- test_region index ensure_full_index trace2.txt
-+ git -C sparse-index status &&
-+ test_region ! index ensure_full_index trace2.txt
+ strbuf_release(&base);
+ free(full);
+diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
+index 45d025f96010..f70fe961902e 100755
+--- a/t/t7519-status-fsmonitor.sh
++++ b/t/t7519-status-fsmonitor.sh
+@@ -73,6 +73,7 @@ test_expect_success 'setup' '
+ expect*
+ actual*
+ marker*
++ trace2*
+ EOF
'
+@@ -383,4 +384,51 @@ test_expect_success 'status succeeds after staging/unstaging' '
+ )
+ '
+
++# Usage:
++# check_sparse_index_behavior [!]
++# If "!" is supplied, then we verify that we do not call ensure_full_index
++# during a call to 'git status'. Otherwise, we verify that we _do_ call it.
++check_sparse_index_behavior () {
++ git status --porcelain=v2 >expect &&
++ git sparse-checkout init --cone --sparse-index &&
++ git sparse-checkout set dir1 dir2 &&
++ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
++ git status --porcelain=v2 >actual &&
++ test_region $1 index ensure_full_index trace2.txt &&
++ test_cmp expect actual &&
++ rm trace2.txt &&
++ git sparse-checkout disable
++}
++
++test_expect_success 'status succeeds with sparse index' '
++ git reset --hard &&
++
++ test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" &&
++ check_sparse_index_behavior ! &&
++
++ write_script .git/hooks/fsmonitor-test<<-\EOF &&
++ printf "last_update_token\0"
++ EOF
++ git config core.fsmonitor .git/hooks/fsmonitor-test &&
++ check_sparse_index_behavior ! &&
++
++ write_script .git/hooks/fsmonitor-test<<-\EOF &&
++ printf "last_update_token\0"
++ printf "dir1/modified\0"
++ EOF
++ check_sparse_index_behavior ! &&
++
++ cp -r dir1 dir1a &&
++ git add dir1a &&
++ git commit -m "add dir1a" &&
++
++ # This one modifies outside the sparse-checkout definition
++ # and hence we expect to expand the sparse-index.
++ write_script .git/hooks/fsmonitor-test<<-\EOF &&
++ printf "last_update_token\0"
++ printf "dir1a/modified\0"
++ EOF
++ check_sparse_index_behavior
++'
++
test_done
--
gitgitgadget
-