{"thread":{"id":"46090","subject":"[PATCH v4 0/6] Fast git status via a file system watcher","startedAt":"2017-06-01T15:51:30Z","lastAt":"2017-06-08T01:57:58Z","messageCount":29,"participants":["Ben Peart","Ævar Arnfjörð Bjarmason","Stefan Beller","Junio C Hamano","David Turner","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":6},"messages":[{"id":"321287","messageId":"20170601155105.28356-1-benpeart@microsoft.com","threadId":"46090","inReplyTo":null,"subject":"[PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:50:59Z","receivedAt":"2017-06-01T15:51:30Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Changes from V3 include:\n - update test script based on feedback\n - update template hook proc with better post-processing code and make\n it executable\n\nBen Peart (6):\n bswap: add 64 bit endianness helper get_be64\n dir: make lookup_untracked() available outside of dir.c\n fsmonitor: teach git to optionally utilize a file system monitor to\n speed up detecting new or changed files.\n fsmonitor: add test cases for fsmonitor extension\n fsmonitor: add documentation for the fsmonitor extension.\n fsmonitor: add a sample query-fsmonitor hook script for Watchman\n\n Documentation/config.txt | 7 +\n Documentation/githooks.txt | 23 +++\n Documentation/technical/index-format.txt | 19 +++\n Makefile | 1 +\n builtin/update-index.c | 1 +\n cache.h | 5 +\n compat/bswap.h | 4 +\n config.c | 5 +\n dir.c | 16 ++-\n dir.h | 5 +\n entry.c | 1 +\n environment.c | 1 +\n fsmonitor.c | 238 +++++++++++++++++++++++++++++++\n fsmonitor.h | 9 ++\n read-cache.c | 28 +++-\n t/t7519-status-fsmonitor.sh | 173 ++++++++++++++++++++++\n templates/hooks--query-fsmonitor.sample | 60 ++++++++\n unpack-trees.c | 1 +\n 18 files changed, 594 insertions(+), 3 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n create mode 100755 t/t7519-status-fsmonitor.sh\n create mode 100755 templates/hooks--query-fsmonitor.sample\n\nInterdiff (v3..v4):\n\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nindex 395db46d55..458eabe6dc 100755\n--- a/t/t7519-status-fsmonitor.sh\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -5,48 +5,46 @@ test_description='git status with file system watcher'\n . ./test-lib.sh\n \n clean_repo () {\n-\tgit reset --hard HEAD\n-\tgit clean -fd\n+\tgit reset --hard HEAD &&\n+\tgit clean -fd &&\n \trm -f marker\n }\n \n dirty_repo () {\n-\t: >untracked\n-\t: >dir1/untracked\n-\t: >dir2/untracked\n-\techo 1 >modified\n-\techo 2 >dir1/modified\n-\techo 3 >dir2/modified\n-\techo 4 >new\n-\techo 5 >dir1/new\n-\techo 6 >dir2/new\n-\tgit add new\n-\tgit add dir1/new\n+\t: >untracked &&\n+\t: >dir1/untracked &&\n+\t: >dir2/untracked &&\n+\techo 1 >modified &&\n+\techo 2 >dir1/modified &&\n+\techo 3 >dir2/modified &&\n+\techo 4 >new &&\n+\techo 5 >dir1/new &&\n+\techo 6 >dir2/new &&\n+\tgit add new &&\n+\tgit add dir1/new &&\n \tgit add dir2/new\n }\n \n # The test query-fsmonitor hook proc will output a marker file we can use to\n # ensure the hook was actually used to generate the correct results.\n \n+# fsmonitor works correctly with or without the untracked cache\n+# but if it is available, we'll turn it on to ensure we test that\n+# codepath as well.\n+\n+test_lazy_prereq UNTRACKED_CACHE '\n+\t{ git update-index --test-untracked-cache; ret=$?; } &&\n+\ttest $ret -ne 1\n+'\n+\n+if test_have_prereq UNTRACKED_CACHE; then\n+\tgit config core.untrackedcache true\n+else\n+\tgit config core.untrackedcache false\n+fi\n+\n test_expect_success 'setup' '\n \tmkdir -p .git/hooks &&\n-\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n-\tif [ $1 -ne 1 ]\n-\tthen\n-\t\techo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n-\t\texit 1;\n-\tfi\n-\t: >marker\n-\tprintf \"untracked\\0\"\n-\tprintf \"dir1/untracked\\0\"\n-\tprintf \"dir2/untracked\\0\"\n-\tprintf \"modified\\0\"\n-\tprintf \"dir1/modified\\0\"\n-\tprintf \"dir2/modified\\0\"\n-\tprintf \"new\\0\"\"\n-\tprintf \"dir1/new\\0\"\n-\tprintf \"dir2/new\\0\"\n-\tEOF\n \t: >tracked &&\n \t: >modified &&\n \tmkdir dir1 &&\n@@ -58,55 +56,19 @@ test_expect_success 'setup' '\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial &&\n-\tdirty_repo\n-'\n-\n-cat >.gitignore <<\\EOF\n-.gitignore\n-expect*\n-output*\n-marker*\n-EOF\n-\n-# Status is well tested elsewhere so we'll just ensure that the results are\n-# the same when using core.fsmonitor. First call after turning on the option\n-# does a complete scan so need to do two calls to ensure we test the new\n-# codepath.\n-\n-test_expect_success 'status with core.untrackedcache true' '\n-\tgit config core.fsmonitor true &&\n-\tgit config core.untrackedcache true &&\n-\tgit -c core.fsmonitor=false -c core.untrackedcache=true status >expect &&\n-\tclean_repo &&\n-\tgit status &&\n-\ttest_path_is_missing marker &&\n-\tdirty_repo &&\n-\tgit status >output &&\n-\ttest_path_is_file marker &&\n-\ttest_i18ncmp expect output\n-'\n-\n-\n-test_expect_success 'status with core.untrackedcache false' '\n \tgit config core.fsmonitor true &&\n-\tgit config core.untrackedcache false &&\n-\tgit -c core.fsmonitor=false -c core.untrackedcache=false status >expect &&\n-\tclean_repo &&\n-\tgit status &&\n-\ttest_path_is_missing marker &&\n-\tdirty_repo &&\n-\tgit status >output &&\n-\ttest_path_is_file marker &&\n-\ttest_i18ncmp expect output\n+\tcat >.gitignore <<-\\EOF\n+\t.gitignore\n+\texpect*\n+\toutput*\n+\tmarker*\n+\tEOF\n '\n \n # Ensure commands that call refresh_index() to move the index back in time\n # properly invalidate the fsmonitor cache\n \n test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n-\tgit config core.fsmonitor true &&\n-\tgit config core.untrackedcache true &&\n-\tclean_repo &&\n \tgit status &&\n \ttest_path_is_missing marker &&\n \tdirty_repo &&\n@@ -118,6 +80,7 @@ test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n \tgit status &&\n \ttest_path_is_file marker &&\n \tgit reset HEAD~1 &&\n+\trm -f marker &&\n \tgit status >output &&\n \ttest_path_is_file marker &&\n \tgit -c core.fsmonitor=false status >expect &&\n@@ -129,9 +92,7 @@ test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n # extensions exist other than 'TREE' so do a \"git status\" to get the extension\n # written before testing the results.\n \n-test_expect_success 'status doesnt detect unreported modifications' '\n-\tgit config core.fsmonitor true &&\n-\tgit config core.untrackedcache true &&\n+test_expect_success \"status doesn't detect unreported modifications\" '\n \twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n \t:>marker\n \tEOF\n@@ -146,13 +107,67 @@ test_expect_success 'status doesnt detect unreported modifications' '\n \ttest_i18ngrep ! \"Untracked files:\" output &&\n \twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n \t:>marker\n-\tprintf \"untracked%s\\0\"\n+\tprintf \"untracked\\0\"\n \tprintf \"dir1/modified\\0\"\n \tEOF\n+\trm -f marker &&\n \tgit status >output &&\n \ttest_path_is_file marker &&\n \ttest_i18ngrep \"Changes not staged for commit:\" output &&\n \ttest_i18ngrep \"Untracked files:\" output\n '\n \n+# Status is well tested elsewhere so we'll just ensure that the results are\n+# the same when using core.fsmonitor. First call after turning on the option\n+# does a complete scan so we need to do two calls to ensure we test the new\n+# codepath.\n+\n+test_expect_success 'status with core.untrackedcache false' '\n+\tgit config core.untrackedcache false &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tif [ $1 -ne 1 ]\n+\tthen\n+\t\techo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n+\t\texit 1;\n+\tfi\n+\t: >marker\n+\tprintf \"untracked\\0\"\n+\tprintf \"dir1/untracked\\0\"\n+\tprintf \"dir2/untracked\\0\"\n+\tprintf \"modified\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tprintf \"dir2/modified\\0\"\n+\tprintf \"new\\0\"\"\n+\tprintf \"dir1/new\\0\"\n+\tprintf \"dir2/new\\0\"\n+\tEOF\n+\tclean_repo &&\n+\tdirty_repo &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n+if ! test_have_prereq UNTRACKED_CACHE; then\n+\tskip_all='This system does not support untracked cache'\n+\ttest_done\n+fi\n+\n+test_expect_success 'status with core.untrackedcache true' '\n+\tgit config core.untrackedcache true &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n test_done\ndiff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--query-fsmonitor.sample\nold mode 100644\nnew mode 100755\nindex 615f3332fa..941c4c5b57\n--- a/templates/hooks--query-fsmonitor.sample\n+++ b/templates/hooks--query-fsmonitor.sample\n@@ -4,10 +4,10 @@\n # (https://facebook.github.io/watchman/) with git to provide fast\n # git status.\n #\n-# The hook is passed a time in nanoseconds formatted as a string and\n-# outputs to stdout all files that have been modified since the given\n-# time. Paths must be relative to the root of the working tree and\n-# separated by a single NUL.\n+# The hook is passed a version (currently 1) and a time in nanoseconds\n+# formatted as a string and outputs to stdout all files that have been\n+# modified since the given time. Paths must be relative to the root of\n+# the working tree and separated by a single NUL.\n #\n # To enable this hook, rename this file to \"query-fsmonitor\"\n \n@@ -33,5 +33,28 @@ esac\n \n # Query Watchman for all the changes since the requested time\n echo \"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $time_t, \\\"fields\\\":[\\\"name\\\"]}]\" | \\\n-watchman -j | \\\n-perl -e 'use JSON::PP; my $o = JSON::PP->new->utf8->decode(join(\"\", <>)); die \"Watchman: $o->{'error'}.\\nFalling back to scanning...\\n\" if defined($o->{\"error\"}); print(join(\"\\0\", @{$o->{\"files\"}}));'\n+\twatchman -j |\n+\tperl -0666 -e '\n+\t\tuse strict;\n+\t\tuse warnings;\n+\n+\t\tmy $stdin = <>;\n+\t\tdie \"Watchman: command returned no output.\\nFalling back to scanning...\\n\" if $stdin eq \"\";\n+\t\tdie \"Watchman: command returned invalid output: $stdin\\nFalling back to scanning...\\n\" unless $stdin =~ /^\\{/;\n+\n+\t\tmy $json_pkg;\n+\t\teval {\n+\t\t\trequire JSON::XS;\n+\t\t\t$json_pkg = \"JSON::XS\";\n+\t\t\t1;\n+\t\t} or do {\n+\t\t\trequire JSON::PP;\n+\t\t\t$json_pkg = \"JSON::PP\";\n+\t\t};\n+\n+\t\tmy $o = $json_pkg->new->utf8->decode($stdin);\n+\t\tdie \"Watchman: $o->{error}.\\nFalling back to scanning...\\n\" if $o->{error};\n+\n+\t\tlocal $, = \"\\0\";\n+\t\tprint @{$o->{files}};\n+\t'\n\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321288","messageId":"20170601155105.28356-5-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 4/6] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:03Z","receivedAt":"2017-06-01T15:51:32Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add test cases that ensure status results are correct when using the new\nfsmonitor extension. Test untracked, modified, and new files by\nensuring the results are identical to when not using the extension.\n\nAdd a test to ensure updates to the index properly mark corresponding\nentries in the index extension as dirty so that the status is correct\nafter commands that modify the index but don't trigger changes in the\nworking directory.\n\nAdd a test that verifies that if the fsmonitor extension doesn't tell\ngit about a change, it doesn't discover it on its own. This ensures\ngit is honoring the extension and that we get the performance benefits\ndesired.\n\nAll test hooks output a marker file that is used to ensure the hook\nwas actually used to generate the test results.\n\nSigned-off-by: Ben Peart \n---\n t/t7519-status-fsmonitor.sh | 173 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 173 insertions(+)\n create mode 100755 t/t7519-status-fsmonitor.sh\n\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nnew file mode 100755\nindex 0000000000..458eabe6dc\n--- /dev/null\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -0,0 +1,173 @@\n+#!/bin/sh\n+\n+test_description='git status with file system watcher'\n+\n+. ./test-lib.sh\n+\n+clean_repo () {\n+\tgit reset --hard HEAD &&\n+\tgit clean -fd &&\n+\trm -f marker\n+}\n+\n+dirty_repo () {\n+\t: >untracked &&\n+\t: >dir1/untracked &&\n+\t: >dir2/untracked &&\n+\techo 1 >modified &&\n+\techo 2 >dir1/modified &&\n+\techo 3 >dir2/modified &&\n+\techo 4 >new &&\n+\techo 5 >dir1/new &&\n+\techo 6 >dir2/new &&\n+\tgit add new &&\n+\tgit add dir1/new &&\n+\tgit add dir2/new\n+}\n+\n+# The test query-fsmonitor hook proc will output a marker file we can use to\n+# ensure the hook was actually used to generate the correct results.\n+\n+# fsmonitor works correctly with or without the untracked cache\n+# but if it is available, we'll turn it on to ensure we test that\n+# codepath as well.\n+\n+test_lazy_prereq UNTRACKED_CACHE '\n+\t{ git update-index --test-untracked-cache; ret=$?; } &&\n+\ttest $ret -ne 1\n+'\n+\n+if test_have_prereq UNTRACKED_CACHE; then\n+\tgit config core.untrackedcache true\n+else\n+\tgit config core.untrackedcache false\n+fi\n+\n+test_expect_success 'setup' '\n+\tmkdir -p .git/hooks &&\n+\t: >tracked &&\n+\t: >modified &&\n+\tmkdir dir1 &&\n+\t: >dir1/tracked &&\n+\t: >dir1/modified &&\n+\tmkdir dir2 &&\n+\t: >dir2/tracked &&\n+\t: >dir2/modified &&\n+\tgit add . &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tgit config core.fsmonitor true &&\n+\tcat >.gitignore <<-\\EOF\n+\t.gitignore\n+\texpect*\n+\toutput*\n+\tmarker*\n+\tEOF\n+'\n+\n+# Ensure commands that call refresh_index() to move the index back in time\n+# properly invalidate the fsmonitor cache\n+\n+test_expect_success 'refresh_index() invalidates fsmonitor cache' '\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tEOF\n+\tgit add . &&\n+\tgit commit -m \"to reset\" &&\n+\tgit status &&\n+\ttest_path_is_file marker &&\n+\tgit reset HEAD~1 &&\n+\trm -f marker &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\ttest_i18ncmp expect output\n+'\n+\n+# Now make sure it's actually skipping the check for modified and untracked\n+# files unless it is told about them. Note, after \"git reset --hard HEAD\" no\n+# extensions exist other than 'TREE' so do a \"git status\" to get the extension\n+# written before testing the results.\n+\n+test_expect_success \"status doesn't detect unreported modifications\" '\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tEOF\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\t: >untracked &&\n+\techo 2 >dir1/modified &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ngrep ! \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep ! \"Untracked files:\" output &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:>marker\n+\tprintf \"untracked\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tEOF\n+\trm -f marker &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ngrep \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep \"Untracked files:\" output\n+'\n+\n+# Status is well tested elsewhere so we'll just ensure that the results are\n+# the same when using core.fsmonitor. First call after turning on the option\n+# does a complete scan so we need to do two calls to ensure we test the new\n+# codepath.\n+\n+test_expect_success 'status with core.untrackedcache false' '\n+\tgit config core.untrackedcache false &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tif [ $1 -ne 1 ]\n+\tthen\n+\t\techo -e \"Unsupported query-fsmonitor hook version.\\n\" >&2\n+\t\texit 1;\n+\tfi\n+\t: >marker\n+\tprintf \"untracked\\0\"\n+\tprintf \"dir1/untracked\\0\"\n+\tprintf \"dir2/untracked\\0\"\n+\tprintf \"modified\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tprintf \"dir2/modified\\0\"\n+\tprintf \"new\\0\"\"\n+\tprintf \"dir1/new\\0\"\n+\tprintf \"dir2/new\\0\"\n+\tEOF\n+\tclean_repo &&\n+\tdirty_repo &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n+if ! test_have_prereq UNTRACKED_CACHE; then\n+\tskip_all='This system does not support untracked cache'\n+\ttest_done\n+fi\n+\n+test_expect_success 'status with core.untrackedcache true' '\n+\tgit config core.untrackedcache true &&\n+\tgit -c core.fsmonitor=false status >expect &&\n+\tclean_repo &&\n+\tgit status &&\n+\ttest_path_is_missing marker &&\n+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_path_is_file marker &&\n+\ttest_i18ncmp expect output\n+'\n+\n+test_done\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321289","messageId":"20170601155105.28356-4-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 3/6] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:02Z","receivedAt":"2017-06-01T15:51:34Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"When the index is read from disk, the query-fsmonitor index extension is\nused to flag the last known potentially dirty index and untracked cache\nentries.\n\nIf git finds out some entries are 'fsmonitor-dirty', but are really\nunchanged (e.g. the file was changed, then reverted back), then Git will\nclear the marking in the extension. If git adds or updates an index\nentry, it is marked 'fsmonitor-dirty' to ensure it is checked for\nchanges in the working directory.\n\nBefore the 'fsmonitor-dirty' flags are used to limit the scope of the\nfiles to be checked, the query-fsmonitor hook proc is called with the\ntime the index was last updated. The hook proc returns the list of\nfiles changed since that last updated time and the list of\npotentially dirty entries is updated to reflect the current state.\n\nrefresh_index() and valid_cached_dir() are updated so that any entry not\nflagged as potentially dirty is not checked as it cannot have any\nchanges.\n\nSigned-off-by: Ben Peart \n---\n Makefile | 1 +\n builtin/update-index.c | 1 +\n cache.h | 5 ++\n config.c | 5 ++\n dir.c | 14 +++\n dir.h | 2 +\n entry.c | 1 +\n environment.c | 1 +\n fsmonitor.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++\n fsmonitor.h | 9 ++\n read-cache.c | 28 +++++-\n unpack-trees.c | 1 +\n 12 files changed, 304 insertions(+), 2 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n\ndiff --git a/Makefile b/Makefile\nindex e35542e631..488a4466cc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -760,6 +760,7 @@ LIB_OBJS += ewah/ewah_rlw.o\n LIB_OBJS += exec_cmd.o\n LIB_OBJS += fetch-pack.o\n LIB_OBJS += fsck.o\n+LIB_OBJS += fsmonitor.o\n LIB_OBJS += gettext.o\n LIB_OBJS += gpg-interface.o\n LIB_OBJS += graph.o\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex ebfc09faa0..32fd977b43 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -232,6 +232,7 @@ static int mark_ce_flags(const char *path, int flag, int mark)\n \t\telse\n \t\t\tactive_cache[pos]->ce_flags &= ~flag;\n \t\tactive_cache[pos]->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tactive_cache[pos]->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tcache_tree_invalidate_path(&the_index, path);\n \t\tactive_cache_changed |= CE_ENTRY_CHANGED;\n \t\treturn 0;\ndiff --git a/cache.h b/cache.h\nindex 188811920c..58e5abf69f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -201,6 +201,7 @@ struct cache_entry {\n #define CE_ADDED (1 << 19)\n \n #define CE_HASHED (1 << 20)\n+#define CE_FSMONITOR_DIRTY (1 << 21)\n #define CE_WT_REMOVE (1 << 22) /* remove in work directory */\n #define CE_CONFLICTED (1 << 23)\n \n@@ -324,6 +325,7 @@ static inline unsigned int canon_mode(unsigned int mode)\n #define CACHE_TREE_CHANGED\t(1 << 5)\n #define SPLIT_INDEX_ORDERED\t(1 << 6)\n #define UNTRACKED_CHANGED\t(1 << 7)\n+#define FSMONITOR_CHANGED\t(1 << 8)\n \n struct split_index;\n struct untracked_cache;\n@@ -342,6 +344,8 @@ struct index_state {\n \tstruct hashmap dir_hash;\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@@ -766,6 +770,7 @@ extern int precomposed_unicode;\n extern int protect_hfs;\n extern int protect_ntfs;\n extern int git_db_env, git_index_env, git_graft_env, git_common_dir_env;\n+extern int core_fsmonitor;\n \n /*\n * Include broken refs in all ref iterations, which will\ndiff --git a/config.c b/config.c\nindex bb4d735701..1a8108636d 100644\n--- a/config.c\n+++ b/config.c\n@@ -1232,6 +1232,11 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.fsmonitor\")) {\n+\t\tcore_fsmonitor = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/dir.c b/dir.c\nindex 1b5558fdf9..37f1c580a5 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -16,6 +16,7 @@\n #include \"utf8.h\"\n #include \"varint.h\"\n #include \"ewah/ewok.h\"\n+#include \"fsmonitor.h\"\n \n /*\n * Tells read_directory_recursive how a file or directory should be treated.\n@@ -1652,6 +1653,18 @@ static int valid_cached_dir(struct dir_struct *dir,\n \tif (!untracked)\n \t\treturn 0;\n \n+\trefresh_by_fsmonitor(&the_index);\n+\tif (dir->untracked->use_fsmonitor) {\n+\t\t/*\n+\t\t * With fsmonitor, we can trust the untracked cache's\n+\t\t * valid field.\n+\t\t */\n+\t\tif (untracked->valid)\n+\t\t\tgoto skip_stat;\n+\t\telse\n+\t\t\tinvalidate_directory(dir->untracked, untracked);\n+\t}\n+\n \tif (stat(path->len ? path->buf : \".\", &st)) {\n \t\tinvalidate_directory(dir->untracked, untracked);\n \t\tmemset(&untracked->stat_data, 0, sizeof(untracked->stat_data));\n@@ -1665,6 +1678,7 @@ static int valid_cached_dir(struct dir_struct *dir,\n \t\treturn 0;\n \t}\n \n+skip_stat:\n \tif (untracked->check_only != !!check_only) {\n \t\tinvalidate_directory(dir->untracked, untracked);\n \t\treturn 0;\ndiff --git a/dir.h b/dir.h\nindex 9e387551bd..ff6a00abcc 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -139,6 +139,8 @@ struct untracked_cache {\n \tint gitignore_invalidated;\n \tint dir_invalidated;\n \tint dir_opened;\n+\t/* fsmonitor invalidation data */\n+\tunsigned int use_fsmonitor : 1;\n };\n \n struct dir_struct {\ndiff --git a/entry.c b/entry.c\nindex d2b512da90..c2d3c1079c 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -221,6 +221,7 @@ static int write_entry(struct cache_entry *ce,\n \t\t\tlstat(ce->name, &st);\n \t\tfill_stat_cache_info(ce, &st);\n \t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tstate->istate->cache_changed |= CE_ENTRY_CHANGED;\n \t}\n \treturn 0;\ndiff --git a/environment.c b/environment.c\nindex 560408953c..1afabbae8c 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -64,6 +64,7 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;\n enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;\n+int core_fsmonitor;\n \n #ifndef PROTECT_HFS_DEFAULT\n #define PROTECT_HFS_DEFAULT 0\ndiff --git a/fsmonitor.c b/fsmonitor.c\nnew file mode 100644\nindex 0000000000..3ce262d47d\n--- /dev/null\n+++ b/fsmonitor.c\n@@ -0,0 +1,238 @@\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"ewah/ewok.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n+#include \"fsmonitor.h\"\n+\n+#define INDEX_EXTENSION_VERSION\t1\n+#define HOOK_INTERFACE_VERSION\t\t1\n+\n+static struct untracked_cache_dir *find_untracked_cache_dir(\n+\tstruct untracked_cache *uc, struct untracked_cache_dir *ucd,\n+\tconst char *name)\n+{\n+\tconst char *end;\n+\tstruct untracked_cache_dir *dir = ucd;\n+\n+\tif (!*name)\n+\t\treturn dir;\n+\n+\tend = strchr(name, '/');\n+\tif (end) {\n+\t\tdir = lookup_untracked(uc, ucd, name, end - name);\n+\t\tif (dir)\n+\t\t\treturn find_untracked_cache_dir(uc, dir, end + 1);\n+\t}\n+\n+\treturn dir;\n+}\n+\n+/* This function will be passed to ewah_each_bit() */\n+static void mark_no_fsmonitor(size_t pos, void *is)\n+{\n+\tstruct index_state *istate = is;\n+\tstruct untracked_cache_dir *dir;\n+\tstruct cache_entry *ce = istate->cache[pos];\n+\n+\tassert(pos < istate->cache_nr);\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn;\n+\n+\tdir = find_untracked_cache_dir(istate->untracked, istate->untracked->root, ce->name);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+}\n+\n+int read_fsmonitor_extension(struct index_state *istate, const void *data,\n+\tunsigned long sz)\n+{\n+\tconst char *index = data;\n+\tuint32_t hdr_version;\n+\tuint32_t ewah_size;\n+\tint ret;\n+\n+\tif (sz < sizeof(uint32_t) + sizeof(uint64_t) + sizeof(uint32_t))\n+\t\treturn error(\"corrupt fsmonitor extension (too short)\");\n+\n+\thdr_version = get_be32(index);\n+\tindex += sizeof(uint32_t);\n+\tif (hdr_version != INDEX_EXTENSION_VERSION)\n+\t\treturn error(\"bad fsmonitor version %d\", hdr_version);\n+\n+\tistate->fsmonitor_last_update = get_be64(index);\n+\tindex += sizeof(uint64_t);\n+\n+\tewah_size = get_be32(index);\n+\tindex += sizeof(uint32_t);\n+\n+\tistate->fsmonitor_dirty = ewah_new();\n+\tret = ewah_read_mmap(istate->fsmonitor_dirty, index, ewah_size);\n+\tif (ret != ewah_size) {\n+\t\tewah_free(istate->fsmonitor_dirty);\n+\t\tistate->fsmonitor_dirty = NULL;\n+\t\treturn error(\"failed to parse ewah bitmap reading fsmonitor index extension\");\n+\t}\n+\n+\treturn 0;\n+}\n+\n+void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)\n+{\n+\tuint32_t hdr_version;\n+\tuint64_t tm;\n+\tstruct ewah_bitmap *bitmap;\n+\tint i;\n+\tuint32_t ewah_start;\n+\tuint32_t ewah_size = 0;\n+\tint fixup = 0;\n+\n+\thdr_version = htonl(INDEX_EXTENSION_VERSION);\n+\tstrbuf_add(sb, &hdr_version, sizeof(uint32_t));\n+\n+\ttm = htonll((uint64_t)istate->fsmonitor_last_update);\n+\tstrbuf_add(sb, &tm, sizeof(uint64_t));\n+\tfixup = sb->len;\n+\tstrbuf_add(sb, &ewah_size, sizeof(uint32_t)); /* we'll fix this up later */\n+\n+\tewah_start = sb->len;\n+\tbitmap = ewah_new();\n+\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\tif (istate->cache[i]->ce_flags & CE_FSMONITOR_DIRTY)\n+\t\t\tewah_set(bitmap, i);\n+\tewah_serialize_strbuf(bitmap, sb);\n+\tewah_free(bitmap);\n+\n+\t/* fix up size field */\n+\tewah_size = htonl(sb->len - ewah_start);\n+\tmemcpy(sb->buf + fixup, &ewah_size, sizeof(uint32_t));\n+}\n+\n+static void mark_file_dirty(struct index_state *istate, const char *name)\n+{\n+\tstruct untracked_cache_dir *dir;\n+\tint pos;\n+\n+\t/* find it in the index and mark that entry as dirty */\n+\tpos = index_name_pos(istate, name, strlen(name));\n+\tif (pos >= 0)\n+\t\tistate->cache[pos]->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\t/*\n+\t * Find the corresponding directory in the untracked cache\n+\t * and mark it as invalid\n+\t */\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn;\n+\n+\tdir = find_untracked_cache_dir(istate->untracked, istate->untracked->root, name);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+}\n+\n+/*\n+ * Call the query-fsmonitor hook passing the time of the last saved results.\n+ */\n+static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *query_result)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar ver[64];\n+\tchar date[64];\n+\tconst char *argv[4];\n+\n+\tif (!(argv[0] = find_hook(\"query-fsmonitor\")))\n+\t\treturn -1;\n+\n+\tsnprintf(ver, sizeof(version), \"%d\", version);\n+\tsnprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n+\targv[1] = ver;\n+\targv[2] = date;\n+\targv[3] = NULL;\n+\tcp.argv = argv;\n+\tcp.out = -1;\n+\n+\treturn capture_command(&cp, query_result, 1024);\n+}\n+\n+void process_fsmonitor_extension(struct index_state *istate)\n+{\n+\tif (!istate->fsmonitor_dirty)\n+\t\treturn;\n+\n+\tewah_each_bit(istate->fsmonitor_dirty, mark_no_fsmonitor, istate);\n+\tewah_free(istate->fsmonitor_dirty);\n+\tistate->fsmonitor_dirty = NULL;\n+}\n+\n+void refresh_by_fsmonitor(struct index_state *istate)\n+{\n+\tstatic int has_run_once = 0;\n+\tstruct strbuf query_result = STRBUF_INIT;\n+\tint query_success = 0;\n+\tsize_t bol = 0; /* beginning of line */\n+\tuint64_t last_update;\n+\tchar *buf, *entry;\n+\tint i;\n+\n+\tif (!core_fsmonitor || has_run_once)\n+\t\treturn;\n+\thas_run_once = 1;\n+\n+\t/*\n+\t * This could be racy so save the date/time now and the hook\n+\t * should be inclusive to ensure we don't miss potential changes.\n+\t */\n+\tlast_update = getnanotime();\n+\n+\t/*\n+\t * If we have a last update time, call query-monitor for the set of\n+\t * changes since that time.\n+\t */\n+\tif (istate->fsmonitor_last_update)\n+\t\tquery_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n+\t\t\t\t\t\t\tistate->fsmonitor_last_update, &query_result);\n+\n+\tif (query_success) {\n+\t\t/* Mark all entries returned by the monitor as dirty */\n+\t\tbuf = entry = query_result.buf;\n+\t\tfor (i = 0; i < query_result.len; i++) {\n+\t\t\tif (buf[i] != '\\0')\n+\t\t\t\tcontinue;\n+\t\t\tmark_file_dirty(istate, buf + bol);\n+\t\t\tbol = i + 1;\n+\t\t}\n+\t\tif (bol < query_result.len)\n+\t\t\tmark_file_dirty(istate, buf + bol);\n+\n+\t\t/* Mark all clean entries up-to-date */\n+\t\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\t\tif (ce_stage(ce) || (ce->ce_flags & CE_FSMONITOR_DIRTY))\n+\t\t\t\tcontinue;\n+\t\t\tce_mark_uptodate(ce);\n+\t\t}\n+\n+\t\t/*\n+\t\t * Now that we've marked the invalid entries in the\n+\t\t * untracked-cache itself, we can mark the untracked cache for\n+\t\t * fsmonitor usage.\n+\t\t */\n+\t\tif (istate->untracked)\n+\t\t\tistate->untracked->use_fsmonitor = 1;\n+\t} else {\n+\t\t/* if we can't update the cache, fall back to checking them all */\n+\t\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\t\tistate->cache[i]->ce_flags |= CE_FSMONITOR_DIRTY;\n+\n+\t\t/* mark the untracked cache as unusable for fsmonitor */\n+\t\tif (istate->untracked)\n+\t\t\tistate->untracked->use_fsmonitor = 0;\n+\t}\n+\tstrbuf_release(&query_result);\n+\n+\t/* Now that we've updated istate, save the last_update time */\n+\tistate->fsmonitor_last_update = last_update;\n+\tistate->cache_changed |= FSMONITOR_CHANGED;\n+}\ndiff --git a/fsmonitor.h b/fsmonitor.h\nnew file mode 100644\nindex 0000000000..57b061688f\n--- /dev/null\n+++ b/fsmonitor.h\n@@ -0,0 +1,9 @@\n+#ifndef FSMONITOR_H\n+#define FSMONITOR_H\n+\n+int read_fsmonitor_extension(struct index_state *istate, const void *data, unsigned long sz);\n+void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate);\n+void process_fsmonitor_extension(struct index_state *istate);\n+void refresh_by_fsmonitor(struct index_state *istate);\n+\n+#endif\ndiff --git a/read-cache.c b/read-cache.c\nindex 3339de8124..5b52f08db2 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -18,6 +18,7 @@\n #include \"varint.h\"\n #include \"split-index.h\"\n #include \"utf8.h\"\n+#include \"fsmonitor.h\"\n \n /* Mask for the name length in ce_flags in the on-disk index */\n \n@@ -37,11 +38,12 @@\n #define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* \"REUC\" */\n #define CACHE_EXT_LINK 0x6c696e6b\t /* \"link\" */\n #define CACHE_EXT_UNTRACKED 0x554E5452\t /* \"UNTR\" */\n+#define CACHE_EXT_FSMONITOR 0x46534D4E\t /* \"FSMN\" */\n \n /* changes that can be kept in $GIT_DIR/index (basically all extensions) */\n #define EXTMASK (RESOLVE_UNDO_CHANGED | CACHE_TREE_CHANGED | \\\n \t\t CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \\\n-\t\t SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED)\n+\t\t SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | FSMONITOR_CHANGED)\n \n struct index_state the_index;\n static const char *alternate_index_output;\n@@ -61,6 +63,7 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache\n \tfree(old);\n \tset_index_entry(istate, nr, ce);\n \tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \tistate->cache_changed |= CE_ENTRY_CHANGED;\n }\n \n@@ -777,6 +780,7 @@ int chmod_index_entry(struct index_state *istate, struct cache_entry *ce,\n \t}\n \tcache_tree_invalidate_path(istate, ce->name);\n \tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \tistate->cache_changed |= CE_ENTRY_CHANGED;\n \n \treturn 0;\n@@ -1344,6 +1348,8 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \tconst char *added_fmt;\n \tconst char *unmerged_fmt;\n \n+\trefresh_by_fsmonitor(istate);\n+\n \tmodified_fmt = (in_porcelain ? \"M\\t%s\\n\" : \"%s: needs update\\n\");\n \tdeleted_fmt = (in_porcelain ? \"D\\t%s\\n\" : \"%s: needs update\\n\");\n \ttypechange_fmt = (in_porcelain ? \"T\\t%s\\n\" : \"%s needs update\\n\");\n@@ -1380,8 +1386,11 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\t\tcontinue;\n \n \t\tnew = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);\n-\t\tif (new == ce)\n+\t\tif (new == ce) {\n+\t\t\tce->ce_flags &= ~CE_FSMONITOR_DIRTY;\n \t\t\tcontinue;\n+\t\t}\n+\n \t\tif (!new) {\n \t\t\tconst char *fmt;\n \n@@ -1391,6 +1400,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\t\t\t */\n \t\t\t\tce->ce_flags &= ~CE_VALID;\n \t\t\t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\t\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\t\t\tistate->cache_changed |= CE_ENTRY_CHANGED;\n \t\t\t}\n \t\t\tif (quiet)\n@@ -1549,6 +1559,9 @@ static int read_index_extension(struct index_state *istate,\n \tcase CACHE_EXT_UNTRACKED:\n \t\tistate->untracked = read_untracked_extension(data, sz);\n \t\tbreak;\n+\tcase CACHE_EXT_FSMONITOR:\n+\t\tread_fsmonitor_extension(istate, data, sz);\n+\t\tbreak;\n \tdefault:\n \t\tif (*ext < 'A' || 'Z' < *ext)\n \t\t\treturn error(\"index uses %.4s extension, which we do not understand\",\n@@ -1721,6 +1734,7 @@ static void post_read_index_from(struct index_state *istate)\n \tcheck_ce_order(istate);\n \ttweak_untracked_cache(istate);\n \ttweak_split_index(istate);\n+\tprocess_fsmonitor_extension(istate);\n }\n \n /* remember to discard_cache() before reading a different cache! */\n@@ -2300,6 +2314,16 @@ static int do_write_index(struct index_state *istate, int newfd,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n+\tif (!strip_extensions && istate->fsmonitor_last_update) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\twrite_fsmonitor_extension(&sb, istate);\n+\t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_FSMONITOR, sb.len) < 0\n+\t\t\t|| ce_write(&c, newfd, sb.buf, sb.len) < 0;\n+\t\tstrbuf_release(&sb);\n+\t\tif (err)\n+\t\t\treturn -1;\n+\t}\n \n \tif (ce_flush(&c, newfd, istate->sha1) || fstat(newfd, &st))\n \t\treturn -1;\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex aa15111fef..259e6960b9 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -412,6 +412,7 @@ static int apply_sparse_checkout(struct index_state *istate,\n \t\tce->ce_flags &= ~CE_SKIP_WORKTREE;\n \tif (was_skip_worktree != ce_skip_worktree(ce)) {\n \t\tce->ce_flags |= CE_UPDATE_IN_BASE;\n+\t\tce->ce_flags |= CE_FSMONITOR_DIRTY;\n \t\tistate->cache_changed |= CE_ENTRY_CHANGED;\n \t}\n \n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321290","messageId":"20170601155105.28356-3-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 2/6] dir: make lookup_untracked() available outside of dir.c","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:01Z","receivedAt":"2017-06-01T15:51:35Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Remove the static qualifier from lookup_untracked() and make it\navailable to other modules by exporting it from dir.h. This will be\nused later when we need to find entries to mark 'fsmonitor dirty.'\n\nSigned-off-by: Ben Peart \n---\n dir.c | 2 +-\n dir.h | 3 +++\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/dir.c b/dir.c\nindex f451bfa48c..1b5558fdf9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -660,7 +660,7 @@ static void trim_trailing_spaces(char *buf)\n *\n * If \"name\" has the trailing slash, it'll be excluded in the search.\n */\n-static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n \t\t\t\t\t\t struct untracked_cache_dir *dir,\n \t\t\t\t\t\t const char *name, int len)\n {\ndiff --git a/dir.h b/dir.h\nindex bf23a470af..9e387551bd 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -339,4 +339,7 @@ extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git\n extern void relocate_gitdir(const char *path,\n \t\t\t const char *old_git_dir,\n \t\t\t const char *new_git_dir);\n+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n+\t\t\t\t\t struct untracked_cache_dir *dir,\n+\t\t\t\t\t const char *name, int len);\n #endif\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321291","messageId":"20170601155105.28356-6-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 5/6] fsmonitor: add documentation for the fsmonitor extension.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:04Z","receivedAt":"2017-06-01T15:51:39Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"This includes the core.fsmonitor setting, the query-fsmonitor hook,\nand the fsmonitor index extension.\n\nSigned-off-by: Ben Peart \n---\n Documentation/config.txt | 7 +++++++\n Documentation/githooks.txt | 23 +++++++++++++++++++++++\n Documentation/technical/index-format.txt | 19 +++++++++++++++++++\n 3 files changed, 49 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e0b9fd0bc3..5211388167 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -389,6 +389,13 @@ core.protectNTFS::\n \t8.3 \"short\" names.\n \tDefaults to `true` on Windows, and `false` elsewhere.\n \n+core.fsmonitor::\n+\tIf set to true, call the query-fsmonitor hook proc which will\n+\tidentify all files that may have had changes since the last\n+\trequest. This information is used to speed up operations like\n+\t'git commit' and 'git status' by limiting what git must scan to\n+\tdetect changes.\n+\n core.trustctime::\n \tIf false, the ctime differences between the index and the\n \tworking tree are ignored; useful when the inode change time\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 706091a569..48127e8729 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -448,6 +448,29 @@ The commits are guaranteed to be listed in the order that they were\n processed by rebase.\n \n \n+[[query-fsmonitor]]\n+query-fsmonitor\n+~~~~~~~~~~~~~~~\n+\n+This hook is invoked when the configuration option core.fsmonitor is\n+set and git needs to identify changed or untracked files. It takes\n+two arguments, a version (currently 1) and the time in elapsed\n+nanoseconds since midnight, January 1, 1970.\n+\n+The hook should output to stdout the list of all files in the working\n+directory that may have changed since the requested time. The logic\n+should be inclusive so that it does not miss any potential changes.\n+The paths should be relative to the root of the working directory\n+and be separated by a single NUL.\n+\n+Git will limit what files it checks for changes as well as which\n+directories are checked for untracked files based on the path names\n+given.\n+\n+The exit status determines whether git will use the data from the\n+hook to limit its search. On error, it will fall back to verifying\n+all files and folders.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt\nindex ade0b0c445..7aeeea6f94 100644\n--- a/Documentation/technical/index-format.txt\n+++ b/Documentation/technical/index-format.txt\n@@ -295,3 +295,22 @@ The remaining data of each directory block is grouped by type:\n in the previous ewah bitmap.\n \n - One NUL.\n+\n+== File System Monitor cache\n+\n+ The file system monitor cache tracks files for which the query-fsmonitor\n+ hook has told us about changes. The signature for this extension is\n+ { 'F', 'S', 'M', 'N' }.\n+\n+ The extension starts with\n+\n+ - 32-bit version number: the current supported version is 1.\n+\n+ - 64-bit time: the extension data reflects all changes through the given\n+\ttime which is stored as the nanoseconds elapsed since midnight,\n+\tJanuary 1, 1970.\n+\n+ - 32-bit bitmap size: the size of the CE_FSMONITOR_DIRTY bitmap.\n+\n+ - An ewah bitmap, the n-th bit indicates whether the n-th index entry\n+ is CE_FSMONITOR_DIRTY.\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321292","messageId":"20170601155105.28356-2-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 1/6] bswap: add 64 bit endianness helper get_be64","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:00Z","receivedAt":"2017-06-01T15:51:44Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Add a new get_be64 macro to enable 64 bit endian conversions on memory\nthat may or may not be aligned.\n\nSigned-off-by: Ben Peart \n---\n compat/bswap.h | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex d47c003544..f89fe7f4b5 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -158,6 +158,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n \n #define get_be16(p)\tntohs(*(unsigned short *)(p))\n #define get_be32(p)\tntohl(*(unsigned int *)(p))\n+#define get_be64(p)\tntohll(*(uint64_t *)(p))\n #define put_be32(p, v)\tdo { *(unsigned int *)(p) = htonl(v); } while (0)\n \n #else\n@@ -170,6 +171,9 @@ static inline uint64_t git_bswap64(uint64_t x)\n \t(*((unsigned char *)(p) + 1) << 16) | \\\n \t(*((unsigned char *)(p) + 2) << 8) | \\\n \t(*((unsigned char *)(p) + 3) << 0) )\n+#define get_be64(p)\t( \\\n+\t((uint64_t)get_be32((unsigned char *)(p) + 0) << 32) | \\\n+\t((uint64_t)get_be32((unsigned char *)(p) + 4) << 0)\n #define put_be32(p, v)\tdo { \\\n \tunsigned int __v = (v); \\\n \t*((unsigned char *)(p) + 0) = __v >> 24; \\\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321293","messageId":"20170601155105.28356-7-benpeart@microsoft.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"[PATCH v4 6/6] fsmonitor: add a sample query-fsmonitor hook script for Watchman","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T15:51:05Z","receivedAt":"2017-06-01T15:51:46Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"This hook script integrates the new fsmonitor capabilities of git with\nthe cross platform Watchman file watching service. To use the script:\n\nDownload and install Watchman from https://facebook.github.io/watchman/\nand instruct Watchman to watch your working directory for changes\n('watchman watch-project /usr/src/git').\n\nRename the sample integration hook from query-fsmonitor.sample to\nquery-fsmonitor.\n\nConfigure git to use the extension ('git config core.fsmonitor true')\nand optionally turn on the untracked cache for optimal performance\n('git config core.untrackedcache true').\n\nSigned-off-by: Ben Peart \nSigned-off-by: Johannes Schindelin \nSigned-off-by: �var Arnfj�r� Bjarmason \n---\n templates/hooks--query-fsmonitor.sample | 60 +++++++++++++++++++++++++++++++++\n 1 file changed, 60 insertions(+)\n create mode 100755 templates/hooks--query-fsmonitor.sample\n\ndiff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--query-fsmonitor.sample\nnew file mode 100755\nindex 0000000000..941c4c5b57\n--- /dev/null\n+++ b/templates/hooks--query-fsmonitor.sample\n@@ -0,0 +1,60 @@\n+#!/bin/sh\n+#\n+# An example hook script to integrate Watchman\n+# (https://facebook.github.io/watchman/) with git to provide fast\n+# git status.\n+#\n+# The hook is passed a version (currently 1) and a time in nanoseconds\n+# formatted as a string and outputs to stdout all files that have been\n+# modified since the given time. Paths must be relative to the root of\n+# the working tree and separated by a single NUL.\n+#\n+# To enable this hook, rename this file to \"query-fsmonitor\"\n+\n+# check the hook interface version\n+if [ $1 -eq 1 ]\n+then\n+\t# convert nanoseconds to seconds\n+\ttime_t=$(($2/1000000000))\n+else\n+\techo -e \"Unsupported query-fsmonitor hook version.\\nFalling back to scanning...\\n\" >&2\n+\texit 1;\n+fi\n+\n+# Convert unix style paths to escaped Windows style paths\n+case \"$(uname -s)\" in\n+MINGW*|MSYS_NT*)\n+ GIT_WORK_TREE=\"$(cygpath -aw \"$PWD\" | sed 's,\\\\,\\\\\\\\,g')\"\n+ ;;\n+*)\n+ GIT_WORK_TREE=\"$PWD\"\n+ ;;\n+esac\n+\n+# Query Watchman for all the changes since the requested time\n+echo \"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $time_t, \\\"fields\\\":[\\\"name\\\"]}]\" | \\\n+\twatchman -j |\n+\tperl -0666 -e '\n+\t\tuse strict;\n+\t\tuse warnings;\n+\n+\t\tmy $stdin = <>;\n+\t\tdie \"Watchman: command returned no output.\\nFalling back to scanning...\\n\" if $stdin eq \"\";\n+\t\tdie \"Watchman: command returned invalid output: $stdin\\nFalling back to scanning...\\n\" unless $stdin =~ /^\\{/;\n+\n+\t\tmy $json_pkg;\n+\t\teval {\n+\t\t\trequire JSON::XS;\n+\t\t\t$json_pkg = \"JSON::XS\";\n+\t\t\t1;\n+\t\t} or do {\n+\t\t\trequire JSON::PP;\n+\t\t\t$json_pkg = \"JSON::PP\";\n+\t\t};\n+\n+\t\tmy $o = $json_pkg->new->utf8->decode($stdin);\n+\t\tdie \"Watchman: $o->{error}.\\nFalling back to scanning...\\n\" if $o->{error};\n+\n+\t\tlocal $, = \"\\0\";\n+\t\tprint @{$o->{files}};\n+\t'\n-- \n2.13.0.windows.1.9.gc201c67b71\n\n"},{"id":"321321","messageId":"CACBZZX6+V6=gSs0pdN9WU+LPZv5qSAmE5u_f=NFYht2dT-hxEQ@mail.gmail.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-01T19:57:44Z","receivedAt":"2017-06-01T19:58:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 1, 2017 at 5:50 PM, Ben Peart wrote:\n> Changes from V3 include:\n> - update test script based on feedback\n> - update template hook proc with better post-processing code and make\n> it executable\n\nThanks, exciting stuff, do you have this pushed somewhere? I didn't\nspot it it in your github repo. I had some issues applying this on top\nof master @ 0339965c70, on 5/6 I got\n\n $ git am /tmp/original_msg.txt\n Applying: fsmonitor: add documentation for the fsmonitor extension.\n error: patch failed: Documentation/githooks.txt:448\n error: Documentation/githooks.txt: patch does not apply\n Patch failed at 0001 fsmonitor: add documentation for the\nfsmonitor extension.\n The copy of the patch that failed is found in: .git/rebase-apply/patch\n When you have resolved this problem, run \"git am --continue\".\n If you prefer to skip this patch, run \"git am --skip\" instead.\n To restore the original branch and stop patching, run \"git am --abort\".\n\nBut it worked with patch, weirdly enough:\n\n $ patch -p1 wrote:\n> Changes from V3 include:\n> - update test script based on feedback\n> - update template hook proc with better post-processing code and make\n> it executable\n\nI have watchman running finally, so aside from issues applying this I\ncan finally test this. I set up a watch of linux.git:\n\n$ watchman watch $PWD\n$ watchman watch-list|jq '.roots[]'\n\"/home/avar/g/linux\"\n\nAnd first, for simplicity, I'll be turning core.fsmonitore on later:\n\n$ for c in fsmonitor untrackedCache splitIndex; do git config core.$c\nfalse; done\n\nOn a hot fs cache running \"git status\" takes me 80ms, but if I flush caches:\n\n# free && sync && echo 3 > /proc/sys/vm/drop_caches && free\n\nRunning \"git status\" takes me 4s. Now, right after flushing those caches:\n\n$ date +%s\n1496349235\n$ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n\"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n\nreal 0m0.664s\nuser 0m0.000s\nsys 0m0.004s\n$ touch foo bar\n$ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n\"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n\"bar\"\n\"foo\"\n\nreal 0m0.044s\nuser 0m0.000s\nsys 0m0.000s\n\nWithout the monitor running \"git status' takes me ~2.5s, i.e. from cold cache:\n\n$ time GIT_TRACE=1 ~/g/git/git-status\n20:34:49.154731 git.c:369 trace: built-in: git 'status'\nOn branch master\nYour branch is up-to-date with 'origin/master'.\n\nUntracked files:\n (use \"git add ...\" to include in what will be committed)\n\n bar\n foo\n\nnothing added to commit but untracked files present (use \"git add\" to track)\n\nreal 0m2.546s\nuser 0m0.060s\nsys 0m0.228s\n\nNow, I would expect the case where the working tree isn't in the fs\ncache to be lightning fast, since the watchman command is really fast\n(flushed the cache again, turned on the fsmonitor):\n\n$ date +%s\n1496349523\n$ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n\"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n\nreal 0m0.529s\nuser 0m0.000s\nsys 0m0.004s\n$ touch foo bar\n$ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n\"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n\"bar\"\n\"foo\"\n\nreal 0m0.312s\nuser 0m0.000s\nsys 0m0.000s\n\nBut if I run \"git status\" (and I instrumented the hook to dump the\nchanged files) it takes a long time (those 10s are just the disk being\nslow, but it should be ~0s, right?):\n\n$ time GIT_TRACE=1 ~/g/git/git-status\n20:39:11.250128 git.c:369 trace: built-in: git 'status'\n20:39:14.586720 run-command.c:626 trace: run_command:\n'.git/hooks/query-fsmonitor' '1' '1496349494197821000'\nWatchman says these changed: bar foo hi there .git .git/index.lock .git/index\nOn branch master\nYour branch is up-to-date with 'origin/master'.\n\nUntracked files:\n (use \"git add ...\" to include in what will be committed)\n\n bar\n foo\n\nnothing added to commit but untracked files present (use \"git add\" to track)\n\nreal 0m10.791s\nuser 0m0.108s\nsys 0m0.228s\n\nIf I re-run that it takes ~160-200ms with the hook, 80-120ms without\nit, so in this case once the data is in the fscache watchman is\nactually slower.\n\nBut manually running watchman shows that it's really fast, usually\nreturning within 3ms, I flush the fscache in the middle of this,\nthat's the 600ms time, that could be some global kernel lock or\nsomething due to the odd manual proc operation, I haven't actually\ntested this on a system under memory pressure:\n\n$ for i in {1..10}; do (time (printf '[\"query\", \"/home/avar/g/linux\",\n{\"since\": %s, \"fields\":[\"name\"]}]' $(($(date +%s)-3)) | watchman -j |\njq '.files[]')) 2>&1 | grep -e '\"' -e ^real && touch $i && sleep 1;\ndone\nreal 0m0.004s\n\"1\"\nreal 0m0.003s\n\"2\"\n\"1\"\nreal 0m0.605s\n\"3\"\nreal 0m0.003s\n\"4\"\n\"3\"\nreal 0m0.003s\n\"5\"\n\"4\"\nreal 0m0.003s\n\"6\"\n\"5\"\nreal 0m0.003s\n\"7\"\n\"6\"\nreal 0m0.003s\n\"8\"\n\"7\"\nreal 0m0.003s\n\"9\"\n\"8\"\nreal 0m0.003s\n\nSo something really odd is going on here. This should be speeding up\n\"git status\" a lot, even with a hot fs cache doing stat on all of\nlinux.git takes a lot longer than 3ms, but for some reason it's\nslower.\n"},{"id":"321327","messageId":"020b09ef-6035-285d-c033-8831564d8752@gmail.com","threadId":"46090","inReplyTo":"CACBZZX6+V6=gSs0pdN9WU+LPZv5qSAmE5u_f=NFYht2dT-hxEQ@mail.gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-01T21:06:38Z","receivedAt":"2017-06-01T21:06:47Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 6/1/2017 3:57 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, Jun 1, 2017 at 5:50 PM, Ben Peart wrote:\n>> Changes from V3 include:\n>> - update test script based on feedback\n>> - update template hook proc with better post-processing code and make\n>> it executable\n> \n> Thanks, exciting stuff, do you have this pushed somewhere? I didn't\n> spot it it in your github repo. I had some issues applying this on top\n> of master @ 0339965c70, on 5/6 I got\n> \n\nI just pushed this to github at \nhttps://github.com/benpeart/git-for-windows/tree/fsmonitor\n\n> $ git am /tmp/original_msg.txt\n> Applying: fsmonitor: add documentation for the fsmonitor extension.\n> error: patch failed: Documentation/githooks.txt:448\n> error: Documentation/githooks.txt: patch does not apply\n> Patch failed at 0001 fsmonitor: add documentation for the\n> fsmonitor extension.\n> The copy of the patch that failed is found in: .git/rebase-apply/patch\n> When you have resolved this problem, run \"git am --continue\".\n> If you prefer to skip this patch, run \"git am --skip\" instead.\n> To restore the original branch and stop patching, run \"git am --abort\".\n> \n\nSorry, no idea on why this didn't work. The patch was formatted with \ngit format-patch but it's possible I've got something wrong.\n\n> But it worked with patch, weirdly enough:\n> \n> $ patch -p1 (Stripping trailing CRs from patch; use --binary to disable.)\n> patching file Documentation/config.txt\n> Hunk #1 succeeded at 410 (offset 21 lines).\n> (Stripping trailing CRs from patch; use --binary to disable.)\n> patching file Documentation/githooks.txt\n> Hunk #1 succeeded at 456 with fuzz 1 (offset 8 lines).\n> (Stripping trailing CRs from patch; use --binary to disable.)\n> patching file Documentation/technical/index-format.txt\n> \n> The 6/6 patch failed due to an unknown charset y, you have\n> \"Content-Type: text/plain; charset=y\" in the header, worked after\n> manually munging it to \"UTF-8\", although it gave a warning...\n> \n\nThe only thing I see different about this patch is the special \ncharacters of your name in the sign-off line. The call to git \nsend-email prompted me about encoding - I wonder if my answer was \nincorrect? Given you've probably dealt with your name in git patches \nbefore :), what should my answer be?\n"},{"id":"321329","messageId":"CACBZZX5g+t_7ViCggB32WBaARV1cz_dj3gCGEqjvjN3A-QJhiA@mail.gmail.com","threadId":"46090","inReplyTo":"020b09ef-6035-285d-c033-8831564d8752@gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-01T21:12:37Z","receivedAt":"2017-06-01T21:13:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 1, 2017 at 11:06 PM, Ben Peart wrote:\n> On 6/1/2017 3:57 PM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Thu, Jun 1, 2017 at 5:50 PM, Ben Peart wrote:\n>>>\n>>> Changes from V3 include:\n>>> - update test script based on feedback\n>>> - update template hook proc with better post-processing code and make\n>>> it executable\n>>\n>>\n>> Thanks, exciting stuff, do you have this pushed somewhere? I didn't\n>> spot it it in your github repo. I had some issues applying this on top\n>> of master @ 0339965c70, on 5/6 I got\n>>\n>\n> I just pushed this to github at\n> https://github.com/benpeart/git-for-windows/tree/fsmonitor\n>\n>> $ git am /tmp/original_msg.txt\n>> Applying: fsmonitor: add documentation for the fsmonitor extension.\n>> error: patch failed: Documentation/githooks.txt:448\n>> error: Documentation/githooks.txt: patch does not apply\n>> Patch failed at 0001 fsmonitor: add documentation for the\n>> fsmonitor extension.\n>> The copy of the patch that failed is found in:\n>> .git/rebase-apply/patch\n>> When you have resolved this problem, run \"git am --continue\".\n>> If you prefer to skip this patch, run \"git am --skip\" instead.\n>> To restore the original branch and stop patching, run \"git am\n>> --abort\".\n>>\n>\n> Sorry, no idea on why this didn't work. The patch was formatted with git\n> format-patch but it's possible I've got something wrong.\nNo idea what's going on there, anyway I can grab it from your github\nurl, thanks!\n>> But it worked with patch, weirdly enough:\n>>\n>> $ patch -p1 > (Stripping trailing CRs from patch; use --binary to disable.)\n>> patching file Documentation/config.txt\n>> Hunk #1 succeeded at 410 (offset 21 lines).\n>> (Stripping trailing CRs from patch; use --binary to disable.)\n>> patching file Documentation/githooks.txt\n>> Hunk #1 succeeded at 456 with fuzz 1 (offset 8 lines).\n>> (Stripping trailing CRs from patch; use --binary to disable.)\n>> patching file Documentation/technical/index-format.txt\n>>\n>> The 6/6 patch failed due to an unknown charset y, you have\n>> \"Content-Type: text/plain; charset=y\" in the header, worked after\n>> manually munging it to \"UTF-8\", although it gave a warning...\n>>\n>\n> The only thing I see different about this patch is the special characters of\n> your name in the sign-off line. The call to git send-email prompted me\n> about encoding - I wonder if my answer was incorrect? Given you've probably\n> dealt with your name in git patches before :), what should my answer be?\n\nHah! I didn't think that the \"y\" could be the result of \"yes\" in some\ninteractive dialog. It never prompts me when I send patches, it just\nworks, and with UTF-8 it seems to have been applied correctly.\n"},{"id":"321330","messageId":"CAGZ79ka1hjTFt9iRDrEy89ie9c3zj=BZ3SnZjPbLt6LGVDzeJQ@mail.gmail.com","threadId":"46090","inReplyTo":"020b09ef-6035-285d-c033-8831564d8752@gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-06-01T21:13:10Z","receivedAt":"2017-06-01T21:13:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">\n>> $ git am /tmp/original_msg.txt\n>> Applying: fsmonitor: add documentation for the fsmonitor extension.\n>> error: patch failed: Documentation/githooks.txt:448\n>> error: Documentation/githooks.txt: patch does not apply\n>> Patch failed at 0001 fsmonitor: add documentation for the\n>> fsmonitor extension.\n>> The copy of the patch that failed is found in:\n>> .git/rebase-apply/patch\n>> When you have resolved this problem, run \"git am --continue\".\n>> If you prefer to skip this patch, run \"git am --skip\" instead.\n>> To restore the original branch and stop patching, run \"git am\n>> --abort\".\n\nTry again with -3. (We should make that the default for am, maybe?)\nIt helped me for most of the conflicts that I saw.\n"},{"id":"321331","messageId":"CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com","threadId":"46090","inReplyTo":"CACBZZX5e58bWuf3NdDYTxu2KyZj29hHONzN=rp-7vXd8nURyWQ@mail.gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-01T21:13:52Z","receivedAt":"2017-06-01T21:14:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 1, 2017 at 10:51 PM, Ævar Arnfjörð Bjarmason\n wrote:\n> On Thu, Jun 1, 2017 at 5:50 PM, Ben Peart wrote:\n>> Changes from V3 include:\n>> - update test script based on feedback\n>> - update template hook proc with better post-processing code and make\n>> it executable\n>\n> I have watchman running finally, so aside from issues applying this I\n> can finally test this. I set up a watch of linux.git:\n>\n> $ watchman watch $PWD\n> $ watchman watch-list|jq '.roots[]'\n> \"/home/avar/g/linux\"\n>\n> And first, for simplicity, I'll be turning core.fsmonitore on later:\n>\n> $ for c in fsmonitor untrackedCache splitIndex; do git config core.$c\n> false; done\n>\n> On a hot fs cache running \"git status\" takes me 80ms, but if I flush caches:\n>\n> # free && sync && echo 3 > /proc/sys/vm/drop_caches && free\n>\n> Running \"git status\" takes me 4s. Now, right after flushing those caches:\n>\n> $ date +%s\n> 1496349235\n> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>\n> real 0m0.664s\n> user 0m0.000s\n> sys 0m0.004s\n> $ touch foo bar\n> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n> \"bar\"\n> \"foo\"\n>\n> real 0m0.044s\n> user 0m0.000s\n> sys 0m0.000s\n>\n> Without the monitor running \"git status' takes me ~2.5s, i.e. from cold cache:\n>\n> $ time GIT_TRACE=1 ~/g/git/git-status\n> 20:34:49.154731 git.c:369 trace: built-in: git 'status'\n> On branch master\n> Your branch is up-to-date with 'origin/master'.\n>\n> Untracked files:\n> (use \"git add ...\" to include in what will be committed)\n>\n> bar\n> foo\n>\n> nothing added to commit but untracked files present (use \"git add\" to track)\n>\n> real 0m2.546s\n> user 0m0.060s\n> sys 0m0.228s\n>\n> Now, I would expect the case where the working tree isn't in the fs\n> cache to be lightning fast, since the watchman command is really fast\n> (flushed the cache again, turned on the fsmonitor):\n>\n> $ date +%s\n> 1496349523\n> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>\n> real 0m0.529s\n> user 0m0.000s\n> sys 0m0.004s\n> $ touch foo bar\n> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n> \"bar\"\n> \"foo\"\n>\n> real 0m0.312s\n> user 0m0.000s\n> sys 0m0.000s\n>\n> But if I run \"git status\" (and I instrumented the hook to dump the\n> changed files) it takes a long time (those 10s are just the disk being\n> slow, but it should be ~0s, right?):\n>\n> $ time GIT_TRACE=1 ~/g/git/git-status\n> 20:39:11.250128 git.c:369 trace: built-in: git 'status'\n> 20:39:14.586720 run-command.c:626 trace: run_command:\n> '.git/hooks/query-fsmonitor' '1' '1496349494197821000'\n> Watchman says these changed: bar foo hi there .git .git/index.lock .git/index\n> On branch master\n> Your branch is up-to-date with 'origin/master'.\n>\n> Untracked files:\n> (use \"git add ...\" to include in what will be committed)\n>\n> bar\n> foo\n>\n> nothing added to commit but untracked files present (use \"git add\" to track)\n>\n> real 0m10.791s\n> user 0m0.108s\n> sys 0m0.228s\n>\n> If I re-run that it takes ~160-200ms with the hook, 80-120ms without\n> it, so in this case once the data is in the fscache watchman is\n> actually slower.\n>\n> But manually running watchman shows that it's really fast, usually\n> returning within 3ms, I flush the fscache in the middle of this,\n> that's the 600ms time, that could be some global kernel lock or\n> something due to the odd manual proc operation, I haven't actually\n> tested this on a system under memory pressure:\n>\n> $ for i in {1..10}; do (time (printf '[\"query\", \"/home/avar/g/linux\",\n> {\"since\": %s, \"fields\":[\"name\"]}]' $(($(date +%s)-3)) | watchman -j |\n> jq '.files[]')) 2>&1 | grep -e '\"' -e ^real && touch $i && sleep 1;\n> done\n> real 0m0.004s\n> \"1\"\n> real 0m0.003s\n> \"2\"\n> \"1\"\n> real 0m0.605s\n> \"3\"\n> real 0m0.003s\n> \"4\"\n> \"3\"\n> real 0m0.003s\n> \"5\"\n> \"4\"\n> real 0m0.003s\n> \"6\"\n> \"5\"\n> real 0m0.003s\n> \"7\"\n> \"6\"\n> real 0m0.003s\n> \"8\"\n> \"7\"\n> real 0m0.003s\n> \"9\"\n> \"8\"\n> real 0m0.003s\n>\n> So something really odd is going on here. This should be speeding up\n> \"git status\" a lot, even with a hot fs cache doing stat on all of\n> linux.git takes a lot longer than 3ms, but for some reason it's\n> slower.\n\nDug a bit further, with cold cache and no fsmonitor, gprof output from\n-O0 compiled git:\n\nEach sample counts as 0.01 seconds.\n % cumulative self self total\n time seconds seconds calls ms/call ms/call name\n 37.50 0.03 0.03 123886 0.00 0.00 memihash\n 12.50 0.04 0.01 284727 0.00 0.00 match_basename\n 12.50 0.05 0.01 73989 0.00 0.00 match_pathname\n 12.50 0.06 0.01 59924 0.00 0.00 hashmap_add\n 12.50 0.07 0.01 59847 0.00 0.00 same_name\n 12.50 0.08 0.01 59844 0.00 0.00 hash_index_entry\n 0.00 0.08 0.00 598446 0.00 0.00 git_bswap32\n 0.00 0.08 0.00 259150 0.00 0.00 fspathncmp\n 0.00 0.08 0.00 178731 0.00 0.00 match_pathspec\n 0.00 0.08 0.00 178655 0.00 0.00 do_match_pathspec\nAnd then cold cache with fsmonitor:\n\nEach sample counts as 0.01 seconds.\n % cumulative self self total\n time seconds seconds calls ms/call ms/call name\n 66.67 0.04 0.04 96696 0.00 0.00 blk_SHA1_Block\n 16.67 0.05 0.01 59844 0.00 0.00 cache_entry_from_ondisk\n 16.67 0.06 0.01 58869 0.00 0.00 longest_path_match\n 0.00 0.06 0.00 1547157 0.00 0.00 git_bswap32\n 0.00 0.06 0.00 1196893 0.00 0.00 git_bswap32\n 0.00 0.06 0.00 284727 0.00 0.00 match_basename\n 0.00 0.06 0.00 259150 0.00 0.00 fspathncmp\n 0.00 0.06 0.00 178561 0.00 0.00 do_match_pathspec\n 0.00 0.06 0.00 178543 0.00 0.00 match_pathspec\n 0.00 0.06 0.00 145141 0.00 0.00 memory_limit_check\n"},{"id":"321365","messageId":"4c3fed40-774b-8ae6-fa1b-50efe6ef552f@gmail.com","threadId":"46090","inReplyTo":"CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-02T00:40:20Z","receivedAt":"2017-06-02T00:40:27Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Any chance you can provide me with a bash script that contains the exact \nsequence of commands you are running to get this result? I've been \ntrying to replicate it using your notes but have not been able to. I'd \nlike to see if it is a repo difference, a platform difference, a command \nsequence difference (or something else entirely :)).\n\nThanks,\n\nBen\n\nOn 6/1/2017 5:13 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Thu, Jun 1, 2017 at 10:51 PM, Ævar Arnfjörð Bjarmason\n> wrote:\n>> On Thu, Jun 1, 2017 at 5:50 PM, Ben Peart wrote:\n>>> Changes from V3 include:\n>>> - update test script based on feedback\n>>> - update template hook proc with better post-processing code and make\n>>> it executable\n>>\n>> I have watchman running finally, so aside from issues applying this I\n>> can finally test this. I set up a watch of linux.git:\n>>\n>> $ watchman watch $PWD\n>> $ watchman watch-list|jq '.roots[]'\n>> \"/home/avar/g/linux\"\n>>\n>> And first, for simplicity, I'll be turning core.fsmonitore on later:\n>>\n>> $ for c in fsmonitor untrackedCache splitIndex; do git config core.$c\n>> false; done\n>>\n>> On a hot fs cache running \"git status\" takes me 80ms, but if I flush caches:\n>>\n>> # free && sync && echo 3 > /proc/sys/vm/drop_caches && free\n>>\n>> Running \"git status\" takes me 4s. Now, right after flushing those caches:\n>>\n>> $ date +%s\n>> 1496349235\n>> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n>> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>>\n>> real 0m0.664s\n>> user 0m0.000s\n>> sys 0m0.004s\n>> $ touch foo bar\n>> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349235,\n>> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>> \"bar\"\n>> \"foo\"\n>>\n>> real 0m0.044s\n>> user 0m0.000s\n>> sys 0m0.000s\n>>\n>> Without the monitor running \"git status' takes me ~2.5s, i.e. from cold cache:\n>>\n>> $ time GIT_TRACE=1 ~/g/git/git-status\n>> 20:34:49.154731 git.c:369 trace: built-in: git 'status'\n>> On branch master\n>> Your branch is up-to-date with 'origin/master'.\n>>\n>> Untracked files:\n>> (use \"git add ...\" to include in what will be committed)\n>>\n>> bar\n>> foo\n>>\n>> nothing added to commit but untracked files present (use \"git add\" to track)\n>>\n>> real 0m2.546s\n>> user 0m0.060s\n>> sys 0m0.228s\n>>\n>> Now, I would expect the case where the working tree isn't in the fs\n>> cache to be lightning fast, since the watchman command is really fast\n>> (flushed the cache again, turned on the fsmonitor):\n>>\n>> $ date +%s\n>> 1496349523\n>> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n>> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>>\n>> real 0m0.529s\n>> user 0m0.000s\n>> sys 0m0.004s\n>> $ touch foo bar\n>> $ time (echo '[\"query\", \"/home/avar/g/linux\", {\"since\": 1496349523,\n>> \"fields\":[\"name\"]}]' | watchman -j) | jq '.files[]'\n>> \"bar\"\n>> \"foo\"\n>>\n>> real 0m0.312s\n>> user 0m0.000s\n>> sys 0m0.000s\n>>\n>> But if I run \"git status\" (and I instrumented the hook to dump the\n>> changed files) it takes a long time (those 10s are just the disk being\n>> slow, but it should be ~0s, right?):\n>>\n>> $ time GIT_TRACE=1 ~/g/git/git-status\n>> 20:39:11.250128 git.c:369 trace: built-in: git 'status'\n>> 20:39:14.586720 run-command.c:626 trace: run_command:\n>> '.git/hooks/query-fsmonitor' '1' '1496349494197821000'\n>> Watchman says these changed: bar foo hi there .git .git/index.lock .git/index\n>> On branch master\n>> Your branch is up-to-date with 'origin/master'.\n>>\n>> Untracked files:\n>> (use \"git add ...\" to include in what will be committed)\n>>\n>> bar\n>> foo\n>>\n>> nothing added to commit but untracked files present (use \"git add\" to track)\n>>\n>> real 0m10.791s\n>> user 0m0.108s\n>> sys 0m0.228s\n>>\n>> If I re-run that it takes ~160-200ms with the hook, 80-120ms without\n>> it, so in this case once the data is in the fscache watchman is\n>> actually slower.\n>>\n>> But manually running watchman shows that it's really fast, usually\n>> returning within 3ms, I flush the fscache in the middle of this,\n>> that's the 600ms time, that could be some global kernel lock or\n>> something due to the odd manual proc operation, I haven't actually\n>> tested this on a system under memory pressure:\n>>\n>> $ for i in {1..10}; do (time (printf '[\"query\", \"/home/avar/g/linux\",\n>> {\"since\": %s, \"fields\":[\"name\"]}]' $(($(date +%s)-3)) | watchman -j |\n>> jq '.files[]')) 2>&1 | grep -e '\"' -e ^real && touch $i && sleep 1;\n>> done\n>> real 0m0.004s\n>> \"1\"\n>> real 0m0.003s\n>> \"2\"\n>> \"1\"\n>> real 0m0.605s\n>> \"3\"\n>> real 0m0.003s\n>> \"4\"\n>> \"3\"\n>> real 0m0.003s\n>> \"5\"\n>> \"4\"\n>> real 0m0.003s\n>> \"6\"\n>> \"5\"\n>> real 0m0.003s\n>> \"7\"\n>> \"6\"\n>> real 0m0.003s\n>> \"8\"\n>> \"7\"\n>> real 0m0.003s\n>> \"9\"\n>> \"8\"\n>> real 0m0.003s\n>>\n>> So something really odd is going on here. This should be speeding up\n>> \"git status\" a lot, even with a hot fs cache doing stat on all of\n>> linux.git takes a lot longer than 3ms, but for some reason it's\n>> slower.\n> \n> Dug a bit further, with cold cache and no fsmonitor, gprof output from\n> -O0 compiled git:\n> \n> Each sample counts as 0.01 seconds.\n> % cumulative self self total\n> time seconds seconds calls ms/call ms/call name\n> 37.50 0.03 0.03 123886 0.00 0.00 memihash\n> 12.50 0.04 0.01 284727 0.00 0.00 match_basename\n> 12.50 0.05 0.01 73989 0.00 0.00 match_pathname\n> 12.50 0.06 0.01 59924 0.00 0.00 hashmap_add\n> 12.50 0.07 0.01 59847 0.00 0.00 same_name\n> 12.50 0.08 0.01 59844 0.00 0.00 hash_index_entry\n> 0.00 0.08 0.00 598446 0.00 0.00 git_bswap32\n> 0.00 0.08 0.00 259150 0.00 0.00 fspathncmp\n> 0.00 0.08 0.00 178731 0.00 0.00 match_pathspec\n> 0.00 0.08 0.00 178655 0.00 0.00 do_match_pathspec\n> And then cold cache with fsmonitor:\n> \n> Each sample counts as 0.01 seconds.\n> % cumulative self self total\n> time seconds seconds calls ms/call ms/call name\n> 66.67 0.04 0.04 96696 0.00 0.00 blk_SHA1_Block\n> 16.67 0.05 0.01 59844 0.00 0.00 cache_entry_from_ondisk\n> 16.67 0.06 0.01 58869 0.00 0.00 longest_path_match\n> 0.00 0.06 0.00 1547157 0.00 0.00 git_bswap32\n> 0.00 0.06 0.00 1196893 0.00 0.00 git_bswap32\n> 0.00 0.06 0.00 284727 0.00 0.00 match_basename\n> 0.00 0.06 0.00 259150 0.00 0.00 fspathncmp\n> 0.00 0.06 0.00 178561 0.00 0.00 do_match_pathspec\n> 0.00 0.06 0.00 178543 0.00 0.00 match_pathspec\n> 0.00 0.06 0.00 145141 0.00 0.00 memory_limit_check\n> \n"},{"id":"321377","messageId":"xmqqmv9r17ah.fsf@gitster.mtv.corp.google.com","threadId":"46090","inReplyTo":"20170601155105.28356-1-benpeart@microsoft.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-02T01:56:38Z","receivedAt":"2017-06-02T01:56:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> Changes from V3 include:\n> - update test script based on feedback\n> - update template hook proc with better post-processing code and make\n> it executable\n\nI take it that the first three patches are the same as before,\nso is the \"add documentation\" one [5/6].\n\nWill replace; Ævar seems to have some interesting numbers, so I'll\nwait a bit more to see how the discussion goes.\n\nThanks.\n"},{"id":"321399","messageId":"20170602102853.23073-1-avarab@gmail.com","threadId":"46090","inReplyTo":"4c3fed40-774b-8ae6-fa1b-50efe6ef552f@gmail.com","subject":"[WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-02T10:28:53Z","receivedAt":"2017-06-02T10:29:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a performance test for the new core.fsmonitor facility using the\nsample query-fsmonitor hook.\n\nThis is WIP code for the reasons explained in the setup comments,\nunfortunately the perf code doesn't easily allow you to run different\nsetup code for different versions you're testing. This test will stop\nworking if the fsmonitor is merged into the master branch.\n\nOutput against linxu.git:\n\n $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8' ./run origin/master avar/fsmonitor ./p7519-fsmonitor.sh\n [...]\n Test origin/master avar/fsmonitor\n -----------------------------------------------------------------------\n 7519.2: status (first) 0.08(0.04+0.09) 0.12(0.07+0.10) +50.0%\n 7519.3: status (subsequent) 0.08(0.04+0.09) 0.12(0.06+0.11) +50.0%\n 7519.4: status -uno 0.02(0.02+0.05) 0.06(0.05+0.06) +200.0%\n 7519.5: status -uall 0.08(0.06+0.07) 0.12(0.07+0.10) +50.0%\n\nAnd against a larger in-house monorepo I have here, with the same\noptions (except the repo path):\n\n Test origin/master avar/fsmonitor\n -----------------------------------------------------------------------\n 7519.2: status (first) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n 7519.3: status (subsequent) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n 7519.4: status -uno 0.04(0.03+0.10) 0.22(0.08+0.12) +450.0%\n 7519.5: status -uall 0.20(0.13+0.16) 0.27(0.18+0.19) +35.0%\n\nAgainst linux.git with a hack to flush the FS cache (on Linux) before\nrunning the first 'git status', only running one test so the result\nisn't discarded as the slowest of N:\n\n $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee /proc/sys/vm/drop_caches >/dev/null && make -j8' ./run origin/master avar/fsmonitor ./p7519-fsmonitor.sh\n [...]\n Test origin/master avar/fsmonitor\n ------------------------------------------------------------------------\n 7519.2: status (first) 0.30(0.18+0.10) 8.26(0.22+0.10) +2653.3%\n 7519.3: status (subsequent) 0.08(0.04+0.08) 0.81(0.09+0.07) +912.5%\n 7519.4: status -uno 0.02(0.01+0.06) 0.08(0.04+0.07) +300.0%\n 7519.5: status -uall 0.08(0.06+0.07) 0.15(0.08+0.09) +87.5%\n\nNow obviously due to 1 run that has a lot of noise, but I would expect\nthat first invocation to be blindingly fast since watchman has info on\nwhat files were modified since the cache was flushed.\n\nThe same on the large monorepo noted above:\n\n Test origin/master avar/fsmonitor\n -----------------------------------------------------------------------\n 7519.2: status (first) 0.59(0.28+0.24) 0.93(0.35+0.19) +57.6%\n 7519.3: status (subsequent) 0.20(0.10+0.19) 0.28(0.16+0.20) +40.0%\n 7519.4: status -uno 0.04(0.04+0.09) 0.11(0.08+0.12) +175.0%\n 7519.5: status -uall 0.29(0.11+0.18) 0.40(0.16+0.19) +37.9%\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\n\nOn Fri, Jun 2, 2017 at 2:40 AM, Ben Peart wrote:\n> Any chance you can provide me with a bash script that contains the exact\n> sequence of commands you are running to get this result?  I've been trying\n> to replicate it using your notes but have not been able to.  I'd like to see\n> if it is a repo difference, a platform difference, a command sequence\n> difference (or something else entirely :)).\n\nI can do better than that, here's a new perf test on top of this\nseries which demonstates the issue. I've only tested this on Linux\n4.9.0 with watchman 4.9.0 compiled from git (yes, they're\ncoincidentally the same version).\n\nA good addition to this would be `printf | watchman -j` as noted in my earlier mail, but I ran out of\ntime.\n\nYou can also set any combination of GIT_PERF_7519_UNTRACKED_CACHE &\nGIT_PERF_7519_SPLIT_INDEX to play with turning that on. I haven't\ntested all combinations of that, but e.g. testing with untrackedCache\ndidn't give results that looked different from the performance\nregressions noted above.\n\nAside from performance, I think a very good addition to stress-test\nthis series would be a patch to t/test-lib*sh guarded by some env flag\nto do a similar watchman watch-del/watch/watch-list dance as the one\nI'm doing here in the setup, and setting up the hook / config.\n\nThat would allow testing the entire git test suite with this feature,\nto find any subtle bugs this might have introduced in git-status.\n\n t/perf/p7519-fsmonitor.sh | 58 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 58 insertions(+)\n create mode 100755 t/perf/p7519-fsmonitor.sh\n\ndiff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh\nnew file mode 100755\nindex 0000000000..b838a0ff14\n--- /dev/null\n+++ b/t/perf/p7519-fsmonitor.sh\n@@ -0,0 +1,58 @@\n+#!/bin/sh\n+\n+test_description=\"Test core.fsmonitor\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+test_expect_success 'setup' '\n+\t# Maybe set untrackedCache & splitIndex depending on the\n+\t# environment, defaulting to false.\n+\tif test -n \"$GIT_PERF_7519_UNTRACKED_CACHE\"\n+\tthen\n+\t\tgit config core.untrackedCache true\n+\telse\n+\t\tgit config core.untrackedCache false\n+\tfi &&\n+\tif test -n \"$GIT_PERF_7519_SPLIT_INDEX\"\n+\tthen\n+\t\tgit config core.splitIndex true\n+\telse\n+\t\tgit config core.splitIndex false\n+\tfi &&\n+\n+\t# Relies on core.fsmonitor not being merged into master. Needs\n+\t# better per-test ways to disable it if it gets merged.\n+\tgit config core.fsmonitor true &&\n+\n+\t# Hook scaffolding\n+\tmkdir .git/hooks &&\n+\tcp ../../../templates/hooks--query-fsmonitor.sample .git/hooks/query-fsmonitor &&\n+\n+\t# Setup watchman & ensure it is actually watching\n+\twatchman watch-del \"$PWD\" >/dev/null 2>&1 &&\n+\twatchman watch \"$PWD\" >/dev/null 2>&1 &&\n+\twatchman watch-list | grep -q -F \"$PWD\"\n+'\n+\n+# Setting:\n+#\n+# GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee /proc/sys/vm/drop_caches && make -j8'\n+#\n+# Can be used as a hack to performance test 'git status' on a cold fs\n+# cache with an existing watchman watching the directory, which should\n+# be blindingly fast, compared to amazingly slow without watchman.\n+test_perf 'status (first)' 'git status'\n+\n+\n+# The same git-status once the fs cache has been warmed, if using the\n+# GIT_PERF_MAKE_COMMAND above. Otherwise the same as above.\n+test_perf 'status (subsequent)' 'git status'\n+\n+# Let's see if -uno & -uall make any difference\n+test_perf 'status -uno' 'git status -uno'\n+test_perf 'status -uall' 'git status -uall'\n+\n+test_done\n-- \n2.13.0.506.g27d5fe0cd\n\n"},{"id":"321444","messageId":"75ecd288047648178ed93105ee1f7a9e@exmbdft7.ad.twosigma.com","threadId":"46090","inReplyTo":"20170602102853.23073-1-avarab@gmail.com","subject":"RE: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-06-02T21:44:29Z","receivedAt":"2017-06-02T21:44:42Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"BTW, a medium-sized (~250k files across 40k dirs) synthetic repo is available over bittorrent at:\nhttp://bitmover.com/2015-04-03-1M-git-bare.tar.bz2.torrent\n\nI tried Ævar's perf test with that (on a beefy laptop with SSD), and got significantly slower results with bp/fsmonitor:\nTest origin/master bp/fsmonitor \n-----------------------------------------------------------------------\n7519.2: status (first) 0.32(0.23+0.39) 0.32(0.26+0.36) +0.0% \n7519.3: status (subsequent) 0.18(0.14+0.34) 0.32(0.24+0.37) +77.8% \n7519.4: status -uno 0.11(0.08+0.32) 0.24(0.18+0.34) +118.2%\n7519.5: status -uall 0.49(0.22+0.56) 0.62(0.36+0.55) +26.5% \n7519.2: status (first) 0.32(0.23+0.39) 0.32(0.26+0.36) +0.0% \n7519.3: status (subsequent) 0.18(0.14+0.34) 0.32(0.24+0.37) +77.8% \n7519.4: status -uno 0.11(0.08+0.32) 0.24(0.18+0.34) +118.2%\n7519.5: status -uall 0.49(0.22+0.56) 0.62(0.36+0.55) +26.5% \n\nI have not yet looked into why this is.\n\n> -----Original Message-----\n> From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com]\n> Sent: Friday, June 2, 2017 6:29 AM\n> To: git@vger.kernel.org\n> Cc: Junio C Hamano ; Ben Peart\n> ; Nguyễn Thái Ngọc Duy ;\n> Johannes Schindelin ; David Turner\n> ; Jeff King ; Christian\n> Couder ; Ævar Arnfjörð Bjarmason\n> \n> Subject: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor\n> \n> Add a performance test for the new core.fsmonitor facility using the sample\n> query-fsmonitor hook.\n> \n> This is WIP code for the reasons explained in the setup comments,\n> unfortunately the perf code doesn't easily allow you to run different setup\n> code for different versions you're testing. This test will stop working if the\n> fsmonitor is merged into the master branch.\n> \n> Output against linxu.git:\n> \n> $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux\n> GIT_PERF_MAKE_OPTS='-j8' ./run origin/master avar/fsmonitor ./p7519-\n> fsmonitor.sh\n> [...]\n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.08(0.04+0.09) 0.12(0.07+0.10) +50.0%\n> 7519.3: status (subsequent) 0.08(0.04+0.09) 0.12(0.06+0.11) +50.0%\n> 7519.4: status -uno 0.02(0.02+0.05) 0.06(0.05+0.06) +200.0%\n> 7519.5: status -uall 0.08(0.06+0.07) 0.12(0.07+0.10) +50.0%\n> \n> And against a larger in-house monorepo I have here, with the same options\n> (except the repo path):\n> \n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n> 7519.3: status (subsequent) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n> 7519.4: status -uno 0.04(0.03+0.10) 0.22(0.08+0.12) +450.0%\n> 7519.5: status -uall 0.20(0.13+0.16) 0.27(0.18+0.19) +35.0%\n> \n> Against linux.git with a hack to flush the FS cache (on Linux) before running\n> the first 'git status', only running one test so the result isn't discarded as the\n> slowest of N:\n> \n> $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_LARGE_REPO=~/g/linux\n> GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee\n> /proc/sys/vm/drop_caches >/dev/null && make -j8' ./run origin/master\n> avar/fsmonitor ./p7519-fsmonitor.sh\n> [...]\n> Test origin/master avar/fsmonitor\n> ------------------------------------------------------------------------\n> 7519.2: status (first) 0.30(0.18+0.10) 8.26(0.22+0.10) +2653.3%\n> 7519.3: status (subsequent) 0.08(0.04+0.08) 0.81(0.09+0.07) +912.5%\n> 7519.4: status -uno 0.02(0.01+0.06) 0.08(0.04+0.07) +300.0%\n> 7519.5: status -uall 0.08(0.06+0.07) 0.15(0.08+0.09) +87.5%\n> \n> Now obviously due to 1 run that has a lot of noise, but I would expect that\n> first invocation to be blindingly fast since watchman has info on what files\n> were modified since the cache was flushed.\n> \n> The same on the large monorepo noted above:\n> \n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.59(0.28+0.24) 0.93(0.35+0.19) +57.6%\n> 7519.3: status (subsequent) 0.20(0.10+0.19) 0.28(0.16+0.20) +40.0%\n> 7519.4: status -uno 0.04(0.04+0.09) 0.11(0.08+0.12) +175.0%\n> 7519.5: status -uall 0.29(0.11+0.18) 0.40(0.16+0.19) +37.9%\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n> \n> On Fri, Jun 2, 2017 at 2:40 AM, Ben Peart wrote:\n> > Any chance you can provide me with a bash script that contains the\n> > exact sequence of commands you are running to get this result?  I've\n> > been trying to replicate it using your notes but have not been able\n> > to.  I'd like to see if it is a repo difference, a platform\n> > difference, a command sequence difference (or something else entirely\n> :)).\n> \n> I can do better than that, here's a new perf test on top of this series which\n> demonstates the issue. I've only tested this on Linux\n> 4.9.0 with watchman 4.9.0 compiled from git (yes, they're coincidentally the\n> same version).\n> \n> A good addition to this would be `printf past> | watchman -j` as noted in my earlier mail, but I ran out of\n> time.\n> \n> You can also set any combination of GIT_PERF_7519_UNTRACKED_CACHE &\n> GIT_PERF_7519_SPLIT_INDEX to play with turning that on. I haven't tested all\n> combinations of that, but e.g. testing with untrackedCache didn't give results\n> that looked different from the performance regressions noted above.\n> \n> Aside from performance, I think a very good addition to stress-test this series\n> would be a patch to t/test-lib*sh guarded by some env flag to do a similar\n> watchman watch-del/watch/watch-list dance as the one I'm doing here in\n> the setup, and setting up the hook / config.\n> \n> That would allow testing the entire git test suite with this feature, to find any\n> subtle bugs this might have introduced in git-status.\n> \n> t/perf/p7519-fsmonitor.sh | 58\n> +++++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 58 insertions(+)\n> create mode 100755 t/perf/p7519-fsmonitor.sh\n> \n> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh new file\n> mode 100755 index 0000000000..b838a0ff14\n> --- /dev/null\n> +++ b/t/perf/p7519-fsmonitor.sh\n> @@ -0,0 +1,58 @@\n> +#!/bin/sh\n> +\n> +test_description=\"Test core.fsmonitor\"\n> +\n> +. ./perf-lib.sh\n> +\n> +test_perf_large_repo\n> +test_checkout_worktree\n> +\n> +test_expect_success 'setup' '\n> +\t# Maybe set untrackedCache & splitIndex depending on the\n> +\t# environment, defaulting to false.\n> +\tif test -n \"$GIT_PERF_7519_UNTRACKED_CACHE\"\n> +\tthen\n> +\t\tgit config core.untrackedCache true\n> +\telse\n> +\t\tgit config core.untrackedCache false\n> +\tfi &&\n> +\tif test -n \"$GIT_PERF_7519_SPLIT_INDEX\"\n> +\tthen\n> +\t\tgit config core.splitIndex true\n> +\telse\n> +\t\tgit config core.splitIndex false\n> +\tfi &&\n> +\n> +\t# Relies on core.fsmonitor not being merged into master. Needs\n> +\t# better per-test ways to disable it if it gets merged.\n> +\tgit config core.fsmonitor true &&\n> +\n> +\t# Hook scaffolding\n> +\tmkdir .git/hooks &&\n> +\tcp ../../../templates/hooks--query-fsmonitor.sample\n> +.git/hooks/query-fsmonitor &&\n> +\n> +\t# Setup watchman & ensure it is actually watching\n> +\twatchman watch-del \"$PWD\" >/dev/null 2>&1 &&\n> +\twatchman watch \"$PWD\" >/dev/null 2>&1 &&\n> +\twatchman watch-list | grep -q -F \"$PWD\"\n> +'\n> +\n> +# Setting:\n> +#\n> +# GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_COMMAND='sudo sync\n> && echo 3 | sudo tee /proc/sys/vm/drop_caches && make -j8'\n> +#\n> +# Can be used as a hack to performance test 'git status' on a cold fs #\n> +cache with an existing watchman watching the directory, which should #\n> +be blindingly fast, compared to amazingly slow without watchman.\n> +test_perf 'status (first)' 'git status'\n> +\n> +\n> +# The same git-status once the fs cache has been warmed, if using the #\n> +GIT_PERF_MAKE_COMMAND above. Otherwise the same as above.\n> +test_perf 'status (subsequent)' 'git status'\n> +\n> +# Let's see if -uno & -uall make any difference\n> +test_perf 'status -uno' 'git status -uno'\n> +test_perf 'status -uall' 'git status -uall'\n> +\n> +test_done\n> --\n> 2.13.0.506.g27d5fe0cd\n\n"},{"id":"321448","messageId":"c84864d3-9823-0b02-a337-29592b7c01f3@gmail.com","threadId":"46090","inReplyTo":"20170602102853.23073-1-avarab@gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-02T22:05:28Z","receivedAt":"2017-06-02T22:05:37Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 6/2/2017 6:28 AM, Ævar Arnfjörð Bjarmason wrote:\n> Add a performance test for the new core.fsmonitor facility using the\n> sample query-fsmonitor hook.\n> \n> This is WIP code for the reasons explained in the setup comments,\n> unfortunately the perf code doesn't easily allow you to run different\n> setup code for different versions you're testing. This test will stop\n> working if the fsmonitor is merged into the master branch.\n> \n> Output against linxu.git:\n> \n> $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8' ./run origin/master avar/fsmonitor ./p7519-fsmonitor.sh\n> [...]\n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.08(0.04+0.09) 0.12(0.07+0.10) +50.0%\n> 7519.3: status (subsequent) 0.08(0.04+0.09) 0.12(0.06+0.11) +50.0%\n> 7519.4: status -uno 0.02(0.02+0.05) 0.06(0.05+0.06) +200.0%\n> 7519.5: status -uall 0.08(0.06+0.07) 0.12(0.07+0.10) +50.0%\n> \n\nWith regular status times this low, the overhead of calling the hook + \nwatchman + perl results in slower overall times as I noted in my initial \ncover letter. If status calls are already this fast, fsmonitor + \nwatchman isn't needed and obviously doesn't help.\n\nThis does highlight an optimization I could add. Even though -uno is \npassed, the fsmonitor code currently still goes through the work of \nmarking the untracked entries as dirty. I'll look at optimizing that \nout to speed status up when using that option.\n\n> And against a larger in-house monorepo I have here, with the same\n> options (except the repo path):\n> \n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n> 7519.3: status (subsequent) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n> 7519.4: status -uno 0.04(0.03+0.10) 0.22(0.08+0.12) +450.0%\n> 7519.5: status -uall 0.20(0.13+0.16) 0.27(0.18+0.19) +35.0%\n> \n> Against linux.git with a hack to flush the FS cache (on Linux) before\n> running the first 'git status', only running one test so the result\n> isn't discarded as the slowest of N:\n> \n\nI don't know know about on Linux but with Windows, when you flush the \nfile system cache via unmount/mount, it causes Watchman to do a full \nscan with the next query. This has a significant negative performance \nimpact on the next status call as it returns the set of _all_ files \nwhich git then uses to find and mark all index and untracked cache \nentries as fsmonitor dirty then does a complete scan. This combination \nmakes the first status after these events slower than without Watchman.\n\nI'm currently working with the Watchman team to return a code indicating \nwe should just scan everything ourselves to avoid the extra overhead, \nbut that solution is TBD. Given it only happens the first time a query \nis done on a new clone or the first time after watchman is started, I \ndidn't intend to hold up the patch series for it but will submit an \nenhanced version once a solution is available. The enhanced version \nwill then be the same performance (for the first status call) as when \nnot running with fsmonitor - not faster - as the state is unknown so \nmust be gathered from the working directory.\n\n> $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee /proc/sys/vm/drop_caches >/dev/null && make -j8' ./run origin/master avar/fsmonitor ./p7519-fsmonitor.sh\n> [...]\n> Test origin/master avar/fsmonitor\n> ------------------------------------------------------------------------\n> 7519.2: status (first) 0.30(0.18+0.10) 8.26(0.22+0.10) +2653.3%\n> 7519.3: status (subsequent) 0.08(0.04+0.08) 0.81(0.09+0.07) +912.5%\n> 7519.4: status -uno 0.02(0.01+0.06) 0.08(0.04+0.07) +300.0%\n> 7519.5: status -uall 0.08(0.06+0.07) 0.15(0.08+0.09) +87.5%\n> \n> Now obviously due to 1 run that has a lot of noise, but I would expect\n> that first invocation to be blindingly fast since watchman has info on\n> what files were modified since the cache was flushed.\n> \n\nEvery (first) run of the performance test will be very expensive for the \nreasons outlined above. This is clearest when you only have 1 run as it \ndoesn't get masked by the faster 2nd+ runs. That first run will never \nbe \"blindingly fast\" as git has to gather the initial state and save out \nthe cache - it's all the subsequent calls that will be faster.\n\n> The same on the large monorepo noted above:\n> \n> Test origin/master avar/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.59(0.28+0.24) 0.93(0.35+0.19) +57.6%\n> 7519.3: status (subsequent) 0.20(0.10+0.19) 0.28(0.16+0.20) +40.0%\n> 7519.4: status -uno 0.04(0.04+0.09) 0.11(0.08+0.12) +175.0%\n> 7519.5: status -uall 0.29(0.11+0.18) 0.40(0.16+0.19) +37.9%\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n\nSame issue here - the overhead of the hook + watchman + perl outweighs \nany savings for these status calls that are already so fast.\n\nThere is certainly noise from run to run but on my machine, the minimum \ntime I saw with fsmonitor was in the .25+ range. Where fsmonitor really \nbecomes useful is when status times get over a second and in extreme \ncases where it gets to a minute or more.\n> On Fri, Jun 2, 2017 at 2:40 AM, Ben Peart wrote:\n>> Any chance you can provide me with a bash script that contains the exact\n>> sequence of commands you are running to get this result? I've been trying\n>> to replicate it using your notes but have not been able to. I'd like to see\n>> if it is a repo difference, a platform difference, a command sequence\n>> difference (or something else entirely :)).\n> \n> I can do better than that, here's a new perf test on top of this\n> series which demonstates the issue. I've only tested this on Linux\n> 4.9.0 with watchman 4.9.0 compiled from git (yes, they're\n> coincidentally the same version).\n> \n\nThank you! Great addition. I've included an updated version below.\n\n> A good addition to this would be `printf past> | watchman -j` as noted in my earlier mail, but I ran out of\n> time.\n> \n> You can also set any combination of GIT_PERF_7519_UNTRACKED_CACHE &\n> GIT_PERF_7519_SPLIT_INDEX to play with turning that on. I haven't\n> tested all combinations of that, but e.g. testing with untrackedCache\n> didn't give results that looked different from the performance\n> regressions noted above.\n> \n\nMakes sense, given how the performance framework creates the test repo, \nthere typically aren't untracked files to optimize. I updated the perf \ntest to turn it on anyway for platforms where it is available.\n\n> Aside from performance, I think a very good addition to stress-test\n> this series would be a patch to t/test-lib*sh guarded by some env flag\n> to do a similar watchman watch-del/watch/watch-list dance as the one\n> I'm doing here in the setup, and setting up the hook / config.\n> That would allow testing the entire git test suite with this feature,\n> to find any subtle bugs this might have introduced in git-status.\n> \n\nAnother good idea. I had started that but ran into bugs with Watchman \non Windows (https://github.com/facebook/watchman/issues/461) where \nrunning the git test suite actually causes Watchman to stop monitoring \nfolders. Once I can get past that, I'll give it another go.\n\n\n\ndiff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh\nnew file mode 100755\nindex 0000000000..e658254ecf\n--- /dev/null\n+++ b/t/perf/p7519-fsmonitor.sh\n@@ -0,0 +1,68 @@\n+#!/bin/sh\n+\n+test_description=\"Test core.fsmonitor\"\n+\n+. ./perf-lib.sh\n+\n+test_perf_large_repo\n+test_checkout_worktree\n+\n+# fsmonitor works correctly with or without the untracked cache\n+# but if it is available, we'll turn it on to ensure we test that\n+# codepath as well.\n+\n+test_lazy_prereq UNTRACKED_CACHE '\n+ { git update-index --test-untracked-cache; ret=$?; } &&\n+ test $ret -ne 1\n+'\n+\n+if test_have_prereq UNTRACKED_CACHE; then\n+ git config core.untrackedcache true\n+else\n+ git config core.untrackedcache false\n+fi\n+\n+test_expect_success 'setup' '\n+ # set splitIndex depending on the environment, defaulting to false.\n+ if test -n \"$GIT_PERF_7519_SPLIT_INDEX\"\n+ then\n+ git config core.splitIndex true\n+ else\n+ git config core.splitIndex false\n+ fi &&\n+\n+ # Hook scaffolding\n+ mkdir .git/hooks &&\n+ cp ../../../templates/hooks--query-fsmonitor.sample \n.git/hooks/query-fsmonitor &&\n+\n+ watchman watch \"$PWD\" >/dev/null 2>&1 &&\n+ watchman watch-list | grep -q -F \"$PWD\"\n+'\n+\n+# Setting:\n+#\n+# GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 \n| sudo tee /proc/sys/vm/dro\np_caches && make -j8'\n+#\n+# Can be used as a hack to performance test 'git status' on a cold fs\n+# cache with an existing watchman watching the directory, which should\n+# be blindingly fast, compared to amazingly slow without watchman.\n+\n+# Run git status without using fsmonitor as a baseline.\n+test_perf 'status (without fsmonitor)' 'git -c core.fsmonitor=false status'\n+\n+# The first time you query Watchman after the daemon starts or the first\n+# call after adding a new folder to watch, it scans the folder and returns\n+# _all_ files which makes the initial status call slower than without.\n+# Working with Watchman team to return a code indicating we should just\n+# scan everything ourselves to avoid the extra overhead, solution TBD.\n+test_perf 'status (populate cache)' 'git -c core.fsmonitor=true status'\n+\n+# Now Watchman has done the initial scan and the fsmonitor index extension\n+# is populated so we'll see the performance win.\n+test_perf 'status (with fsmonitor)' 'git -c core.fsmonitor=true status'\n+\n+# Let's see if -uno & -uall make any difference\n+test_perf 'status -uno' 'git -c core.fsmonitor=true status -uno'\n+test_perf 'status -uall' 'git -c core.fsmonitor=true status -uall'\n+\n+test_done\n\n\n"},{"id":"321450","messageId":"20170601212616.ecnisoedlkjzs6ln@sigill.intra.peff.net","threadId":"46090","inReplyTo":"CAGZ79ka1hjTFt9iRDrEy89ie9c3zj=BZ3SnZjPbLt6LGVDzeJQ@mail.gmail.com","subject":"Re: [PATCH v4 0/6] Fast git status via a file system watcher","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-01T21:26:16Z","receivedAt":"2017-06-02T22:26:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 01, 2017 at 02:13:10PM -0700, Stefan Beller wrote:\n\n> >\n> >> $ git am /tmp/original_msg.txt\n> >> Applying: fsmonitor: add documentation for the fsmonitor extension.\n> >> error: patch failed: Documentation/githooks.txt:448\n> >> error: Documentation/githooks.txt: patch does not apply\n> >> Patch failed at 0001 fsmonitor: add documentation for the\n> >> fsmonitor extension.\n> >> The copy of the patch that failed is found in:\n> >> .git/rebase-apply/patch\n> >> When you have resolved this problem, run \"git am --continue\".\n> >> If you prefer to skip this patch, run \"git am --skip\" instead.\n> >> To restore the original branch and stop patching, run \"git am\n> >> --abort\".\n> \n> Try again with -3. (We should make that the default for am, maybe?)\n> It helped me for most of the conflicts that I saw.\n\nYeah, I was going to suggest the same thing. \"git apply\" is much pickier\nthan \"patch\" about fuzzing the hunks (intentionally so, because the\nresults can sometimes end up not what the sender intended). But in most\ncases \"-3\" can apply the patch without losing safety (unless the sender\nbuilt off a branch that you don't even have).\n\nThere's some more discussion of --3way as the default in\n\n http://public-inbox.org/git/CAKwvOdn9j=_Ob=xq4ucN6Ar1G537zNiU9ox4iF6o1qO7kPY41A@mail.gmail.com/T/#u\n\n-Peff\n"},{"id":"321456","messageId":"CACBZZX48t3Jcy=eiga1f_ATSZZvy9_LG9wEe7avD2NCq2bsmJA@mail.gmail.com","threadId":"46090","inReplyTo":"c84864d3-9823-0b02-a337-29592b7c01f3@gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-02T23:06:16Z","receivedAt":"2017-06-02T23:06:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Jun 3, 2017 at 12:05 AM, Ben Peart wrote:\n>\n>\n> On 6/2/2017 6:28 AM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> Add a performance test for the new core.fsmonitor facility using the\n>> sample query-fsmonitor hook.\n>>\n>> This is WIP code for the reasons explained in the setup comments,\n>> unfortunately the perf code doesn't easily allow you to run different\n>> setup code for different versions you're testing. This test will stop\n>> working if the fsmonitor is merged into the master branch.\n>>\n>> Output against linxu.git:\n>>\n>> $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux\n>> GIT_PERF_MAKE_OPTS='-j8' ./run origin/master avar/fsmonitor\n>> ./p7519-fsmonitor.sh\n>> [...]\n>> Test origin/master avar/fsmonitor\n>>\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.08(0.04+0.09) 0.12(0.07+0.10)\n>> +50.0%\n>> 7519.3: status (subsequent) 0.08(0.04+0.09) 0.12(0.06+0.11)\n>> +50.0%\n>> 7519.4: status -uno 0.02(0.02+0.05) 0.06(0.05+0.06)\n>> +200.0%\n>> 7519.5: status -uall 0.08(0.06+0.07) 0.12(0.07+0.10)\n>> +50.0%\n>>\n>\n> With regular status times this low, the overhead of calling the hook +\n> watchman + perl results in slower overall times as I noted in my initial\n> cover letter. If status calls are already this fast, fsmonitor + watchman\n> isn't needed and obviously doesn't help.\n>\n> This does highlight an optimization I could add. Even though -uno is\n> passed, the fsmonitor code currently still goes through the work of marking\n> the untracked entries as dirty. I'll look at optimizing that out to speed\n> status up when using that option.\n>\n>> And against a larger in-house monorepo I have here, with the same\n>> options (except the repo path):\n>>\n>> Test origin/master avar/fsmonitor\n>>\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.20(0.11+0.18) 0.27(0.15+0.21)\n>> +35.0%\n>> 7519.3: status (subsequent) 0.20(0.11+0.18) 0.27(0.15+0.21)\n>> +35.0%\n>> 7519.4: status -uno 0.04(0.03+0.10) 0.22(0.08+0.12)\n>> +450.0%\n>> 7519.5: status -uall 0.20(0.13+0.16) 0.27(0.18+0.19)\n>> +35.0%\n>>\n>> Against linux.git with a hack to flush the FS cache (on Linux) before\n>> running the first 'git status', only running one test so the result\n>> isn't discarded as the slowest of N:\n>>\n>\n> I don't know know about on Linux but with Windows, when you flush the file\n> system cache via unmount/mount, it causes Watchman to do a full scan with\n> the next query. This has a significant negative performance impact on the\n> next status call as it returns the set of _all_ files which git then uses to\n> find and mark all index and untracked cache entries as fsmonitor dirty then\n> does a complete scan. This combination makes the first status after these\n> events slower than without Watchman.\n\nI'm not sure, but I don't think this is a thing at all on Linux. How\ninotify works is unrelated to whether some part of the filesystem\nhappens to be cached, and the echo+proc command I have is just\nflushing all those pages. That I'm flushing all the pages at once\nshould be as much of a non-event as one page being claimed from the\nfscache for use by the RAM allocation of some application.\n\nI suspect Windows somehow conflates its fs watching implementation\nwith what on Linux is the inotify queue the application has to\nconsume, if that queue isn't consumed watchman will need to re-scan.\n\nOr maybe my mental model of this whole thing is wrong and this also\nhappens on Linux...\n\n> I'm currently working with the Watchman team to return a code indicating we\n> should just scan everything ourselves to avoid the extra overhead, but that\n> solution is TBD. Given it only happens the first time a query is done on a\n> new clone or the first time after watchman is started, I didn't intend to\n> hold up the patch series for it but will submit an enhanced version once a\n> solution is available. The enhanced version will then be the same\n> performance (for the first status call) as when not running with fsmonitor -\n> not faster - as the state is unknown so must be gathered from the working\n> directory.\n\nI don't have time to update the perf test now or dig into it, but most\nof what you're describing in this mail doesn't at all match with the\nad-hoc tests I ran in\nhttps://public-inbox.org/git/CACBZZX5e58bWuf3NdDYTxu2KyZj29hHONzN=rp-7vXd8nURyWQ@mail.gmail.com/\n\nThere (at the very end of the E-Mail) I'm running watchman in a tight\nloop while I flush the entire fs cache, its runtime is never longer\nthan 600ms, with 3ms being the norm.\n\nI.e. flushing the cache doesn't slow things down much at all compared\nto how long a \"git status\" takes from cold cache. Something else must\nbe going on, and the smoking gun is the gprof output I posted in the\nfollow-up E-Mail:\nhttps://public-inbox.org/git/CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com/\n\nThere with the fsmonitor we end up calling blk_SHA1_Block ~100K times\nduring \"status\", but IIRC (I don't have the output in front of me,\nthis is from memory) something like twenty times without the\nfsmonitor.\n\nIt can't be a coincidence that with the fscache:\n\n$ pwd; git ls-files | wc -l\n/home/avar/g/linux\n59844\n\nAnd you can see that in the fsmonitor \"git status\" we make exactly\nthat many calls to cache_entry_from_ondisk(), but those calls don't\nshow up at all in the non-fscache codepath.\n\nSo, again, I haven't dug and really must step away from the computer\nnow, but this really looks like the fscache saves us the recursive\nreaddir() / lstat() etc, but in return we somehow fall though to a\ncodepath where we re-read the entire on-disk state back into the\nindex, which we don't do in the non-fscache codepath.\n\n>> $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_LARGE_REPO=~/g/linux\n>> GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee\n>> /proc/sys/vm/drop_caches >/dev/null && make -j8' ./run origin/master\n>> avar/fsmonitor ./p7519-fsmonitor.sh\n>> [...]\n>> Test origin/master avar/fsmonitor\n>>\n>> ------------------------------------------------------------------------\n>> 7519.2: status (first) 0.30(0.18+0.10) 8.26(0.22+0.10)\n>> +2653.3%\n>> 7519.3: status (subsequent) 0.08(0.04+0.08) 0.81(0.09+0.07)\n>> +912.5%\n>> 7519.4: status -uno 0.02(0.01+0.06) 0.08(0.04+0.07)\n>> +300.0%\n>> 7519.5: status -uall 0.08(0.06+0.07) 0.15(0.08+0.09)\n>> +87.5%\n>>\n>> Now obviously due to 1 run that has a lot of noise, but I would expect\n>> that first invocation to be blindingly fast since watchman has info on\n>> what files were modified since the cache was flushed.\n>>\n>\n> Every (first) run of the performance test will be very expensive for the\n> reasons outlined above. This is clearest when you only have 1 run as it\n> doesn't get masked by the faster 2nd+ runs. That first run will never be\n> \"blindingly fast\" as git has to gather the initial state and save out the\n> cache - it's all the subsequent calls that will be faster.\n>\n>> The same on the large monorepo noted above:\n>>\n>> Test origin/master avar/fsmonitor\n>>\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.59(0.28+0.24) 0.93(0.35+0.19)\n>> +57.6%\n>> 7519.3: status (subsequent) 0.20(0.10+0.19) 0.28(0.16+0.20)\n>> +40.0%\n>> 7519.4: status -uno 0.04(0.04+0.09) 0.11(0.08+0.12)\n>> +175.0%\n>> 7519.5: status -uall 0.29(0.11+0.18) 0.40(0.16+0.19)\n>> +37.9%\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>>\n>\n> Same issue here - the overhead of the hook + watchman + perl outweighs any\n> savings for these status calls that are already so fast.\n>\n> There is certainly noise from run to run but on my machine, the minimum time\n> I saw with fsmonitor was in the .25+ range. Where fsmonitor really becomes\n> useful is when status times get over a second and in extreme cases where it\n> gets to a minute or more.\n>>\n>> On Fri, Jun 2, 2017 at 2:40 AM, Ben Peart wrote:\n>>>\n>>> Any chance you can provide me with a bash script that contains the exact\n>>> sequence of commands you are running to get this result? I've been\n>>> trying\n>>> to replicate it using your notes but have not been able to. I'd like to\n>>> see\n>>> if it is a repo difference, a platform difference, a command sequence\n>>> difference (or something else entirely :)).\n>>\n>>\n>> I can do better than that, here's a new perf test on top of this\n>> series which demonstates the issue. I've only tested this on Linux\n>> 4.9.0 with watchman 4.9.0 compiled from git (yes, they're\n>> coincidentally the same version).\n>>\n>\n> Thank you! Great addition. I've included an updated version below.\n>\n>> A good addition to this would be `printf > past> | watchman -j` as noted in my earlier mail, but I ran out of\n>> time.\n>>\n>> You can also set any combination of GIT_PERF_7519_UNTRACKED_CACHE &\n>> GIT_PERF_7519_SPLIT_INDEX to play with turning that on. I haven't\n>> tested all combinations of that, but e.g. testing with untrackedCache\n>> didn't give results that looked different from the performance\n>> regressions noted above.\n>>\n>\n> Makes sense, given how the performance framework creates the test repo,\n> there typically aren't untracked files to optimize. I updated the perf test\n> to turn it on anyway for platforms where it is available.\n\nThe unfortunately named core.untrackedCache option has almost nothing\nto do with untracked files per-se. It's about caching mtimes of\ndirectories on the fs to avoid looking at them the next time, here on\na linux.git with no untracked files whatsoever:\n\n $ strace git -c core.untrackedCache=false status 2>&1 | wc -l\n 22780\n $ strace git -c core.untrackedCache=true status 2>&1 | wc -l\n 5105\n\nI.e. syscalls are cut down by 4x. This will/can matter in an isolated\nperformance test like this.\n\nIt should really have been called core.mtimeCache or something like that...\n\n>> Aside from performance, I think a very good addition to stress-test\n>> this series would be a patch to t/test-lib*sh guarded by some env flag\n>> to do a similar watchman watch-del/watch/watch-list dance as the one\n>> I'm doing here in the setup, and setting up the hook / config.\n>> That would allow testing the entire git test suite with this feature,\n>> to find any subtle bugs this might have introduced in git-status.\n>>\n>\n> Another good idea. I had started that but ran into bugs with Watchman on\n> Windows (https://github.com/facebook/watchman/issues/461) where running the\n> git test suite actually causes Watchman to stop monitoring folders. Once I\n> can get past that, I'll give it another go.\n>\n>\n>\n> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh\n> new file mode 100755\n> index 0000000000..e658254ecf\n> --- /dev/null\n> +++ b/t/perf/p7519-fsmonitor.sh\n> @@ -0,0 +1,68 @@\n> +#!/bin/sh\n> +\n> +test_description=\"Test core.fsmonitor\"\n> +\n> +. ./perf-lib.sh\n> +\n> +test_perf_large_repo\n> +test_checkout_worktree\n> +\n> +# fsmonitor works correctly with or without the untracked cache\n> +# but if it is available, we'll turn it on to ensure we test that\n> +# codepath as well.\n> +\n> +test_lazy_prereq UNTRACKED_CACHE '\n> + { git update-index --test-untracked-cache; ret=$?; } &&\n> + test $ret -ne 1\n> +'\n> +\n> +if test_have_prereq UNTRACKED_CACHE; then\n> + git config core.untrackedcache true\n> +else\n> + git config core.untrackedcache false\n> +fi\n> +\n> +test_expect_success 'setup' '\n> + # set splitIndex depending on the environment, defaulting to false.\n> + if test -n \"$GIT_PERF_7519_SPLIT_INDEX\"\n> + then\n> + git config core.splitIndex true\n> + else\n> + git config core.splitIndex false\n> + fi &&\n> +\n> + # Hook scaffolding\n> + mkdir .git/hooks &&\n> + cp ../../../templates/hooks--query-fsmonitor.sample\n> .git/hooks/query-fsmonitor &&\n> +\n> + watchman watch \"$PWD\" >/dev/null 2>&1 &&\n> + watchman watch-list | grep -q -F \"$PWD\"\n> +'\n> +\n> +# Setting:\n> +#\n> +# GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 |\n> sudo tee /proc/sys/vm/dro\n> p_caches && make -j8'\n> +#\n> +# Can be used as a hack to performance test 'git status' on a cold fs\n> +# cache with an existing watchman watching the directory, which should\n> +# be blindingly fast, compared to amazingly slow without watchman.\n> +\n> +# Run git status without using fsmonitor as a baseline.\n> +test_perf 'status (without fsmonitor)' 'git -c core.fsmonitor=false status'\n> +\n> +# The first time you query Watchman after the daemon starts or the first\n> +# call after adding a new folder to watch, it scans the folder and returns\n> +# _all_ files which makes the initial status call slower than without.\n> +# Working with Watchman team to return a code indicating we should just\n> +# scan everything ourselves to avoid the extra overhead, solution TBD.\n> +test_perf 'status (populate cache)' 'git -c core.fsmonitor=true status'\n> +\n> +# Now Watchman has done the initial scan and the fsmonitor index extension\n> +# is populated so we'll see the performance win.\n> +test_perf 'status (with fsmonitor)' 'git -c core.fsmonitor=true status'\n> +\n> +# Let's see if -uno & -uall make any difference\n> +test_perf 'status -uno' 'git -c core.fsmonitor=true status -uno'\n> +test_perf 'status -uall' 'git -c core.fsmonitor=true status -uall'\n> +\n> +test_done\n>\n>\n"},{"id":"321484","messageId":"CACBZZX7Xttqk9pbhtARHef_hy=WnbokTLnfe7W-sGN18611MYQ@mail.gmail.com","threadId":"46090","inReplyTo":"75ecd288047648178ed93105ee1f7a9e@exmbdft7.ad.twosigma.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-03T18:08:37Z","receivedAt":"2017-06-03T18:09:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Jun 2, 2017 at 11:44 PM, David Turner wrote:\n> BTW, a medium-sized (~250k files across 40k dirs) synthetic repo is available over bittorrent at:\n> http://bitmover.com/2015-04-03-1M-git-bare.tar.bz2.torrent\n\nCan you seed this please? I've been trying to download that since\nyesterday, still at 0%.\n"},{"id":"321494","messageId":"xmqq1sr0wm0i.fsf@gitster.mtv.corp.google.com","threadId":"46090","inReplyTo":"20170602102853.23073-1-avarab@gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-04T01:59:41Z","receivedAt":"2017-06-04T01:59:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is WIP code for the reasons explained in the setup comments,\n> unfortunately the perf code doesn't easily allow you to run different\n> setup code for different versions you're testing. This test will stop\n> working if the fsmonitor is merged into the master branch.\n> ...\n> +\n> +\t# Relies on core.fsmonitor not being merged into master. Needs\n> +\t# better per-test ways to disable it if it gets merged.\n> +\tgit config core.fsmonitor true &&\n\nWill stop working and relies on not merged can be read but I cannot\nread \"why\" explained, and I cannot quite guess what the reason is.\n\nIf the code to read the configuration is not there, setting this\nwould not have any effect. If the code is there, setting this would\nhave effect (either talking fsmonitor helps or it hurts).\n\nAnd I do not think we'd ever see a version of Git that always relies\non talking to fsmonitor, i.e. \"git config core.fsmonitor false\" is not\na way to disable it, so...\n\nPuzzled.\n\n\n"},{"id":"321499","messageId":"CACBZZX4XOaN8o89vetoU8NMqRH+BaqHGkxq77MpqzvAM40exEA@mail.gmail.com","threadId":"46090","inReplyTo":"xmqq1sr0wm0i.fsf@gitster.mtv.corp.google.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-04T07:46:19Z","receivedAt":"2017-06-04T07:47:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jun 4, 2017 at 3:59 AM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> This is WIP code for the reasons explained in the setup comments,\n>> unfortunately the perf code doesn't easily allow you to run different\n>> setup code for different versions you're testing. This test will stop\n>> working if the fsmonitor is merged into the master branch.\n>> ...\n>> +\n>> + # Relies on core.fsmonitor not being merged into master. Needs\n>> + # better per-test ways to disable it if it gets merged.\n>> + git config core.fsmonitor true &&\n>\n> Will stop working and relies on not merged can be read but I cannot\n> read \"why\" explained, and I cannot quite guess what the reason is.\n>\n> If the code to read the configuration is not there, setting this\n> would not have any effect. If the code is there, setting this would\n> have effect (either talking fsmonitor helps or it hurts).\n>\n> And I do not think we'd ever see a version of Git that always relies\n> on talking to fsmonitor, i.e. \"git config core.fsmonitor false\" is not\n> a way to disable it, so...\n>\n> Puzzled.\n\nSorry about the unclear brevity.\n\nWhat I'm referring to is not a limitation of git (we'll always be able\nto turn off core.fsmonitor), but a limitation of the perf framework.\nThere's no way to run a test like this:\n\n ./run master next -- p*.sh\n\nAnd have some information passed to the test to apply different\nruntime options to the test depending on master or next, or be to test\nmaster twice, once with the fsmonitor, once without, which this\nhypothetical feature would do:\n\n ./run master:\"GIT_PERF_7519_NO_FSMONITOR=Y\" master -- p*.sh\n\nSo right now the test works because there's no core.fsmonitor in\nmaster, so turning it on all the time only impacts avar/fsmonitor, not\nmaster.\n\nI started trying to add this to the perf framework the other day but\nran out of time, the option should also be passed down early enough to\nbe intercepted by the GIT_PERF_MAKE_COMMAND, so you could do e.g.:\n\n GIT_PERF_MAKE_COMMAND='make CFLAGS=\"$F\"' \\\n ./run v2.13.0:\"F=-O0\" v2.13.0:\"F=-O1\" v2.13.0:\"F=-O2\"\nv2.13.0:\"F=-O3\" -- p*.sh\n\nTo test the same revision with different compilation flags.\n\nA change like this would break the ability to enact certain perf\noptimizations, right now we unpack the revision(s) you specify into\n, and e.g. make use of the fact that that directory\nis already unpacked so we don't need to unpack it again.\n\nIf there was no way to pass a flag to specific revisions being tested,\nthen perf could just optimize:\n\n ./run v2.12.0 v2.13.0 b06d364310 -- p*.sh\n\nTo skip the b06d364310 run entirely since it's the same as v2.13.0. I\nthink breaking this minor assumption in the framework is fine, but\nit's worth noting that it's an assumption we couldn't make anymore.\n"},{"id":"321501","messageId":"20170604082141.54wanlacy5ksaalt@sigill.intra.peff.net","threadId":"46090","inReplyTo":"CACBZZX4XOaN8o89vetoU8NMqRH+BaqHGkxq77MpqzvAM40exEA@mail.gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-04T08:21:42Z","receivedAt":"2017-06-04T08:21:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 04, 2017 at 09:46:19AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> What I'm referring to is not a limitation of git (we'll always be able\n> to turn off core.fsmonitor), but a limitation of the perf framework.\n> There's no way to run a test like this:\n> \n> ./run master next -- p*.sh\n> \n> And have some information passed to the test to apply different\n> runtime options to the test depending on master or next, or be to test\n> master twice, once with the fsmonitor, once without, which this\n> hypothetical feature would do:\n> \n> ./run master:\"GIT_PERF_7519_NO_FSMONITOR=Y\" master -- p*.sh\n\nYeah, the perf framework was originally designed to find regressions\nbetween versions of Git. It's really bad at comparing results between\nany other dimensions. You can test different repository sizes by\ntweaking the environment, but it can't aggregate or compare results\nbetween those runs. Likewise, you can have two tests in a script which\ntime Git with and without certain options set, but there's no way to\ncompare the results of those tests.\n\nIt would be nice if the perf framework was aware of all of\nthese dimensions, stored each result as a tuple of all of the properties\n(rather than just the version), and then let you group the results along\nany dimension (I suspect there are cases where multi-dimensional\nsummaries would be useful, but that could come later).\n\nFor some dimensions you'd probably want support in the perf scripts\nthemselves to run a test with two variants. E.g., something like:\n\n test_perf_group 'fsmonitor' \\\n\t'false' 'git config core.fsmonitor false' \\\n\t 'true' 'git config core.fsmonitor true'\n test_perf 'status' 'git status'\n test_perf_group_end\n\nwhere that would run the \"git status\" test for each of the named setups,\nand store the timing results under \"($VERSION, fsmonitor=false)\", and so\non.\n\nThat's less flexible than specifying it to \"./run\" (which would let you\nrun just one tuple if you chose). But it also relieves the burden from\nthe user of figuring out which dimensions are interesting to tweak.\n\n-Peff\n"},{"id":"321538","messageId":"65158deb-981f-47f2-4ded-e5f691e2264f@gmail.com","threadId":"46090","inReplyTo":"75ecd288047648178ed93105ee1f7a9e@exmbdft7.ad.twosigma.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-05T14:27:10Z","receivedAt":"2017-06-05T14:27:19Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 6/2/2017 5:44 PM, David Turner wrote:\n> BTW, a medium-sized (~250k files across 40k dirs) synthetic repo is available over bittorrent at:\n> http://bitmover.com/2015-04-03-1M-git-bare.tar.bz2.torrent\n> \n> I tried Ævar's perf test with that (on a beefy laptop with SSD), and got significantly slower results with bp/fsmonitor:\n> Test origin/master bp/fsmonitor\n> -----------------------------------------------------------------------\n> 7519.2: status (first) 0.32(0.23+0.39) 0.32(0.26+0.36) +0.0%\n> 7519.3: status (subsequent) 0.18(0.14+0.34) 0.32(0.24+0.37) +77.8%\n> 7519.4: status -uno 0.11(0.08+0.32) 0.24(0.18+0.34) +118.2%\n> 7519.5: status -uall 0.49(0.22+0.56) 0.62(0.36+0.55) +26.5%\n> 7519.2: status (first) 0.32(0.23+0.39) 0.32(0.26+0.36) +0.0%\n> 7519.3: status (subsequent) 0.18(0.14+0.34) 0.32(0.24+0.37) +77.8%\n> 7519.4: status -uno 0.11(0.08+0.32) 0.24(0.18+0.34) +118.2%\n> 7519.5: status -uall 0.49(0.22+0.56) 0.62(0.36+0.55) +26.5%\n> \n> I have not yet looked into why this is.\n> \n\nI was very focused on getting minute long status calls down to seconds \nand multiple seconds down to sub-second. The greatest benefits are when \nthe file system cache doesn't already have all the file information \ncached and the current perf test doesn't test that case - just the warm \ncache test which has the least benefit.\n\nThat said, status times shouldn't be getting worse and this has \nhighlighted that they are. I've found one reason (the current patch \nseries always flags the index as dirty so it gets written out every \ntime). I've got a fix that only flags it dirty when the extension is \nturned on or off or when it actually finds an entry that has become \ndirty. This helps but there is more going on than that.\n\nI'm looking into why the minimum status time with fsmonitor turned on \nand a warm cache seems to be ~30ms. More to come...\n\n>> -----Original Message-----\n>> From: Ævar Arnfjörð Bjarmason [mailto:avarab@gmail.com]\n>> Sent: Friday, June 2, 2017 6:29 AM\n>> To: git@vger.kernel.org\n>> Cc: Junio C Hamano ; Ben Peart\n>> ; Nguyễn Thái Ngọc Duy ;\n>> Johannes Schindelin ; David Turner\n>> ; Jeff King ; Christian\n>> Couder ; Ævar Arnfjörð Bjarmason\n>> \n>> Subject: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor\n>>\n>> Add a performance test for the new core.fsmonitor facility using the sample\n>> query-fsmonitor hook.\n>>\n>> This is WIP code for the reasons explained in the setup comments,\n>> unfortunately the perf code doesn't easily allow you to run different setup\n>> code for different versions you're testing. This test will stop working if the\n>> fsmonitor is merged into the master branch.\n>>\n>> Output against linxu.git:\n>>\n>> $ GIT_PERF_REPEAT_COUNT=10 GIT_PERF_LARGE_REPO=~/g/linux\n>> GIT_PERF_MAKE_OPTS='-j8' ./run origin/master avar/fsmonitor ./p7519-\n>> fsmonitor.sh\n>> [...]\n>> Test origin/master avar/fsmonitor\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.08(0.04+0.09) 0.12(0.07+0.10) +50.0%\n>> 7519.3: status (subsequent) 0.08(0.04+0.09) 0.12(0.06+0.11) +50.0%\n>> 7519.4: status -uno 0.02(0.02+0.05) 0.06(0.05+0.06) +200.0%\n>> 7519.5: status -uall 0.08(0.06+0.07) 0.12(0.07+0.10) +50.0%\n>>\n>> And against a larger in-house monorepo I have here, with the same options\n>> (except the repo path):\n>>\n>> Test origin/master avar/fsmonitor\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n>> 7519.3: status (subsequent) 0.20(0.11+0.18) 0.27(0.15+0.21) +35.0%\n>> 7519.4: status -uno 0.04(0.03+0.10) 0.22(0.08+0.12) +450.0%\n>> 7519.5: status -uall 0.20(0.13+0.16) 0.27(0.18+0.19) +35.0%\n>>\n>> Against linux.git with a hack to flush the FS cache (on Linux) before running\n>> the first 'git status', only running one test so the result isn't discarded as the\n>> slowest of N:\n>>\n>> $ GIT_PERF_REPEAT_COUNT=1 GIT_PERF_LARGE_REPO=~/g/linux\n>> GIT_PERF_MAKE_COMMAND='sudo sync && echo 3 | sudo tee\n>> /proc/sys/vm/drop_caches >/dev/null && make -j8' ./run origin/master\n>> avar/fsmonitor ./p7519-fsmonitor.sh\n>> [...]\n>> Test origin/master avar/fsmonitor\n>> ------------------------------------------------------------------------\n>> 7519.2: status (first) 0.30(0.18+0.10) 8.26(0.22+0.10) +2653.3%\n>> 7519.3: status (subsequent) 0.08(0.04+0.08) 0.81(0.09+0.07) +912.5%\n>> 7519.4: status -uno 0.02(0.01+0.06) 0.08(0.04+0.07) +300.0%\n>> 7519.5: status -uall 0.08(0.06+0.07) 0.15(0.08+0.09) +87.5%\n>>\n>> Now obviously due to 1 run that has a lot of noise, but I would expect that\n>> first invocation to be blindingly fast since watchman has info on what files\n>> were modified since the cache was flushed.\n>>\n>> The same on the large monorepo noted above:\n>>\n>> Test origin/master avar/fsmonitor\n>> -----------------------------------------------------------------------\n>> 7519.2: status (first) 0.59(0.28+0.24) 0.93(0.35+0.19) +57.6%\n>> 7519.3: status (subsequent) 0.20(0.10+0.19) 0.28(0.16+0.20) +40.0%\n>> 7519.4: status -uno 0.04(0.04+0.09) 0.11(0.08+0.12) +175.0%\n>> 7519.5: status -uall 0.29(0.11+0.18) 0.40(0.16+0.19) +37.9%\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>>\n>>\n>> On Fri, Jun 2, 2017 at 2:40 AM, Ben Peart wrote:\n>>> Any chance you can provide me with a bash script that contains the\n>>> exact sequence of commands you are running to get this result? I've\n>>> been trying to replicate it using your notes but have not been able\n>>> to. I'd like to see if it is a repo difference, a platform\n>>> difference, a command sequence difference (or something else entirely\n>> :)).\n>>\n>> I can do better than that, here's a new perf test on top of this series which\n>> demonstates the issue. I've only tested this on Linux\n>> 4.9.0 with watchman 4.9.0 compiled from git (yes, they're coincidentally the\n>> same version).\n>>\n>> A good addition to this would be `printf > past> | watchman -j` as noted in my earlier mail, but I ran out of\n>> time.\n>>\n>> You can also set any combination of GIT_PERF_7519_UNTRACKED_CACHE &\n>> GIT_PERF_7519_SPLIT_INDEX to play with turning that on. I haven't tested all\n>> combinations of that, but e.g. testing with untrackedCache didn't give results\n>> that looked different from the performance regressions noted above.\n>>\n>> Aside from performance, I think a very good addition to stress-test this series\n>> would be a patch to t/test-lib*sh guarded by some env flag to do a similar\n>> watchman watch-del/watch/watch-list dance as the one I'm doing here in\n>> the setup, and setting up the hook / config.\n>>\n>> That would allow testing the entire git test suite with this feature, to find any\n>> subtle bugs this might have introduced in git-status.\n>>\n>> t/perf/p7519-fsmonitor.sh | 58\n>> +++++++++++++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 58 insertions(+)\n>> create mode 100755 t/perf/p7519-fsmonitor.sh\n>>\n>> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh new file\n>> mode 100755 index 0000000000..b838a0ff14\n>> --- /dev/null\n>> +++ b/t/perf/p7519-fsmonitor.sh\n>> @@ -0,0 +1,58 @@\n>> +#!/bin/sh\n>> +\n>> +test_description=\"Test core.fsmonitor\"\n>> +\n>> +. ./perf-lib.sh\n>> +\n>> +test_perf_large_repo\n>> +test_checkout_worktree\n>> +\n>> +test_expect_success 'setup' '\n>> +\t# Maybe set untrackedCache & splitIndex depending on the\n>> +\t# environment, defaulting to false.\n>> +\tif test -n \"$GIT_PERF_7519_UNTRACKED_CACHE\"\n>> +\tthen\n>> +\t\tgit config core.untrackedCache true\n>> +\telse\n>> +\t\tgit config core.untrackedCache false\n>> +\tfi &&\n>> +\tif test -n \"$GIT_PERF_7519_SPLIT_INDEX\"\n>> +\tthen\n>> +\t\tgit config core.splitIndex true\n>> +\telse\n>> +\t\tgit config core.splitIndex false\n>> +\tfi &&\n>> +\n>> +\t# Relies on core.fsmonitor not being merged into master. Needs\n>> +\t# better per-test ways to disable it if it gets merged.\n>> +\tgit config core.fsmonitor true &&\n>> +\n>> +\t# Hook scaffolding\n>> +\tmkdir .git/hooks &&\n>> +\tcp ../../../templates/hooks--query-fsmonitor.sample\n>> +.git/hooks/query-fsmonitor &&\n>> +\n>> +\t# Setup watchman & ensure it is actually watching\n>> +\twatchman watch-del \"$PWD\" >/dev/null 2>&1 &&\n>> +\twatchman watch \"$PWD\" >/dev/null 2>&1 &&\n>> +\twatchman watch-list | grep -q -F \"$PWD\"\n>> +'\n>> +\n>> +# Setting:\n>> +#\n>> +# GIT_PERF_REPEAT_COUNT=1 GIT_PERF_MAKE_COMMAND='sudo sync\n>> && echo 3 | sudo tee /proc/sys/vm/drop_caches && make -j8'\n>> +#\n>> +# Can be used as a hack to performance test 'git status' on a cold fs #\n>> +cache with an existing watchman watching the directory, which should #\n>> +be blindingly fast, compared to amazingly slow without watchman.\n>> +test_perf 'status (first)' 'git status'\n>> +\n>> +\n>> +# The same git-status once the fs cache has been warmed, if using the #\n>> +GIT_PERF_MAKE_COMMAND above. Otherwise the same as above.\n>> +test_perf 'status (subsequent)' 'git status'\n>> +\n>> +# Let's see if -uno & -uall make any difference\n>> +test_perf 'status -uno' 'git status -uno'\n>> +test_perf 'status -uall' 'git status -uall'\n>> +\n>> +test_done\n>> --\n>> 2.13.0.506.g27d5fe0cd\n> \n"},{"id":"321707","messageId":"0cfd57d3-4092-af06-0d6c-cb3ceecc4633@gmail.com","threadId":"46090","inReplyTo":"CACBZZX48t3Jcy=eiga1f_ATSZZvy9_LG9wEe7avD2NCq2bsmJA@mail.gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-07T19:51:26Z","receivedAt":"2017-06-07T19:51:35Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 6/2/2017 7:06 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> I don't have time to update the perf test now or dig into it, but most\n> of what you're describing in this mail doesn't at all match with the\n> ad-hoc tests I ran in\n> https://public-inbox.org/git/CACBZZX5e58bWuf3NdDYTxu2KyZj29hHONzN=rp-7vXd8nURyWQ@mail.gmail.com/\n> \n> There (at the very end of the E-Mail) I'm running watchman in a tight\n> loop while I flush the entire fs cache, its runtime is never longer\n> than 600ms, with 3ms being the norm.\n\nI added a perf trace around the entire query-fsmonitor hook proc (patch \nbelow) to measure the total actual impact of running the hook script + \nquerying watchman + parsing the output with perl + passing the result \nback to git. On my machine, the total cost of the hook runs between 130 \nms and 180 ms when there are zero changes to report (ie best case).\n\nWith short status times, the overhead of watchman simply outweighs any \ngains in performance - especially when you have a warm file system cache \nas that cancels out the biggest win of avoiding the IO associated with \nscanning the working directory.\n\n\ndiff --git a/fsmonitor.c b/fsmonitor.c\nindex 763a8a3a3f..cb47f31863 100644\n--- a/fsmonitor.c\n+++ b/fsmonitor.c\n@@ -210,9 +210,11 @@ void refresh_by_fsmonitor(struct index_state *istate)\n * If we have a last update time, call query-monitor for the set of\n * changes since that time.\n */\n- if (istate->fsmonitor_last_update)\n+ if (istate->fsmonitor_last_update) {\n query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n istate->fsmonitor_last_update, &query_result);\n+ trace_performance_since(last_update, \"query-fsmonitor\");\n+ }\n\n if (query_success) {\n /* Mark all entries returned by the monitor as dirty */\n\n\n\n> \n> I.e. flushing the cache doesn't slow things down much at all compared\n> to how long a \"git status\" takes from cold cache. Something else must\n> be going on, and the smoking gun is the gprof output I posted in the\n> follow-up E-Mail:\n> https://public-inbox.org/git/CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com/\n> \n> There with the fsmonitor we end up calling blk_SHA1_Block ~100K times\n> during \"status\", but IIRC (I don't have the output in front of me,\n> this is from memory) something like twenty times without the\n> fsmonitor.\n> \n> It can't be a coincidence that with the fscache:\n> \n> $ pwd; git ls-files | wc -l\n> /home/avar/g/linux\n> 59844\n> \n> And you can see that in the fsmonitor \"git status\" we make exactly\n> that many calls to cache_entry_from_ondisk(), but those calls don't\n> show up at all in the non-fscache codepath.\n> \n\nI don't see how the gprof numbers for the non-fsmonitor case can be \ncorrect. It appears they don't contain any calls related to loading the \nindex while the fsmonitor gprof numbers do. Here is a typical call stack:\n\ngit.exe!cache_entry_from_ondisk()\ngit.exe!create_from_disk()\ngit.exe!do_read_index()\ngit.exe!read_index_from()\ngit.exe!read_index()\n\nDuring read_index(), cache_entry_from_ondisk() gets called for every \nitem in the index (which explains the 59K calls). How can the \nnon-fsmonitor codepath not be loading the index?\n\n> So, again, I haven't dug and really must step away from the computer\n> now, but this really looks like the fscache saves us the recursive\n> readdir() / lstat() etc, but in return we somehow fall though to a\n> codepath where we re-read the entire on-disk state back into the\n> index, which we don't do in the non-fscache codepath.\n> \n\nI've run multiple profiles and compared them with fsmonitor on and off \nand have been unable to find any performance regression caused by \nfsmonitor (other than flagging the index as dirty at times when it isn't \nrequired which I have fixed for the next patch series).\n\nI have done many performance runs and when I subtract the _actual_ time \nspent in the hook from the overall command time, it comes in at slightly \nless time than when status is run with fsmonitor off. This also leads \nme to believe there is no regression with fsmonitor on.\n\nAll this leads me back to my original conclusion: the reason status is \nslower in these specific cases is because the overhead of calling the \nhook exceeds the savings gained. If your status calls are taking less \nthan a second, it just doesn't make sense to add the complexity and \noverhead of calling a file system watcher.\n\nI'm working on an updated perf test that will demonstrate the best case \nscenario (warm watchman, cold file system cache) in addition to the \nworst case (cold watchman, warm file system cache). The reality is that \nin normal use cases, perf will be between the two. I'll add that to the \nnext iteration of the patch series.\n"},{"id":"321712","messageId":"CACBZZX5a6Op7dH_g9WOFBnejh2zgNK4b34ygxA8daNDqvitFVA@mail.gmail.com","threadId":"46090","inReplyTo":"20170601155105.28356-7-benpeart@microsoft.com","subject":"Re: [PATCH v4 6/6] fsmonitor: add a sample query-fsmonitor hook script for Watchman","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-07T21:38:08Z","receivedAt":"2017-06-07T21:38:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jun 1, 2017 at 5:51 PM, Ben Peart wrote:\n> +if [ $1 -eq 1 ]\n\nTiny nit: Needs quoting:\n\n $ .git/hooks/query-fsmonitor\n .git/hooks/query-fsmonitor: 15: [: -eq: unexpected operator\n"},{"id":"321713","messageId":"CACBZZX6D8oC34qat7kdrDOWC5eYm-DRkMWG9eOPPvKKsQtgPyw@mail.gmail.com","threadId":"46090","inReplyTo":"0cfd57d3-4092-af06-0d6c-cb3ceecc4633@gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-06-07T21:46:28Z","receivedAt":"2017-06-07T21:46:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Wed, Jun 7, 2017 at 9:51 PM, Ben Peart wrote:\n>\n>\n> On 6/2/2017 7:06 PM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>\n>> I don't have time to update the perf test now or dig into it, but most\n>> of what you're describing in this mail doesn't at all match with the\n>> ad-hoc tests I ran in\n>>\n>> https://public-inbox.org/git/CACBZZX5e58bWuf3NdDYTxu2KyZj29hHONzN=rp-7vXd8nURyWQ@mail.gmail.com/\n>>\n>> There (at the very end of the E-Mail) I'm running watchman in a tight\n>> loop while I flush the entire fs cache, its runtime is never longer\n>> than 600ms, with 3ms being the norm.\n>\n>\n> I added a perf trace around the entire query-fsmonitor hook proc (patch\n> below) to measure the total actual impact of running the hook script +\n> querying watchman + parsing the output with perl + passing the result back\n> to git. On my machine, the total cost of the hook runs between 130 ms and\n> 180 ms when there are zero changes to report (ie best case).\n>\n> With short status times, the overhead of watchman simply outweighs any gains\n> in performance - especially when you have a warm file system cache as that\n> cancels out the biggest win of avoiding the IO associated with scanning the\n> working directory.\n>\n>\n> diff --git a/fsmonitor.c b/fsmonitor.c\n> index 763a8a3a3f..cb47f31863 100644\n> --- a/fsmonitor.c\n> +++ b/fsmonitor.c\n> @@ -210,9 +210,11 @@ void refresh_by_fsmonitor(struct index_state *istate)\n> * If we have a last update time, call query-monitor for the set of\n> * changes since that time.\n> */\n> - if (istate->fsmonitor_last_update)\n> + if (istate->fsmonitor_last_update) {\n> query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n> istate->fsmonitor_last_update, &query_result);\n> + trace_performance_since(last_update, \"query-fsmonitor\");\n> + }\n>\n> if (query_success) {\n> /* Mark all entries returned by the monitor as dirty */\n>\n>\n>\n>>\n>> I.e. flushing the cache doesn't slow things down much at all compared\n>> to how long a \"git status\" takes from cold cache. Something else must\n>> be going on, and the smoking gun is the gprof output I posted in the\n>> follow-up E-Mail:\n>>\n>> https://public-inbox.org/git/CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com/\n>>\n>> There with the fsmonitor we end up calling blk_SHA1_Block ~100K times\n>> during \"status\", but IIRC (I don't have the output in front of me,\n>> this is from memory) something like twenty times without the\n>> fsmonitor.\n>>\n>> It can't be a coincidence that with the fscache:\n>>\n>> $ pwd; git ls-files | wc -l\n>> /home/avar/g/linux\n>> 59844\n>>\n>> And you can see that in the fsmonitor \"git status\" we make exactly\n>> that many calls to cache_entry_from_ondisk(), but those calls don't\n>> show up at all in the non-fscache codepath.\n>>\n>\n> I don't see how the gprof numbers for the non-fsmonitor case can be correct.\n> It appears they don't contain any calls related to loading the index while\n> the fsmonitor gprof numbers do. Here is a typical call stack:\n>\n> git.exe!cache_entry_from_ondisk()\n> git.exe!create_from_disk()\n> git.exe!do_read_index()\n> git.exe!read_index_from()\n> git.exe!read_index()\n>\n> During read_index(), cache_entry_from_ondisk() gets called for every item in\n> the index (which explains the 59K calls). How can the non-fsmonitor\n> codepath not be loading the index?\n>\n>> So, again, I haven't dug and really must step away from the computer\n>> now, but this really looks like the fscache saves us the recursive\n>> readdir() / lstat() etc, but in return we somehow fall though to a\n>> codepath where we re-read the entire on-disk state back into the\n>> index, which we don't do in the non-fscache codepath.\n>>\n>\n> I've run multiple profiles and compared them with fsmonitor on and off and\n> have been unable to find any performance regression caused by fsmonitor\n> (other than flagging the index as dirty at times when it isn't required\n> which I have fixed for the next patch series).\n>\n> I have done many performance runs and when I subtract the _actual_ time\n> spent in the hook from the overall command time, it comes in at slightly\n> less time than when status is run with fsmonitor off. This also leads me to\n> believe there is no regression with fsmonitor on.\n>\n> All this leads me back to my original conclusion: the reason status is\n> slower in these specific cases is because the overhead of calling the hook\n> exceeds the savings gained. If your status calls are taking less than a\n> second, it just doesn't make sense to add the complexity and overhead of\n> calling a file system watcher.\n>\n> I'm working on an updated perf test that will demonstrate the best case\n> scenario (warm watchman, cold file system cache) in addition to the worst\n> case (cold watchman, warm file system cache). The reality is that in normal\n> use cases, perf will be between the two. I'll add that to the next iteration\n> of the patch series.\n\nI'll try to dig further once we have the next submission + that perf test.\n\nOn Linux the time spent calling the hook itself is minimal:\n\n $ touch foo; time .git/hooks/query-fsmonitor 1 $(($(date +%s) *\n1000000000 - 10))\n Watchman says these changed: foo\n foo\n real 0m0.009s\n user 0m0.004s\n sys 0m0.000s\n\nSo I'm fairly sure something else entirely is going on, but anyway, we\ncan look at that later with the next submission.\n\nAside from that, I wonder on Windows how much speedier this could be\nfor you if the entire hook was written in perl instead of a\nshellscript that calls perl. I could help with that if you'd like.\n\nYou can likely replace that call to uname with reading the $^O variable.\n\nWhat you're doing shelling out to cygpath can probably be done by\nloading the File::Spec library, but I'm not sure.\n\nThe echo / watchman / perl would need to be an IPC::Open3 invocation I think.\n\nI think it being in shellscript is fine, just a suggestion in case\nhaving it all in one process (aside from the watchman shell-out) would\nhelp or Windows.\n"},{"id":"321730","messageId":"468f6b07-f61f-d4ca-a8df-d0a80302b251@gmail.com","threadId":"46090","inReplyTo":"CACBZZX6D8oC34qat7kdrDOWC5eYm-DRkMWG9eOPPvKKsQtgPyw@mail.gmail.com","subject":"Re: [WIP/PATCH 7/6] perf: add a performance test for core.fsmonitor","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-06-08T01:57:49Z","receivedAt":"2017-06-08T01:57:58Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 6/7/2017 5:46 PM, Ævar Arnfjörð Bjarmason wrote:\n> On Wed, Jun 7, 2017 at 9:51 PM, Ben Peart wrote:\n>>\n>>\n>> On 6/2/2017 7:06 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>>\n>>> I don't have time to update the perf test now or dig into it, but most\n>>> of what you're describing in this mail doesn't at all match with the\n>>> ad-hoc tests I ran in\n>>>\n>>> https://public-inbox.org/git/CACBZZX5e58bWuf3NdDYTxu2KyZj29hHONzN=rp-7vXd8nURyWQ@mail.gmail.com/\n>>>\n>>> There (at the very end of the E-Mail) I'm running watchman in a tight\n>>> loop while I flush the entire fs cache, its runtime is never longer\n>>> than 600ms, with 3ms being the norm.\n>>\n>>\n>> I added a perf trace around the entire query-fsmonitor hook proc (patch\n>> below) to measure the total actual impact of running the hook script +\n>> querying watchman + parsing the output with perl + passing the result back\n>> to git. On my machine, the total cost of the hook runs between 130 ms and\n>> 180 ms when there are zero changes to report (ie best case).\n>>\n>> With short status times, the overhead of watchman simply outweighs any gains\n>> in performance - especially when you have a warm file system cache as that\n>> cancels out the biggest win of avoiding the IO associated with scanning the\n>> working directory.\n>>\n>>\n>> diff --git a/fsmonitor.c b/fsmonitor.c\n>> index 763a8a3a3f..cb47f31863 100644\n>> --- a/fsmonitor.c\n>> +++ b/fsmonitor.c\n>> @@ -210,9 +210,11 @@ void refresh_by_fsmonitor(struct index_state *istate)\n>> * If we have a last update time, call query-monitor for the set of\n>> * changes since that time.\n>> */\n>> - if (istate->fsmonitor_last_update)\n>> + if (istate->fsmonitor_last_update) {\n>> query_success = !query_fsmonitor(HOOK_INTERFACE_VERSION,\n>> istate->fsmonitor_last_update, &query_result);\n>> + trace_performance_since(last_update, \"query-fsmonitor\");\n>> + }\n>>\n>> if (query_success) {\n>> /* Mark all entries returned by the monitor as dirty */\n>>\n>>\n>>\n>>>\n>>> I.e. flushing the cache doesn't slow things down much at all compared\n>>> to how long a \"git status\" takes from cold cache. Something else must\n>>> be going on, and the smoking gun is the gprof output I posted in the\n>>> follow-up E-Mail:\n>>>\n>>> https://public-inbox.org/git/CACBZZX4eZ3G8LQ8O+_BkbkJ-ZXTOkUi9cW=QKYjfHKtmA3pgrA@mail.gmail.com/\n>>>\n>>> There with the fsmonitor we end up calling blk_SHA1_Block ~100K times\n>>> during \"status\", but IIRC (I don't have the output in front of me,\n>>> this is from memory) something like twenty times without the\n>>> fsmonitor.\n>>>\n>>> It can't be a coincidence that with the fscache:\n>>>\n>>> $ pwd; git ls-files | wc -l\n>>> /home/avar/g/linux\n>>> 59844\n>>>\n>>> And you can see that in the fsmonitor \"git status\" we make exactly\n>>> that many calls to cache_entry_from_ondisk(), but those calls don't\n>>> show up at all in the non-fscache codepath.\n>>>\n>>\n>> I don't see how the gprof numbers for the non-fsmonitor case can be correct.\n>> It appears they don't contain any calls related to loading the index while\n>> the fsmonitor gprof numbers do. Here is a typical call stack:\n>>\n>> git.exe!cache_entry_from_ondisk()\n>> git.exe!create_from_disk()\n>> git.exe!do_read_index()\n>> git.exe!read_index_from()\n>> git.exe!read_index()\n>>\n>> During read_index(), cache_entry_from_ondisk() gets called for every item in\n>> the index (which explains the 59K calls). How can the non-fsmonitor\n>> codepath not be loading the index?\n>>\n>>> So, again, I haven't dug and really must step away from the computer\n>>> now, but this really looks like the fscache saves us the recursive\n>>> readdir() / lstat() etc, but in return we somehow fall though to a\n>>> codepath where we re-read the entire on-disk state back into the\n>>> index, which we don't do in the non-fscache codepath.\n>>>\n>>\n>> I've run multiple profiles and compared them with fsmonitor on and off and\n>> have been unable to find any performance regression caused by fsmonitor\n>> (other than flagging the index as dirty at times when it isn't required\n>> which I have fixed for the next patch series).\n>>\n>> I have done many performance runs and when I subtract the _actual_ time\n>> spent in the hook from the overall command time, it comes in at slightly\n>> less time than when status is run with fsmonitor off. This also leads me to\n>> believe there is no regression with fsmonitor on.\n>>\n>> All this leads me back to my original conclusion: the reason status is\n>> slower in these specific cases is because the overhead of calling the hook\n>> exceeds the savings gained. If your status calls are taking less than a\n>> second, it just doesn't make sense to add the complexity and overhead of\n>> calling a file system watcher.\n>>\n>> I'm working on an updated perf test that will demonstrate the best case\n>> scenario (warm watchman, cold file system cache) in addition to the worst\n>> case (cold watchman, warm file system cache). The reality is that in normal\n>> use cases, perf will be between the two. I'll add that to the next iteration\n>> of the patch series.\n> \n> I'll try to dig further once we have the next submission + that perf test.\n> \n> On Linux the time spent calling the hook itself is minimal:\n> \n> $ touch foo; time .git/hooks/query-fsmonitor 1 $(($(date +%s) *\n> 1000000000 - 10))\n> Watchman says these changed: foo\n> foo\n> real 0m0.009s\n> user 0m0.004s\n> sys 0m0.000s\n> \n\nWow, what a difference:\n\n$ touch foo; time .git/hooks/query-fsmonitor 1 $(($(date +%s) *\n > 1000000000 - 10))\n.git/hooks/query-fsmonitor 1 1496885562999999990\n.gitfoo\nreal 0m0.170s\nuser 0m0.045s\nsys 0m0.091s\n\n\n> So I'm fairly sure something else entirely is going on, but anyway, we\n> can look at that later with the next submission.\n> \n> Aside from that, I wonder on Windows how much speedier this could be\n> for you if the entire hook was written in perl instead of a\n> shellscript that calls perl. I could help with that if you'd like.\n> \n\nAnything we can do to speed this up and have fewer moving pieces is all \ngood. I'm not a perl expert so I'd very much appreciate your expertise. \n I have a couple of notes from Wez (author of Watchman) on things we \ncould do to better optimize certain scenarios I'll pass along as well.\n\n> You can likely replace that call to uname with reading the $^O variable.\n> \n> What you're doing shelling out to cygpath can probably be done by\n> loading the File::Spec library, but I'm not sure.\n> \n> The echo / watchman / perl would need to be an IPC::Open3 invocation I think.\n> \n> I think it being in shellscript is fine, just a suggestion in case\n> having it all in one process (aside from the watchman shell-out) would\n> help or Windows.\n> \n"}]}