{"thread":{"id":"47030","subject":"[PATCH v2 0/4] fsmonitor fixes","startedAt":"2017-10-26T01:31:35Z","lastAt":"2017-10-29T12:31:51Z","messageCount":17,"participants":["Alex Vandiver","Ben Peart","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"331052","messageId":"20171026013117.30034-1-alexmv@dropbox.com","threadId":"47030","inReplyTo":null,"subject":"[PATCH v2 0/4] fsmonitor fixes","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:31:13Z","receivedAt":"2017-10-26T01:31:35Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"Updated based on comments from Dscho and Ben.  Thanks for those!\n - Alex\n\n"},{"id":"331053","messageId":"9af6d28ef43edbc99a9b7a9c41990de0989bfc76.1508981451.git.alexmv@dropbox.com","threadId":"47030","inReplyTo":"20171026013117.30034-1-alexmv@dropbox.com","subject":"[PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:31:14Z","receivedAt":"2017-10-26T01:31:37Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"The fsmonitor command inherits the PWD of its caller, which may be\nanywhere in the working copy.  This makes is difficult for the\nfsmonitor command to operate on the whole repository.  Specifically,\nfor the watchman integration, this causes each subdirectory to get its\nown watch entry.\n\nSet the CWD to the top of the working directory, for consistency.\n\nSigned-off-by: Alex Vandiver <alexmv@dropbox.com>\n---\n fsmonitor.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 7c1540c05..0d26ff34f 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que\n \targv[3] = NULL;\n \tcp.argv = argv;\n \tcp.use_shell = 1;\n+        cp.dir = get_git_work_tree();\n \n \treturn capture_command(&cp, query_result, 1024);\n }\n-- \n2.15.0.rc1.413.g76aedb451\n\n"},{"id":"331054","messageId":"42fd8bccb78992a2894e711e057230a673891628.1508981451.git.alexmv@dropbox.com","threadId":"47030","inReplyTo":"20171026013117.30034-1-alexmv@dropbox.com","subject":"[PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:31:15Z","receivedAt":"2017-10-26T01:31:41Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"This provides modest performance savings.  Benchmarking with the\nfollowing program, with and without `--no-pretty`, we find savings of\n23% (0.316s -> 0.242s) in the git repository, and savings of 8% (5.24s\n-> 4.86s) on a large repository with 580k files in the working copy.\n\n    #!/usr/bin/perl\n\n    use strict;\n    use warnings;\n    use IPC::Open2;\n\n    my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, \"watchman -j @ARGV\")\n        or die \"open2() failed: $!\\n\" .\n        \"Falling back to scanning...\\n\";\n\n    my $query = qq|[\"query\", \"$ENV{PWD}\", {}]|;\n\n    print CHLD_IN $query;\n    close CHLD_IN;\n    my $response = do {local $/; <CHLD_OUT>};\n\n    my $json_pkg;\n    eval {\n        require JSON::XSomething;\n        $json_pkg = \"JSON::XSomething\";\n        1;\n    } or do {\n        require JSON::PP;\n        $json_pkg = \"JSON::PP\";\n    };\n\n    my $o = $json_pkg->new->utf8->decode($response);\n\nSigned-off-by: Alex Vandiver <alexmv@dropbox.com>\n---\n t/t7519/fsmonitor-watchman                 | 2 +-\n templates/hooks--fsmonitor-watchman.sample | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman\nindex a3e30bf54..79f24325c 100755\n--- a/t/t7519/fsmonitor-watchman\n+++ b/t/t7519/fsmonitor-watchman\n@@ -50,7 +50,7 @@ launch_watchman();\n \n sub launch_watchman {\n \n-\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n+\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n \t    or die \"open2() failed: $!\\n\" .\n \t    \"Falling back to scanning...\\n\";\n \ndiff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample\nindex 9eba8a740..9a082f278 100755\n--- a/templates/hooks--fsmonitor-watchman.sample\n+++ b/templates/hooks--fsmonitor-watchman.sample\n@@ -49,7 +49,7 @@ launch_watchman();\n \n sub launch_watchman {\n \n-\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n+\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n \t    or die \"open2() failed: $!\\n\" .\n \t    \"Falling back to scanning...\\n\";\n \n-- \n2.15.0.rc1.413.g76aedb451\n\n"},{"id":"331055","messageId":"7b71969808cc83c677b7b349920fc5cffd5237fd.1508981451.git.alexmv@dropbox.com","threadId":"47030","inReplyTo":"20171026013117.30034-1-alexmv@dropbox.com","subject":"[PATCH v2 3/4] fsmonitor: Document GIT_TRACE_FSMONITOR","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:31:16Z","receivedAt":"2017-10-26T01:31:42Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n---\n Documentation/git.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 1fca63634..720db196e 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -594,6 +594,10 @@ into it.\n Unsetting the variable, or setting it to empty, \"0\" or\n \"false\" (case insensitive) disables trace messages.\n \n+`GIT_TRACE_FSMONITOR`::\n+\tEnables trace messages for the filesystem monitor extension.\n+\tSee `GIT_TRACE` for available trace output options.\n+\n `GIT_TRACE_PACK_ACCESS`::\n \tEnables trace messages for all accesses to any packs. For each\n \taccess, the pack file name and an offset in the pack is\n-- \n2.15.0.rc1.413.g76aedb451\n\n"},{"id":"331056","messageId":"38fa281fb175c086a5b1f1d3747597a9924e9f46.1508981451.git.alexmv@dropbox.com","threadId":"47030","inReplyTo":"20171026013117.30034-1-alexmv@dropbox.com","subject":"[PATCH v2 4/4] fsmonitor: Delay updating state until after split index is merged","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:31:17Z","receivedAt":"2017-10-26T01:31:44Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"If the fsmonitor extension is used in conjunction with the split index\nextension, the set of entries in the index when it is first loaded is\nonly a subset of the real index.  This leads to only the non-\"base\"\nindex being marked as CE_FSMONITOR_VALID.\n\nDelay the expansion of the ewah bitmap until after tweak_split_index\nhas been called to merge in the base index as well.\n\nThe new fsmonitor_dirty is kept from being leaked by dint of being\ncleaned up in post_read_index_from, which is guaranteed to be called\nafter do_read_index in read_index_from.\n\nSigned-off-by: Alex Vandiver <alexmv@dropbox.com>\n---\n cache.h     |  1 +\n fsmonitor.c | 39 ++++++++++++++++++++++++---------------\n 2 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 25adcf681..0a4f43ec2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -348,6 +348,7 @@ struct index_state {\n \tunsigned char sha1[20];\n \tstruct untracked_cache *untracked;\n \tuint64_t fsmonitor_last_update;\n+\tstruct ewah_bitmap *fsmonitor_dirty;\n };\n \n extern struct index_state the_index;\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 0d26ff34f..fad9c6b13 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,\n \t\tewah_free(fsmonitor_dirty);\n \t\treturn error(\"failed to parse ewah bitmap reading fsmonitor index extension\");\n \t}\n-\n-\tif (git_config_get_fsmonitor()) {\n-\t\t/* Mark all entries valid */\n-\t\tfor (i = 0; i < istate->cache_nr; i++)\n-\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;\n-\n-\t\t/* Mark all previously saved entries as dirty */\n-\t\tewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);\n-\n-\t\t/* Now mark the untracked cache for fsmonitor usage */\n-\t\tif (istate->untracked)\n-\t\t\tistate->untracked->use_fsmonitor = 1;\n-\t}\n-\tewah_free(fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = fsmonitor_dirty;\n \n \ttrace_printf_key(&trace_fsmonitor, \"read fsmonitor extension successful\");\n \treturn 0;\n@@ -239,7 +226,29 @@ void remove_fsmonitor(struct index_state *istate)\n \n void tweak_fsmonitor(struct index_state *istate)\n {\n-\tswitch (git_config_get_fsmonitor()) {\n+\tint i;\n+\tint fsmonitor_enabled = git_config_get_fsmonitor();\n+\n+\tif (istate->fsmonitor_dirty) {\n+\t\tif (fsmonitor_enabled) {\n+\t\t\t/* Mark all entries valid */\n+\t\t\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;\n+\t\t\t}\n+\n+\t\t\t/* Mark all previously saved entries as dirty */\n+\t\t\tewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate);\n+\n+\t\t\t/* Now mark the untracked cache for fsmonitor usage */\n+\t\t\tif (istate->untracked)\n+\t\t\t\tistate->untracked->use_fsmonitor = 1;\n+\t\t}\n+\n+\t\tewah_free(istate->fsmonitor_dirty);\n+\t\tistate->fsmonitor_dirty = NULL;\n+\t}\n+\n+\tswitch (fsmonitor_enabled) {\n \tcase -1: /* keep: do nothing */\n \t\tbreak;\n \tcase 0: /* false */\n-- \n2.15.0.rc1.413.g76aedb451\n\n"},{"id":"331057","messageId":"alpine.DEB.2.10.1710251832180.9817@alexmv-linux","threadId":"47030","inReplyTo":"9af6d28ef43edbc99a9b7a9c41990de0989bfc76.1508981451.git.alexmv@dropbox.com","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T01:33:37Z","receivedAt":"2017-10-26T01:33:51Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"\n\nOn Wed, 25 Oct 2017, Alex Vandiver wrote:\n> The fsmonitor command inherits the PWD of its caller, which may be\n> anywhere in the working copy.  This makes is difficult for the\n> fsmonitor command to operate on the whole repository.  Specifically,\n> for the watchman integration, this causes each subdirectory to get its\n> own watch entry.\n> \n> Set the CWD to the top of the working directory, for consistency.\n> \n> Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n> ---\n>  fsmonitor.c | 1 +\n>  1 file changed, 1 insertion(+)\n> \n> diff --git a/fsmonitor.c b/fsmonitor.c\n> index 7c1540c05..0d26ff34f 100644\n> --- a/fsmonitor.c\n> +++ b/fsmonitor.c\n> @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que\n>  \targv[3] = NULL;\n>  \tcp.argv = argv;\n>  \tcp.use_shell = 1;\n> +        cp.dir = get_git_work_tree();\n\nLooks like my editor swapped out a tab on me.  I'll hold off on\nsending a revised version to collect any other comments.\n - Alex\n"},{"id":"331109","messageId":"5a389621-961d-48e3-339a-ef182d3ed628@gmail.com","threadId":"47030","inReplyTo":"9af6d28ef43edbc99a9b7a9c41990de0989bfc76.1508981451.git.alexmv@dropbox.com","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-26T19:56:24Z","receivedAt":"2017-10-26T19:56:33Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> The fsmonitor command inherits the PWD of its caller, which may be\n> anywhere in the working copy.  This makes is difficult for the\n> fsmonitor command to operate on the whole repository.  Specifically,\n> for the watchman integration, this causes each subdirectory to get its\n> own watch entry.\n> \n> Set the CWD to the top of the working directory, for consistency.\n> \n> Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n> ---\n>   fsmonitor.c | 1 +\n>   1 file changed, 1 insertion(+)\n> \n> diff --git a/fsmonitor.c b/fsmonitor.c\n> index 7c1540c05..0d26ff34f 100644\n> --- a/fsmonitor.c\n> +++ b/fsmonitor.c\n> @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que\n>   \targv[3] = NULL;\n>   \tcp.argv = argv;\n>   \tcp.use_shell = 1;\n> +        cp.dir = get_git_work_tree();\n>   \n\nI'm not sure what would trigger this problem but I don't doubt that it \nis possible.  Better to be safe than sorry. This is a better/faster \nsolution than you're previous patch.  Thank you!\n\n>   \treturn capture_command(&cp, query_result, 1024);\n>   }\n> \n"},{"id":"331111","messageId":"9bd24b15-6232-1afd-abbe-02870c51c7ad@gmail.com","threadId":"47030","inReplyTo":"42fd8bccb78992a2894e711e057230a673891628.1508981451.git.alexmv@dropbox.com","subject":"Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-26T20:05:13Z","receivedAt":"2017-10-26T20:05:19Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> This provides modest performance savings.  Benchmarking with the\n> following program, with and without `--no-pretty`, we find savings of\n> 23% (0.316s -> 0.242s) in the git repository, and savings of 8% (5.24s\n> -> 4.86s) on a large repository with 580k files in the working copy.\n> \n\nGiven this patch series is all about speed, it's good to see *any* wins \n- especially those that don't impact functionality at all.  The \nperformance win of --no-pretty is greater than I expected.\n\n>      #!/usr/bin/perl\n> \n>      use strict;\n>      use warnings;\n>      use IPC::Open2;\n> \n>      my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, \"watchman -j @ARGV\")\n>          or die \"open2() failed: $!\\n\" .\n>          \"Falling back to scanning...\\n\";\n> \n>      my $query = qq|[\"query\", \"$ENV{PWD}\", {}]|;\n> \n>      print CHLD_IN $query;\n>      close CHLD_IN;\n>      my $response = do {local $/; <CHLD_OUT>};\n> \n>      my $json_pkg;\n>      eval {\n>          require JSON::XSomething;\n>          $json_pkg = \"JSON::XSomething\";\n>          1;\n>      } or do {\n>          require JSON::PP;\n>          $json_pkg = \"JSON::PP\";\n>      };\n> \n>      my $o = $json_pkg->new->utf8->decode($response);\n> \n> Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n> ---\n>   t/t7519/fsmonitor-watchman                 | 2 +-\n>   templates/hooks--fsmonitor-watchman.sample | 2 +-\n>   2 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman\n> index a3e30bf54..79f24325c 100755\n> --- a/t/t7519/fsmonitor-watchman\n> +++ b/t/t7519/fsmonitor-watchman\n> @@ -50,7 +50,7 @@ launch_watchman();\n>   \n>   sub launch_watchman {\n>   \n> -\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n> +\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n\nSince this is a test script performance isn't critical.  This version of \nthe integration script logs the response to a file in \n.git/watchman-response.json and is much more human readable without the \n\"--no-pretty.\"  As such, I'd leave this one pretty.\n\n>   \t    or die \"open2() failed: $!\\n\" .\n>   \t    \"Falling back to scanning...\\n\";\n>   \n> diff --git a/templates/hooks--fsmonitor-watchman.sample b/templates/hooks--fsmonitor-watchman.sample\n> index 9eba8a740..9a082f278 100755\n> --- a/templates/hooks--fsmonitor-watchman.sample\n> +++ b/templates/hooks--fsmonitor-watchman.sample\n> @@ -49,7 +49,7 @@ launch_watchman();\n>   \n>   sub launch_watchman {\n>   \n> -\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n> +\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n\nNo human will see this response so the faster --no-pretty option makes \nsense.\n\n>   \t    or die \"open2() failed: $!\\n\" .\n>   \t    \"Falling back to scanning...\\n\";\n>   \n> \n"},{"id":"331113","messageId":"0c2c567e-e978-dc59-9bf5-a41f31405139@gmail.com","threadId":"47030","inReplyTo":"38fa281fb175c086a5b1f1d3747597a9924e9f46.1508981451.git.alexmv@dropbox.com","subject":"Re: [PATCH v2 4/4] fsmonitor: Delay updating state until after split index is merged","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-26T20:29:22Z","receivedAt":"2017-10-26T20:29:29Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> If the fsmonitor extension is used in conjunction with the split index\n> extension, the set of entries in the index when it is first loaded is\n> only a subset of the real index.  This leads to only the non-\"base\"\n> index being marked as CE_FSMONITOR_VALID.\n> \n> Delay the expansion of the ewah bitmap until after tweak_split_index\n> has been called to merge in the base index as well.\n> \n> The new fsmonitor_dirty is kept from being leaked by dint of being\n> cleaned up in post_read_index_from, which is guaranteed to be called\n> after do_read_index in read_index_from.\n> \n> Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n> ---\n>   cache.h     |  1 +\n>   fsmonitor.c | 39 ++++++++++++++++++++++++---------------\n>   2 files changed, 25 insertions(+), 15 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index 25adcf681..0a4f43ec2 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -348,6 +348,7 @@ struct index_state {\n>   \tunsigned char sha1[20];\n>   \tstruct untracked_cache *untracked;\n>   \tuint64_t fsmonitor_last_update;\n> +\tstruct ewah_bitmap *fsmonitor_dirty;\n>   };\n>   \n>   extern struct index_state the_index;\n> diff --git a/fsmonitor.c b/fsmonitor.c\n> index 0d26ff34f..fad9c6b13 100644\n> --- a/fsmonitor.c\n> +++ b/fsmonitor.c\n> @@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,\n>   \t\tewah_free(fsmonitor_dirty);\n>   \t\treturn error(\"failed to parse ewah bitmap reading fsmonitor index extension\");\n>   \t}\n> -\n> -\tif (git_config_get_fsmonitor()) {\n> -\t\t/* Mark all entries valid */\n> -\t\tfor (i = 0; i < istate->cache_nr; i++)\n> -\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;\n> -\n> -\t\t/* Mark all previously saved entries as dirty */\n> -\t\tewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);\n> -\n> -\t\t/* Now mark the untracked cache for fsmonitor usage */\n> -\t\tif (istate->untracked)\n> -\t\t\tistate->untracked->use_fsmonitor = 1;\n> -\t}\n> -\tewah_free(fsmonitor_dirty);\n> +\tistate->fsmonitor_dirty = fsmonitor_dirty;\n>   \n>   \ttrace_printf_key(&trace_fsmonitor, \"read fsmonitor extension successful\");\n>   \treturn 0;\n> @@ -239,7 +226,29 @@ void remove_fsmonitor(struct index_state *istate)\n>   \n>   void tweak_fsmonitor(struct index_state *istate)\n>   {\n> -\tswitch (git_config_get_fsmonitor()) {\n> +\tint i;\n> +\tint fsmonitor_enabled = git_config_get_fsmonitor();\n> +\n\nThe logic looks good this time.  It is nice to know this will now be \noptimal when split index is also turned on.  Thank you.\n\n> +\tif (istate->fsmonitor_dirty) {\n> +\t\tif (fsmonitor_enabled) {\n> +\t\t\t/* Mark all entries valid */\n> +\t\t\tfor (i = 0; i < istate->cache_nr; i++) {\n> +\t\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;\n> +\t\t\t}\n> +\n> +\t\t\t/* Mark all previously saved entries as dirty */\n> +\t\t\tewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate);\n> +\n> +\t\t\t/* Now mark the untracked cache for fsmonitor usage */\n> +\t\t\tif (istate->untracked)\n> +\t\t\t\tistate->untracked->use_fsmonitor = 1;\n> +\t\t}\n> +\n> +\t\tewah_free(istate->fsmonitor_dirty);\n> +\t\tistate->fsmonitor_dirty = NULL;\n> +\t}\n> +\n> +\tswitch (fsmonitor_enabled) {\n>   \tcase -1: /* keep: do nothing */\n>   \t\tbreak;\n>   \tcase 0: /* false */\n> \n"},{"id":"331115","messageId":"e7820a80-eb47-2241-b36a-62a70bbceb5a@gmail.com","threadId":"47030","inReplyTo":"9bd24b15-6232-1afd-abbe-02870c51c7ad@gmail.com","subject":"Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-26T20:32:10Z","receivedAt":"2017-10-26T20:32:16Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/26/2017 4:05 PM, Ben Peart wrote:\n> \n> \n> On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n>> This provides modest performance savings.  Benchmarking with the\n>> following program, with and without `--no-pretty`, we find savings of\n>> 23% (0.316s -> 0.242s) in the git repository, and savings of 8% (5.24s\n>> -> 4.86s) on a large repository with 580k files in the working copy.\n>>\n> \n> Given this patch series is all about speed, it's good to see *any* wins \n> - especially those that don't impact functionality at all.  The \n> performance win of --no-pretty is greater than I expected.\n> \n>>      #!/usr/bin/perl\n>>\n>>      use strict;\n>>      use warnings;\n>>      use IPC::Open2;\n>>\n>>      my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, \"watchman -j @ARGV\")\n>>          or die \"open2() failed: $!\\n\" .\n>>          \"Falling back to scanning...\\n\";\n>>\n>>      my $query = qq|[\"query\", \"$ENV{PWD}\", {}]|;\n>>\n>>      print CHLD_IN $query;\n>>      close CHLD_IN;\n>>      my $response = do {local $/; <CHLD_OUT>};\n>>\n>>      my $json_pkg;\n>>      eval {\n>>          require JSON::XSomething;\n>>          $json_pkg = \"JSON::XSomething\";\n>>          1;\n>>      } or do {\n>>          require JSON::PP;\n>>          $json_pkg = \"JSON::PP\";\n>>      };\n>>\n>>      my $o = $json_pkg->new->utf8->decode($response);\n>>\n>> Signed-off-by: Alex Vandiver <alexmv@dropbox.com>\n>> ---\n>>   t/t7519/fsmonitor-watchman                 | 2 +-\n>>   templates/hooks--fsmonitor-watchman.sample | 2 +-\n>>   2 files changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman\n>> index a3e30bf54..79f24325c 100755\n>> --- a/t/t7519/fsmonitor-watchman\n>> +++ b/t/t7519/fsmonitor-watchman\n>> @@ -50,7 +50,7 @@ launch_watchman();\n>>   sub launch_watchman {\n>> -    my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n>> +    my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n> \n> Since this is a test script performance isn't critical.  This version of \n> the integration script logs the response to a file in \n> .git/watchman-response.json and is much more human readable without the \n> \"--no-pretty.\"  As such, I'd leave this one pretty.\n\nI didn't see anything (including this) worth another roll so only \naddress it if something else comes up.\n\n> \n>>           or die \"open2() failed: $!\\n\" .\n>>           \"Falling back to scanning...\\n\";\n>> diff --git a/templates/hooks--fsmonitor-watchman.sample \n>> b/templates/hooks--fsmonitor-watchman.sample\n>> index 9eba8a740..9a082f278 100755\n>> --- a/templates/hooks--fsmonitor-watchman.sample\n>> +++ b/templates/hooks--fsmonitor-watchman.sample\n>> @@ -49,7 +49,7 @@ launch_watchman();\n>>   sub launch_watchman {\n>> -    my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n>> +    my $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n> \n> No human will see this response so the faster --no-pretty option makes \n> sense.\n> \n>>           or die \"open2() failed: $!\\n\" .\n>>           \"Falling back to scanning...\\n\";\n>>\n"},{"id":"331120","messageId":"alpine.DEB.2.10.1710261425390.9817@alexmv-linux","threadId":"47030","inReplyTo":"5a389621-961d-48e3-339a-ef182d3ed628@gmail.com","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T21:27:07Z","receivedAt":"2017-10-26T21:27:21Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"On Thu, 26 Oct 2017, Ben Peart wrote:\n> On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> > diff --git a/fsmonitor.c b/fsmonitor.c\n> > index 7c1540c05..0d26ff34f 100644\n> > --- a/fsmonitor.c\n> > +++ b/fsmonitor.c\n> > @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t\n> > last_update, struct strbuf *que\n> >   \targv[3] = NULL;\n> >   \tcp.argv = argv;\n> >   \tcp.use_shell = 1;\n> > +        cp.dir = get_git_work_tree();\n> >   \n>\n> I'm not sure what would trigger this problem but I don't doubt that it is\n> possible.  Better to be safe than sorry. This is a better/faster solution than\n> you're previous patch.  Thank you!\n\nSee my response on the v1 of this series -- I'm curious how you're\n_not_ seeing it, actually.  Can  you not replicate just by running\n`git status` from differing parts of the working tree?\n - Alex\n"},{"id":"331121","messageId":"alpine.DEB.2.10.1710261427300.9817@alexmv-linux","threadId":"47030","inReplyTo":"9bd24b15-6232-1afd-abbe-02870c51c7ad@gmail.com","subject":"Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-26T21:29:57Z","receivedAt":"2017-10-26T21:30:10Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"On Thu, 26 Oct 2017, Ben Peart wrote:\n> On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> > diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman\n> > index a3e30bf54..79f24325c 100755\n> > --- a/t/t7519/fsmonitor-watchman\n> > +++ b/t/t7519/fsmonitor-watchman\n> > @@ -50,7 +50,7 @@ launch_watchman();\n> >     sub launch_watchman {\n> >   -\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n> > +\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n> \n> Since this is a test script performance isn't critical.  This version of the\n> integration script logs the response to a file in .git/watchman-response.json\n> and is much more human readable without the \"--no-pretty.\"  As such, I'd leave\n> this one pretty.\n\nThis would be the first delta between the test file and the template\nfile.  It seems quite important to me to attempt to ensure that we're\ntesting the _same_ contents that we're suggesting users set up.  In\nfact, it makes more sense to me to just turn this into a symlink to the\nsample template.\n - Alex\n"},{"id":"331137","messageId":"11f3e539-1a4f-0572-a712-9f1c50359389@gmail.com","threadId":"47030","inReplyTo":"alpine.DEB.2.10.1710261427300.9817@alexmv-linux","subject":"Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-27T03:36:13Z","receivedAt":"2017-10-27T03:36:20Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/26/2017 5:29 PM, Alex Vandiver wrote:\n> On Thu, 26 Oct 2017, Ben Peart wrote:\n>> On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n>>> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman\n>>> index a3e30bf54..79f24325c 100755\n>>> --- a/t/t7519/fsmonitor-watchman\n>>> +++ b/t/t7519/fsmonitor-watchman\n>>> @@ -50,7 +50,7 @@ launch_watchman();\n>>>      sub launch_watchman {\n>>>    -\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j')\n>>> +\tmy $pid = open2(\\*CHLD_OUT, \\*CHLD_IN, 'watchman -j --no-pretty')\n>>\n>> Since this is a test script performance isn't critical.  This version of the\n>> integration script logs the response to a file in .git/watchman-response.json\n>> and is much more human readable without the \"--no-pretty.\"  As such, I'd leave\n>> this one pretty.\n> \n> This would be the first delta between the test file and the template\n> file.  It seems quite important to me to attempt to ensure that we're\n> testing the _same_ contents that we're suggesting users set up.  In\n> fact, it makes more sense to me to just turn this into a symlink to the\n> sample template.\n>   - Alex\n> \n\nIf you look closer (actually diff the two files) you will see that the \ntest version includes logging that the sample doesn't include.  I found \nthe logging very helpful during testing of the feature and debugging \nWatchman issues but don't want end users to pay the performance penalty \nof the logging.\n\nt/t7519/fsmonitor-watchman isn't actually used by any of the automated \ntests as they can't assume Watchman is available. It is just provided as \na convenience to enable manual testing and debugging.\n"},{"id":"331138","messageId":"16467d0e-b585-17d7-fb22-3eed7efe6747@gmail.com","threadId":"47030","inReplyTo":"alpine.DEB.2.10.1710261425390.9817@alexmv-linux","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-10-27T03:40:34Z","receivedAt":"2017-10-27T03:40:40Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 10/26/2017 5:27 PM, Alex Vandiver wrote:\n> On Thu, 26 Oct 2017, Ben Peart wrote:\n>> On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n>>> diff --git a/fsmonitor.c b/fsmonitor.c\n>>> index 7c1540c05..0d26ff34f 100644\n>>> --- a/fsmonitor.c\n>>> +++ b/fsmonitor.c\n>>> @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t\n>>> last_update, struct strbuf *que\n>>>    \targv[3] = NULL;\n>>>    \tcp.argv = argv;\n>>>    \tcp.use_shell = 1;\n>>> +        cp.dir = get_git_work_tree();\n>>>    \n>>\n>> I'm not sure what would trigger this problem but I don't doubt that it is\n>> possible.  Better to be safe than sorry. This is a better/faster solution than\n>> you're previous patch.  Thank you!\n> \n> See my response on the v1 of this series -- I'm curious how you're\n> _not_ seeing it, actually.  Can  you not replicate just by running\n> `git status` from differing parts of the working tree?\n>   - Alex\n> \n\nI saw your response but actually can't replicate it locally.  I've been \nrunning with Watchman integration on all my repos for months and my \n\"watchman watch-list\" command only shows the root of the various working \ndirectories - no subdirectories.\n"},{"id":"331192","messageId":"alpine.DEB.2.10.1710271619180.1371@alexmv-linux","threadId":"47030","inReplyTo":"16467d0e-b585-17d7-fb22-3eed7efe6747@gmail.com","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Alex Vandiver","fromEmail":"alexmv@dropbox.com","sentAt":"2017-10-27T23:20:39Z","receivedAt":"2017-10-27T23:20:58Z","isPatch":true,"sender":{"key":"alexmv@dropbox.com","avatar":null},"body":"On Thu, 26 Oct 2017, Ben Peart wrote:\n> I saw your response but actually can't replicate it locally.  I've been\n> running with Watchman integration on all my repos for months and my \"watchman\n> watch-list\" command only shows the root of the various working directories -\n> no subdirectories.\n\nWeird.  I double-checked and I see the same behavior with watchman\n4.9.0 as with 4.7.0 that I had been using previously.  I wonder if\nsomething's different between `git` in `next` from wherever your\nbranch was based.\n - Alex\n"},{"id":"331266","messageId":"alpine.DEB.2.21.1.1710291318300.6482@virtualbox","threadId":"47030","inReplyTo":"16467d0e-b585-17d7-fb22-3eed7efe6747@gmail.com","subject":"Re: [PATCH v2 1/4] fsmonitor: Set the PWD to the top of the working tree","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-29T12:27:32Z","receivedAt":"2017-10-29T12:28:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Oct 2017, Ben Peart wrote:\n\n> On 10/26/2017 5:27 PM, Alex Vandiver wrote:\n> > On Thu, 26 Oct 2017, Ben Peart wrote:\n> > > On 10/25/2017 9:31 PM, Alex Vandiver wrote:\n> > > > diff --git a/fsmonitor.c b/fsmonitor.c\n> > > > index 7c1540c05..0d26ff34f 100644\n> > > > --- a/fsmonitor.c\n> > > > +++ b/fsmonitor.c\n> > > > @@ -121,6 +121,7 @@ static int query_fsmonitor(int version, uint64_t\n> > > > last_update, struct strbuf *que\n> > > >     argv[3] = NULL;\n> > > >     cp.argv = argv;\n> > > >     cp.use_shell = 1;\n> > > > +        cp.dir = get_git_work_tree();\n> > > >    \n> > >\n> > > I'm not sure what would trigger this problem but I don't doubt that it is\n> > > possible.  Better to be safe than sorry. This is a better/faster solution\n> > > than\n> > > you're previous patch.  Thank you!\n> > \n> > See my response on the v1 of this series -- I'm curious how you're\n> > _not_ seeing it, actually.  Can  you not replicate just by running\n> > `git status` from differing parts of the working tree?\n> >   - Alex\n> > \n> \n> I saw your response but actually can't replicate it locally.  I've been\n> running with Watchman integration on all my repos for months and my \"watchman\n> watch-list\" command only shows the root of the various working directories -\n> no subdirectories.\n\nIndeed, I cannot replicate either. The thing is that \"status\" is marked\nwith GIT_SETUP in git.c:\n\n\thttps://github.com/git-for-windows/git/blob/v2.14.3.windows.1/git.c#L465\n\nThat means that the setup_git_directory() is run, which sets the current\nworking directory to the top-level directory.\n\nSo there is your explanation why neither Ben nor I saw this.\n\nAnd I agree with Ben that it is safer to do it the way you suggested, just\nin case that the call path comes from a Git command that was not marked\nwith GIT_SETUP.\n\nCiao,\nJohannes\n"},{"id":"331267","messageId":"alpine.DEB.2.21.1.1710291331250.6482@virtualbox","threadId":"47030","inReplyTo":"20171026013117.30034-1-alexmv@dropbox.com","subject":"Re: [PATCH v2 0/4] fsmonitor fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-10-29T12:31:44Z","receivedAt":"2017-10-29T12:31:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Alex,\n\nOn Wed, 25 Oct 2017, Alex Vandiver wrote:\n\n> Updated based on comments from Dscho and Ben.  Thanks for those!\n\nThank you for this excellent improvement.\n\nCiao,\nDscho\n"}]}