From eb10e637cf04a19b9a3d5d7c904d71bc9e4ea408 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:40 +0000 Subject: [PATCH 01/11] p7519: do not rely on "xargs -d" in test Convert the test to use a more portable method to update the mtime on a large number of files under version control. The Mac version of xargs does not support the "-d" option. Likewise, the "-0" and "--null" options are not portable. Furthermore, use `test-tool chmtime` rather than `touch` to update the mtime to ensure that it is actually updated (especially on file systems with only whole second resolution). Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/perf/p7519-fsmonitor.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh index 9b43342806b9e0..6677e0ef7ab334 100755 --- a/t/perf/p7519-fsmonitor.sh +++ b/t/perf/p7519-fsmonitor.sh @@ -164,8 +164,18 @@ test_fsmonitor_suite() { git status -uall ' + # Update the mtimes on upto 100k files to make status think + # that they are dirty. For simplicity, omit any files with + # LFs (i.e. anything that ls-files thinks it needs to dquote). + # Then fully backslash-quote the paths to capture any + # whitespace so that they pass thru xargs properly. + # test_perf_w_drop_caches "status (dirty) ($DESC)" ' - git ls-files | head -100000 | xargs -d "\n" touch -h && + git ls-files | \ + head -100000 | \ + grep -v \" | \ + sed '\''s/\(.\)/\\\1/g'\'' | \ + xargs test-tool chmtime -300 && git status ' From 0917763d67b99c22d980a10e94102047cc4e9a73 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:41 +0000 Subject: [PATCH 02/11] p7519: fix watchman watch-list test on Windows Only use the final portion of the test trash directory file name when verifying that Watchman was started. On Windows and under the SDK, $GIT_WORKTREE is a cygwin-style path with forward slashes and a "/c/" drive name. However `watchman watch-list` reports a proper Windows-style pathname with drive letters and backslashes. This causes the grep to fail. Since we don't really care about the full pathname (and we really don't want to bother with normalizaing them), just see if the test-name portion of the path is found. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/perf/p7519-fsmonitor.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh index 6677e0ef7ab334..21d525541d5932 100755 --- a/t/perf/p7519-fsmonitor.sh +++ b/t/perf/p7519-fsmonitor.sh @@ -101,7 +101,7 @@ test_expect_success "one time repo setup" ' # If Watchman exists, watch the work tree and attempt a query. if test_have_prereq WATCHMAN; then watchman watch "$GIT_WORK_TREE" && - watchman watch-list | grep -q -F "$GIT_WORK_TREE" + watchman watch-list | grep -q -F "p7519-fsmonitor" fi ' From a7556c3bde48b9266f11794a20d12719269becd3 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:42 +0000 Subject: [PATCH 03/11] p7519: move watchman cleanup earlier in the test Shutdown Watchman after the Watchman-based tests and before the block of "no fsmonitor" tests. This helps ensure that Watchman cannot affect the test results for the other. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/perf/p7519-fsmonitor.sh | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh index 21d525541d5932..78e7d2c03f5618 100755 --- a/t/perf/p7519-fsmonitor.sh +++ b/t/perf/p7519-fsmonitor.sh @@ -208,6 +208,11 @@ test_fsmonitor_suite() { ' } +# +# Run a full set of perf tests using each Hook-based fsmonitor provider, +# such as Watchman. +# + if test -n "$GIT_PERF_7519_FSMONITOR"; then for INTEGRATION_PATH in $GIT_PERF_7519_FSMONITOR; do test_expect_success "setup for fsmonitor $INTEGRATION_PATH" 'setup_for_fsmonitor' @@ -218,14 +223,6 @@ else test_fsmonitor_suite fi -test_expect_success "setup without fsmonitor" ' - unset INTEGRATION_SCRIPT && - git config --unset core.fsmonitor && - git update-index --no-fsmonitor -' - -test_fsmonitor_suite - if test_have_prereq WATCHMAN then watchman watch-del "$GIT_WORK_TREE" >/dev/null 2>&1 && @@ -235,4 +232,16 @@ then watchman shutdown-server >/dev/null 2>&1 fi +# +# Run a full set of perf tests with the fsmonitor feature disabled. +# + +test_expect_success "setup without fsmonitor" ' + unset INTEGRATION_SCRIPT && + git config --unset core.fsmonitor && + git update-index --no-fsmonitor +' + +test_fsmonitor_suite + test_done From 4f2009dce2d270ded8bb94417becf6cc2143c70c Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:43 +0000 Subject: [PATCH 04/11] p7519: add trace logging during perf test Add optional trace logging to allow us to better compare performance of various fsmonitor providers and compare results with non-fsmonitor runs. Currently, this includes Trace2 logging, but may be extended to include other trace targets, such as GIT_TRACE_FSMONITOR if desired. Using this logging helped me explain an odd behavior on MacOS where the kernel was dropping events and causing the hook to Watchman to timeout. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- t/perf/.gitignore | 1 + t/perf/Makefile | 4 ++-- t/perf/p7519-fsmonitor.sh | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/t/perf/.gitignore b/t/perf/.gitignore index 982eb8e3a949a7..72f5d0d3148943 100644 --- a/t/perf/.gitignore +++ b/t/perf/.gitignore @@ -1,3 +1,4 @@ /build/ /test-results/ +/test-trace/ /trash directory*/ diff --git a/t/perf/Makefile b/t/perf/Makefile index fcb0e8865e4933..2465770a7825b4 100644 --- a/t/perf/Makefile +++ b/t/perf/Makefile @@ -7,10 +7,10 @@ perf: pre-clean ./run pre-clean: - rm -rf test-results + rm -rf test-results test-trace clean: - rm -rf build "trash directory".* test-results + rm -rf build "trash directory".* test-results test-trace test-lint: $(MAKE) -C .. test-lint diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh index 78e7d2c03f5618..aa0ea0e2ec8c2f 100755 --- a/t/perf/p7519-fsmonitor.sh +++ b/t/perf/p7519-fsmonitor.sh @@ -32,6 +32,8 @@ test_description="Test core.fsmonitor" # # GIT_PERF_7519_DROP_CACHE: if set, the OS caches are dropped between tests # +# GIT_PERF_7519_TRACE: if set, enable trace logging during the test. +# Trace logs will be grouped by fsmonitor provider. test_perf_large_repo test_checkout_worktree @@ -70,6 +72,32 @@ then fi fi +trace_start() { + if test -n "$GIT_PERF_7519_TRACE" + then + name="$1" + TEST_TRACE_DIR="$TEST_OUTPUT_DIRECTORY/test-trace/p7519/" + echo "Writing trace logging to $TEST_TRACE_DIR" + + mkdir -p "$TEST_TRACE_DIR" + + # Start Trace2 logging and any other GIT_TRACE_* logs that you + # want for this named test case. + + GIT_TRACE2_PERF="$TEST_TRACE_DIR/$name.trace2perf" + export GIT_TRACE2_PERF + + >"$GIT_TRACE2_PERF" + fi +} + +trace_stop() { + if test -n "$GIT_PERF_7519_TRACE" + then + unset GIT_TRACE2_PERF + fi +} + test_expect_success "one time repo setup" ' # set untrackedCache depending on the environment if test -n "$GIT_PERF_7519_UNTRACKED_CACHE" @@ -213,6 +241,7 @@ test_fsmonitor_suite() { # such as Watchman. # +trace_start fsmonitor-watchman if test -n "$GIT_PERF_7519_FSMONITOR"; then for INTEGRATION_PATH in $GIT_PERF_7519_FSMONITOR; do test_expect_success "setup for fsmonitor $INTEGRATION_PATH" 'setup_for_fsmonitor' @@ -231,11 +260,13 @@ then # preventing the removal of the trash directory watchman shutdown-server >/dev/null 2>&1 fi +trace_stop # # Run a full set of perf tests with the fsmonitor feature disabled. # +trace_start fsmonitor-disabled test_expect_success "setup without fsmonitor" ' unset INTEGRATION_SCRIPT && git config --unset core.fsmonitor && @@ -243,5 +274,6 @@ test_expect_success "setup without fsmonitor" ' ' test_fsmonitor_suite +trace_stop test_done From 8c4b7503d033d8ba20771a1af8e077a79a18ce49 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:44 +0000 Subject: [PATCH 05/11] preload-index: log the number of lstat calls to trace2 Report the total number of calls made to lstat() inside preload_index(). FSMonitor improves the performance of commands like `git status` by avoiding scanning the disk for changed files. This can be seen in `preload_index()`. Let's measure this. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- preload-index.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/preload-index.c b/preload-index.c index ed6eaa47388af8..e5529a586366d4 100644 --- a/preload-index.c +++ b/preload-index.c @@ -31,6 +31,7 @@ struct thread_data { struct pathspec pathspec; struct progress_data *progress; int offset, nr; + int t2_nr_lstat; }; static void *preload_thread(void *_data) @@ -73,6 +74,7 @@ static void *preload_thread(void *_data) continue; if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce))) continue; + p->t2_nr_lstat++; if (lstat(ce->name, &st)) continue; if (ie_match_stat(index, ce, &st, CE_MATCH_RACY_IS_DIRTY|CE_MATCH_IGNORE_FSMONITOR)) @@ -98,6 +100,7 @@ void preload_index(struct index_state *index, int threads, i, work, offset; struct thread_data data[MAX_PARALLEL]; struct progress_data pd; + int t2_sum_lstat = 0; if (!HAVE_THREADS || !core_preload_index) return; @@ -107,6 +110,9 @@ void preload_index(struct index_state *index, threads = 2; if (threads < 2) return; + + trace2_region_enter("index", "preload", NULL); + trace_performance_enter(); if (threads > MAX_PARALLEL) threads = MAX_PARALLEL; @@ -141,10 +147,14 @@ void preload_index(struct index_state *index, struct thread_data *p = data+i; if (pthread_join(p->pthread, NULL)) die("unable to join threaded lstat"); + t2_sum_lstat += p->t2_nr_lstat; } stop_progress(&pd.progress); trace_performance_leave("preload index"); + + trace2_data_intmax("index", NULL, "preload/sum_lstat", t2_sum_lstat); + trace2_region_leave("index", "preload", NULL); } int repo_read_index_preload(struct repository *repo, From a98e0f2d317818bfcb7f3c9cf53dfad2334e8ca7 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:45 +0000 Subject: [PATCH 06/11] read-cache: log the number of lstat calls to trace2 Report the total number of calls made to lstat() inside of refresh_index(). FSMonitor improves the performance of commands like `git status` by avoiding scanning the disk for changed files. This can be seen in `refresh_index()`. Let's measure this. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- read-cache.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index ecf6f689940556..893cc41e1d93ee 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1364,7 +1364,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti static struct cache_entry *refresh_cache_ent(struct index_state *istate, struct cache_entry *ce, unsigned int options, int *err, - int *changed_ret) + int *changed_ret, + int *t2_did_lstat) { struct stat st; struct cache_entry *updated; @@ -1406,6 +1407,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, return NULL; } + if (t2_did_lstat) + *t2_did_lstat = 1; if (lstat(ce->name, &st) < 0) { if (ignore_missing && errno == ENOENT) return ce; @@ -1519,6 +1522,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char *added_fmt; const char *unmerged_fmt; struct progress *progress = NULL; + int t2_sum_lstat = 0; if (flags & REFRESH_PROGRESS && isatty(2)) progress = start_delayed_progress(_("Refresh index"), @@ -1536,11 +1540,13 @@ int refresh_index(struct index_state *istate, unsigned int flags, * we only have to do the special cases that are left. */ preload_index(istate, pathspec, 0); + trace2_region_enter("index", "refresh", NULL); for (i = 0; i < istate->cache_nr; i++) { struct cache_entry *ce, *new_entry; int cache_errno = 0; int changed = 0; int filtered = 0; + int t2_did_lstat = 0; ce = istate->cache[i]; if (ignore_submodules && S_ISGITLINK(ce->ce_mode)) @@ -1566,7 +1572,10 @@ int refresh_index(struct index_state *istate, unsigned int flags, if (filtered) continue; - new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed); + new_entry = refresh_cache_ent(istate, ce, options, + &cache_errno, &changed, + &t2_did_lstat); + t2_sum_lstat += t2_did_lstat; if (new_entry == ce) continue; if (progress) @@ -1602,6 +1611,8 @@ int refresh_index(struct index_state *istate, unsigned int flags, replace_index_entry(istate, i, new_entry); } + trace2_data_intmax("index", NULL, "refresh/sum_lstat", t2_sum_lstat); + trace2_region_leave("index", "refresh", NULL); if (progress) { display_progress(progress, istate->cache_nr); stop_progress(&progress); @@ -1614,7 +1625,7 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate, struct cache_entry *ce, unsigned int options) { - return refresh_cache_ent(istate, ce, options, NULL, NULL); + return refresh_cache_ent(istate, ce, options, NULL, NULL, NULL); } From 15268d12bef12a40753767882088f4fcfefb3b2b Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:46 +0000 Subject: [PATCH 07/11] read-cache: log the number of scanned files to trace2 Report the number of files in the working directory that were read and their hashes verified in `refresh_index()`. FSMonitor improves the performance of commands like `git status` by avoiding scanning the disk for changed files. Let's measure this. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- read-cache.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index 893cc41e1d93ee..c9dd7f4015ed9c 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1365,7 +1365,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, struct cache_entry *ce, unsigned int options, int *err, int *changed_ret, - int *t2_did_lstat) + int *t2_did_lstat, + int *t2_did_scan) { struct stat st; struct cache_entry *updated; @@ -1445,6 +1446,8 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate, } } + if (t2_did_scan) + *t2_did_scan = 1; if (ie_modified(istate, ce, &st, options)) { if (err) *err = EINVAL; @@ -1523,6 +1526,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char *unmerged_fmt; struct progress *progress = NULL; int t2_sum_lstat = 0; + int t2_sum_scan = 0; if (flags & REFRESH_PROGRESS && isatty(2)) progress = start_delayed_progress(_("Refresh index"), @@ -1547,6 +1551,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, int changed = 0; int filtered = 0; int t2_did_lstat = 0; + int t2_did_scan = 0; ce = istate->cache[i]; if (ignore_submodules && S_ISGITLINK(ce->ce_mode)) @@ -1574,8 +1579,9 @@ int refresh_index(struct index_state *istate, unsigned int flags, new_entry = refresh_cache_ent(istate, ce, options, &cache_errno, &changed, - &t2_did_lstat); + &t2_did_lstat, &t2_did_scan); t2_sum_lstat += t2_did_lstat; + t2_sum_scan += t2_did_scan; if (new_entry == ce) continue; if (progress) @@ -1612,6 +1618,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, replace_index_entry(istate, i, new_entry); } trace2_data_intmax("index", NULL, "refresh/sum_lstat", t2_sum_lstat); + trace2_data_intmax("index", NULL, "refresh/sum_scan", t2_sum_scan); trace2_region_leave("index", "refresh", NULL); if (progress) { display_progress(progress, istate->cache_nr); @@ -1625,7 +1632,7 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate, struct cache_entry *ce, unsigned int options) { - return refresh_cache_ent(istate, ce, options, NULL, NULL, NULL); + return refresh_cache_ent(istate, ce, options, NULL, NULL, NULL, NULL); } From 940b94f35cf1858adf23fe939bcbbe73147ca1f3 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:47 +0000 Subject: [PATCH 08/11] fsmonitor: log invocation of FSMonitor hook to trace2 Let's measure the time taken to request and receive FSMonitor data via the hook API and the size of the response. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- fsmonitor.c | 29 ++++++++++++++++++++++++++++- fsmonitor.h | 5 +++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/fsmonitor.c b/fsmonitor.c index ca031c3abb8bf5..7a2be24cd43313 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -142,6 +142,7 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate) static int query_fsmonitor(int version, const char *last_update, struct strbuf *query_result) { struct child_process cp = CHILD_PROCESS_INIT; + int result; if (!core_fsmonitor) return -1; @@ -152,7 +153,33 @@ static int query_fsmonitor(int version, const char *last_update, struct strbuf * cp.use_shell = 1; cp.dir = get_git_work_tree(); - return capture_command(&cp, query_result, 1024); + trace2_region_enter("fsm_hook", "query", NULL); + + result = capture_command(&cp, query_result, 1024); + + if (result) + trace2_data_intmax("fsm_hook", NULL, "query/failed", result); + else { + trace2_data_intmax("fsm_hook", NULL, "query/response-length", + query_result->len); + + if (fsmonitor_is_trivial_response(query_result)) + trace2_data_intmax("fsm_hook", NULL, + "query/trivial-response", 1); + } + + trace2_region_leave("fsm_hook", "query", NULL); + + return result; +} + +int fsmonitor_is_trivial_response(const struct strbuf *query_result) +{ + static char trivial_response[3] = { '\0', '/', '\0' }; + int is_trivial = !memcmp(trivial_response, + &query_result->buf[query_result->len - 3], 3); + + return is_trivial; } static void fsmonitor_refresh_callback(struct index_state *istate, const char *name) diff --git a/fsmonitor.h b/fsmonitor.h index 739318ab6d1060..7f1794b90b001c 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -44,6 +44,11 @@ void tweak_fsmonitor(struct index_state *istate); */ void refresh_fsmonitor(struct index_state *istate); +/* + * Does the received result contain the "trivial" response? + */ +int fsmonitor_is_trivial_response(const struct strbuf *query_result); + /* * Set the given cache entries CE_FSMONITOR_VALID bit. This should be * called any time the cache entry has been updated to reflect the From 29fbbf43a031cefb6fe6e3f723d78f82af4979b0 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:48 +0000 Subject: [PATCH 09/11] fsmonitor: log FSMN token when reading and writing the index Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- fsmonitor.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fsmonitor.c b/fsmonitor.c index 7a2be24cd43313..3105dc370ab6c2 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -87,7 +87,11 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data, BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)", (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr); - trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful"); + trace2_data_string("index", NULL, "extension/fsmn/read/token", + istate->fsmonitor_last_update); + trace_printf_key(&trace_fsmonitor, + "read fsmonitor extension successful '%s'", + istate->fsmonitor_last_update); return 0; } @@ -133,7 +137,11 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate) put_be32(&ewah_size, sb->len - ewah_start); memcpy(sb->buf + fixup, &ewah_size, sizeof(uint32_t)); - trace_printf_key(&trace_fsmonitor, "write fsmonitor extension successful"); + trace2_data_string("index", NULL, "extension/fsmn/write/token", + istate->fsmonitor_last_update); + trace_printf_key(&trace_fsmonitor, + "write fsmonitor extension successful '%s'", + istate->fsmonitor_last_update); } /* From ff03836b9d696617d699da2e4108057f9a48b9ef Mon Sep 17 00:00:00 2001 From: Kevin Willford Date: Wed, 3 Feb 2021 15:34:49 +0000 Subject: [PATCH 10/11] fsmonitor: allow all entries for a folder to be invalidated Allow fsmonitor to report directory changes by reporting paths with a trailing slash. Signed-off-by: Jeff Hostetler Signed-off-by: Kevin Willford Signed-off-by: Johannes Schindelin Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- fsmonitor.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/fsmonitor.c b/fsmonitor.c index 3105dc370ab6c2..64deeda597ead8 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -190,13 +190,34 @@ int fsmonitor_is_trivial_response(const struct strbuf *query_result) return is_trivial; } -static void fsmonitor_refresh_callback(struct index_state *istate, const char *name) +static void fsmonitor_refresh_callback(struct index_state *istate, char *name) { - int pos = index_name_pos(istate, name, strlen(name)); + int i, len = strlen(name); + if (name[len - 1] == '/') { + + /* + * TODO We should binary search to find the first path with + * TODO this directory prefix. Then linearly update entries + * TODO while the prefix matches. Taking care to search without + * TODO the trailing slash -- because '/' sorts after a few + * TODO interesting special chars, like '.' and ' '. + */ + + /* Mark all entries for the folder invalid */ + for (i = 0; i < istate->cache_nr; i++) { + if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID && + starts_with(istate->cache[i]->name, name)) + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; + } + /* Need to remove the / from the path for the untracked cache */ + name[len - 1] = '\0'; + } else { + int pos = index_name_pos(istate, name, strlen(name)); - if (pos >= 0) { - struct cache_entry *ce = istate->cache[pos]; - ce->ce_flags &= ~CE_FSMONITOR_VALID; + if (pos >= 0) { + struct cache_entry *ce = istate->cache[pos]; + ce->ce_flags &= ~CE_FSMONITOR_VALID; + } } /* From fcd19b09f8b2cddffe18dc4d2f974fce94dc27b0 Mon Sep 17 00:00:00 2001 From: Jeff Hostetler Date: Wed, 3 Feb 2021 15:34:50 +0000 Subject: [PATCH 11/11] fsmonitor: refactor initialization of fsmonitor_last_update token Isolate and document initialization of `istate->fsmonitor_last_update`. This field should contain a fsmonitor-specific opaque token, but we need to initialize it before we can actually talk to a fsmonitor process, so we create a generic default value. Signed-off-by: Jeff Hostetler Reviewed-by: Taylor Blau Signed-off-by: Junio C Hamano --- fsmonitor.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/fsmonitor.c b/fsmonitor.c index 64deeda597ead8..e12214b30071e7 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -343,16 +343,45 @@ void refresh_fsmonitor(struct index_state *istate) istate->fsmonitor_last_update = strbuf_detach(&last_update_token, NULL); } +/* + * The caller wants to turn on FSMonitor. And when the caller writes + * the index to disk, a FSMonitor extension should be included. This + * requires that `istate->fsmonitor_last_update` not be NULL. But we + * have not actually talked to a FSMonitor process yet, so we don't + * have an initial value for this field. + * + * For a protocol V1 FSMonitor process, this field is a formatted + * "nanoseconds since epoch" field. However, for a protocol V2 + * FSMonitor process, this field is an opaque token. + * + * Historically, `add_fsmonitor()` has initialized this field to the + * current time for protocol V1 processes. There are lots of race + * conditions here, but that code has shipped... + * + * The only true solution is to use a V2 FSMonitor and get a current + * or default token value (that it understands), but we cannot do that + * until we have actually talked to an instance of the FSMonitor process + * (but the protocol requires that we send a token first...). + * + * For simplicity, just initialize like we have a V1 process and require + * that V2 processes adapt. + */ +static void initialize_fsmonitor_last_update(struct index_state *istate) +{ + struct strbuf last_update = STRBUF_INIT; + + strbuf_addf(&last_update, "%"PRIu64"", getnanotime()); + istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL); +} + void add_fsmonitor(struct index_state *istate) { unsigned int i; - struct strbuf last_update = STRBUF_INIT; if (!istate->fsmonitor_last_update) { trace_printf_key(&trace_fsmonitor, "add fsmonitor"); istate->cache_changed |= FSMONITOR_CHANGED; - strbuf_addf(&last_update, "%"PRIu64"", getnanotime()); - istate->fsmonitor_last_update = strbuf_detach(&last_update, NULL); + initialize_fsmonitor_last_update(istate); /* reset the fsmonitor state */ for (i = 0; i < istate->cache_nr; i++)