From 11cf33bec62512f2c3f0bdcfdaa41e31e9b0c3e8 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 27 Oct 2017 16:26:34 -0700 Subject: [PATCH 1/7] fsmonitor: set the PWD to the top of the working tree The fsmonitor command inherits the PWD of its caller, which may be anywhere in the working copy. This makes is difficult for the fsmonitor command to operate on the whole repository. Specifically, for the watchman integration, this causes each subdirectory to get its own watch entry. Set the CWD to the top of the working directory, for consistency. Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- fsmonitor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fsmonitor.c b/fsmonitor.c index 7c1540c054..4ea44dcc6a 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que argv[3] = NULL; cp.argv = argv; cp.use_shell = 1; + cp.dir = get_git_work_tree(); return capture_command(&cp, query_result, 1024); } From c87fbcf7614a064877cd0323f0ed31840fed9c0c Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 27 Oct 2017 16:26:35 -0700 Subject: [PATCH 2/7] fsmonitor: don't bother pretty-printing JSON from watchman This provides modest performance savings. Benchmarking with the following program, with and without `--no-pretty`, we find savings of 23% (0.316s -> 0.242s) in the git repository, and savings of 8% (5.24s -> 4.86s) on a large repository with 580k files in the working copy. #!/usr/bin/perl use strict; use warnings; use IPC::Open2; use JSON::XS; my $pid = open2(\*CHLD_OUT, \*CHLD_IN, "watchman -j @ARGV") or die "open2() failed: $!\n" . "Falling back to scanning...\n"; my $query = qq|["query", "$ENV{PWD}", {}]|; print CHLD_IN $query; close CHLD_IN; my $response = do {local $/; }; JSON::XS->new->utf8->decode($response); Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- templates/hooks--fsmonitor-watchman.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample index 9eba8a7409..9a082f2781 100755 --- a/templates/hooks--fsmonitor-watchman.sample +++ b/templates/hooks--fsmonitor-watchman.sample @@ -49,7 +49,7 @@ launch_watchman(); sub launch_watchman { - my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j') + my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty') or die "open2() failed: $!\n" . "Falling back to scanning...\n"; From bd76afd13dcee4a07ba5136704ac8592af26d332 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 27 Oct 2017 16:26:36 -0700 Subject: [PATCH 3/7] fsmonitor: document GIT_TRACE_FSMONITOR Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- Documentation/git.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/git.txt b/Documentation/git.txt index 6e3a6767e5..ba31d95a82 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -591,6 +591,10 @@ into it. Unsetting the variable, or setting it to empty, "0" or "false" (case insensitive) disables trace messages. +`GIT_TRACE_FSMONITOR`:: + Enables trace messages for the filesystem monitor extension. + See `GIT_TRACE` for available trace output options. + `GIT_TRACE_PACK_ACCESS`:: Enables trace messages for all accesses to any packs. For each access, the pack file name and an offset in the pack is From ba1b9caca69909b3c048bda1bbfab4fefb070bff Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 27 Oct 2017 16:26:37 -0700 Subject: [PATCH 4/7] fsmonitor: delay updating state until after split index is merged If the fsmonitor extension is used in conjunction with the split index extension, the set of entries in the index when it is first loaded is only a subset of the real index. This leads to only the non-"base" index being marked as CE_FSMONITOR_VALID. Delay the expansion of the ewah bitmap until after tweak_split_index has been called to merge in the base index as well. The new fsmonitor_dirty is kept from being leaked by dint of being cleaned up in post_read_index_from, which is guaranteed to be called after do_read_index in read_index_from. Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- cache.h | 1 + fsmonitor.c | 40 ++++++++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/cache.h b/cache.h index f1c903e1b6..a15edc7e1d 100644 --- a/cache.h +++ b/cache.h @@ -347,6 +347,7 @@ struct index_state { unsigned char sha1[20]; struct untracked_cache *untracked; uint64_t fsmonitor_last_update; + struct ewah_bitmap *fsmonitor_dirty; }; extern struct index_state the_index; diff --git a/fsmonitor.c b/fsmonitor.c index 4ea44dcc6a..f494a866d5 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -26,7 +26,6 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data, uint32_t hdr_version; uint32_t ewah_size; struct ewah_bitmap *fsmonitor_dirty; - int i; int ret; if (sz < sizeof(uint32_t) + sizeof(uint64_t) + sizeof(uint32_t)) @@ -49,20 +48,7 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data, ewah_free(fsmonitor_dirty); return error("failed to parse ewah bitmap reading fsmonitor index extension"); } - - if (git_config_get_fsmonitor()) { - /* Mark all entries valid */ - for (i = 0; i < istate->cache_nr; i++) - istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID; - - /* Mark all previously saved entries as dirty */ - ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate); - - /* Now mark the untracked cache for fsmonitor usage */ - if (istate->untracked) - istate->untracked->use_fsmonitor = 1; - } - ewah_free(fsmonitor_dirty); + istate->fsmonitor_dirty = fsmonitor_dirty; trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful"); return 0; @@ -239,7 +225,29 @@ void remove_fsmonitor(struct index_state *istate) void tweak_fsmonitor(struct index_state *istate) { - switch (git_config_get_fsmonitor()) { + int i; + int fsmonitor_enabled = git_config_get_fsmonitor(); + + if (istate->fsmonitor_dirty) { + if (fsmonitor_enabled) { + /* Mark all entries valid */ + for (i = 0; i < istate->cache_nr; i++) { + istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID; + } + + /* Mark all previously saved entries as dirty */ + ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate); + + /* Now mark the untracked cache for fsmonitor usage */ + if (istate->untracked) + istate->untracked->use_fsmonitor = 1; + } + + ewah_free(istate->fsmonitor_dirty); + istate->fsmonitor_dirty = NULL; + } + + switch (fsmonitor_enabled) { case -1: /* keep: do nothing */ break; case 0: /* false */ From 6f1dc21d983a495877c8054d41d9b634e234ccc4 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 9 Nov 2017 11:58:09 -0800 Subject: [PATCH 5/7] fsmonitor: read from getcwd(), not the PWD environment variable Though the process has chdir'd to the root of the working tree, the PWD environment variable is only guaranteed to be updated accordingly if a shell is involved -- which is not guaranteed to be the case. That is, if `/usr/bin/perl` is a binary, $ENV{PWD} is unchanged from whatever spawned `git` -- if `/usr/bin/perl` is a trivial shell wrapper to the real `perl`, `$ENV{PWD}` will have been updated to the root of the working copy. Update to read from the Cwd module using the `getcwd` syscall, not the PWD environment variable. The Cygwin case is left unchanged, as it necessarily _does_ go through a shell. Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- t/t7519/fsmonitor-watchman | 3 ++- templates/hooks--fsmonitor-watchman.sample | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman index a3e30bf54f..5fe72cefaf 100755 --- a/t/t7519/fsmonitor-watchman +++ b/t/t7519/fsmonitor-watchman @@ -41,7 +41,8 @@ if ($system =~ m/^MSYS_NT/ || $system =~ m/^MINGW/) { $git_work_tree =~ s/[\r\n]+//g; $git_work_tree =~ s,\\,/,g; } else { - $git_work_tree = $ENV{'PWD'}; + require Cwd; + $git_work_tree = Cwd::cwd(); } my $retry = 1; diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample index 9a082f2781..ba6d88c5f8 100755 --- a/templates/hooks--fsmonitor-watchman.sample +++ b/templates/hooks--fsmonitor-watchman.sample @@ -40,7 +40,8 @@ if ($system =~ m/^MSYS_NT/ || $system =~ m/^MINGW/) { $git_work_tree =~ s/[\r\n]+//g; $git_work_tree =~ s,\\,/,g; } else { - $git_work_tree = $ENV{'PWD'}; + require Cwd; + $git_work_tree = Cwd::cwd(); } my $retry = 1; From 3bd28eb29912801481b293691039b05caebf13a3 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 9 Nov 2017 11:58:10 -0800 Subject: [PATCH 6/7] fsmonitor: store fsmonitor bitmap before splitting index ba1b9cac ("fsmonitor: delay updating state until after split index is merged", 2017-10-27) resolved the problem of the fsmonitor data being applied to the non-base index when reading; however, a similar problem exists when writing the index. Specifically, writing of the fsmonitor extension happens only after the work to split the index has been applied -- as such, the information in the index is only for the non-"base" index, and thus the extension information contains only partial data. When saving, compute the ewah bitmap before the index is split, and store it in the fsmonitor_dirty field, mirroring the behavior that occurred during reading. fsmonitor_dirty is kept from being leaked by being freed when the extension data is written -- which always happens precisely once, no matter the split index configuration. Signed-off-by: Alex Vandiver Signed-off-by: Junio C Hamano --- fsmonitor.c | 20 ++++++++++++-------- fsmonitor.h | 9 ++++++++- read-cache.c | 3 +++ t/t7519-status-fsmonitor.sh | 13 +++++++++++++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/fsmonitor.c b/fsmonitor.c index f494a866d5..0af7c4edba 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -54,12 +54,19 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data, return 0; } +void fill_fsmonitor_bitmap(struct index_state *istate) +{ + int i; + istate->fsmonitor_dirty = ewah_new(); + for (i = 0; i < istate->cache_nr; i++) + if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID)) + ewah_set(istate->fsmonitor_dirty, i); +} + void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate) { uint32_t hdr_version; uint64_t tm; - struct ewah_bitmap *bitmap; - int i; uint32_t ewah_start; uint32_t ewah_size = 0; int fixup = 0; @@ -73,12 +80,9 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate) strbuf_add(sb, &ewah_size, sizeof(uint32_t)); /* we'll fix this up later */ ewah_start = sb->len; - bitmap = ewah_new(); - for (i = 0; i < istate->cache_nr; i++) - if (!(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID)) - ewah_set(bitmap, i); - ewah_serialize_strbuf(bitmap, sb); - ewah_free(bitmap); + ewah_serialize_strbuf(istate->fsmonitor_dirty, sb); + ewah_free(istate->fsmonitor_dirty); + istate->fsmonitor_dirty = NULL; /* fix up size field */ put_be32(&ewah_size, sb->len - ewah_start); diff --git a/fsmonitor.h b/fsmonitor.h index 0de644e01a..cd3cc0ccf2 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -10,7 +10,14 @@ extern struct trace_key trace_fsmonitor; extern int read_fsmonitor_extension(struct index_state *istate, const void *data, unsigned long sz); /* - * Write the CE_FSMONITOR_VALID state into the fsmonitor index extension. + * Fill the fsmonitor_dirty ewah bits with their state from the index, + * before it is split during writing. + */ +extern void fill_fsmonitor_bitmap(struct index_state *istate); + +/* + * Write the CE_FSMONITOR_VALID state into the fsmonitor index + * extension. Reads from the fsmonitor_dirty ewah in the index. */ extern void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate); diff --git a/read-cache.c b/read-cache.c index 05c0a33fdd..0724ea8c9b 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2525,6 +2525,9 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock, int new_shared_index, ret; struct split_index *si = istate->split_index; + if (istate->fsmonitor_last_update) + fill_fsmonitor_bitmap(istate); + if (!si || alternate_index_output || (istate->cache_changed & ~EXTMASK)) { if (si) diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index c6df85af5e..eb2d13bbcf 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -301,4 +301,17 @@ do done done +# test that splitting the index dosn't interfere +test_expect_success 'splitting the index results in the same state' ' + write_integration_script && + dirty_repo && + git update-index --fsmonitor && + git ls-files -f >expect && + test-dump-fsmonitor >&2 && echo && + git update-index --fsmonitor --split-index && + test-dump-fsmonitor >&2 && echo && + git ls-files -f >actual && + test_cmp expect actual +' + test_done From 1fff303fc2b31d5005f38f55f38c4e8521da5a93 Mon Sep 17 00:00:00 2001 From: Ben Peart Date: Fri, 10 Nov 2017 16:03:11 -0500 Subject: [PATCH 7/7] fsmonitor: simplify determining the git worktree under Windows Simplify and speed up the process of finding the git worktree when running on Windows by keeping it in perl and avoiding spawning helper processes. Signed-off-by: Ben Peart Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t7519/fsmonitor-watchman | 13 +++---------- templates/hooks--fsmonitor-watchman.sample | 13 +++---------- 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman index 5fe72cefaf..5514edcf68 100755 --- a/t/t7519/fsmonitor-watchman +++ b/t/t7519/fsmonitor-watchman @@ -29,17 +29,10 @@ if ($version == 1) { "Falling back to scanning...\n"; } -# Convert unix style paths to escaped Windows style paths when running -# in Windows command prompt - -my $system = `uname -s`; -$system =~ s/[\r\n]+//g; my $git_work_tree; - -if ($system =~ m/^MSYS_NT/ || $system =~ m/^MINGW/) { - $git_work_tree = `cygpath -aw "\$PWD"`; - $git_work_tree =~ s/[\r\n]+//g; - $git_work_tree =~ s,\\,/,g; +if ($^O =~ 'msys' || $^O =~ 'cygwin') { + $git_work_tree = Win32::GetCwd(); + $git_work_tree =~ tr/\\/\//; } else { require Cwd; $git_work_tree = Cwd::cwd(); diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample index ba6d88c5f8..e673bb3980 100755 --- a/templates/hooks--fsmonitor-watchman.sample +++ b/templates/hooks--fsmonitor-watchman.sample @@ -28,17 +28,10 @@ if ($version == 1) { "Falling back to scanning...\n"; } -# Convert unix style paths to escaped Windows style paths when running -# in Windows command prompt - -my $system = `uname -s`; -$system =~ s/[\r\n]+//g; my $git_work_tree; - -if ($system =~ m/^MSYS_NT/ || $system =~ m/^MINGW/) { - $git_work_tree = `cygpath -aw "\$PWD"`; - $git_work_tree =~ s/[\r\n]+//g; - $git_work_tree =~ s,\\,/,g; +if ($^O =~ 'msys' || $^O =~ 'cygwin') { + $git_work_tree = Win32::GetCwd(); + $git_work_tree =~ tr/\\/\//; } else { require Cwd; $git_work_tree = Cwd::cwd();