From eb10e637cf04a19b9a3d5d7c904d71bc9e4ea408 Mon Sep 17 00:00:00 2001
From: Jeff Hostetler <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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 9b43342806..6677e0ef7a 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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 6677e0ef7a..21d525541d 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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 21d525541d..78e7d2c03f 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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 982eb8e3a9..72f5d0d314 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 fcb0e8865e..2465770a78 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 78e7d2c03f..aa0ea0e2ec 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
preload-index.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/preload-index.c b/preload-index.c
index ed6eaa4738..e5529a5863 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
read-cache.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index ecf6f68994..893cc41e1d 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
read-cache.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 893cc41e1d..c9dd7f4015 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fsmonitor.c | 29 ++++++++++++++++++++++++++++-
fsmonitor.h | 5 +++++
2 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index ca031c3abb..7a2be24cd4 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 739318ab6d..7f1794b90b 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 <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fsmonitor.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 7a2be24cd4..3105dc370a 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 <Kevin.Willford@microsoft.com>
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 <jeffhost@microsoft.com>
Signed-off-by: Kevin Willford <Kevin.Willford@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fsmonitor.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 3105dc370a..64deeda597 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] == '/') {
- if (pos >= 0) {
- struct cache_entry *ce = istate->cache[pos];
- ce->ce_flags &= ~CE_FSMONITOR_VALID;
+ /*
+ * 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;
+ }
}
/*
From fcd19b09f8b2cddffe18dc4d2f974fce94dc27b0 Mon Sep 17 00:00:00 2001
From: Jeff Hostetler <jeffhost@microsoft.com>
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 <jeffhost@microsoft.com>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
fsmonitor.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index 64deeda597..e12214b300 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++)