{"thread":{"id":"45972","subject":"[PATCH v1 0/5] Fast git status via a file system watcher","startedAt":"2017-05-15T19:14:07Z","lastAt":"2017-05-18T04:52:15Z","messageCount":26,"participants":["Ben Peart","David Turner","brian m. carlson","Jeff King","Junio C Hamano","Johannes Sixt","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"319818","messageId":"20170515191347.1892-1-benpeart@microsoft.com","threadId":"45972","inReplyTo":null,"subject":"[PATCH v1 0/5] Fast git status via a file system watcher","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T19:13:42Z","receivedAt":"2017-05-15T19:14:07Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Goal\n~~~~\n�\nToday, git must check existing files to see if there have been changes\nand scan the working directory looking for new, untracked files. As the\nnumber of files and folders in the working directory increases, the time\nto perform these checks can become very expensive O(# files in working\ndirectory).\n\nGiven the number of new or modified files is typically a very small\npercentage of the total number of files, it would be much more\nperformant if git only had to check files and folders that potentially\nhad changes. This reduces the cost to O(# modified files).\n\nThis patch series makes it possible to optionally add a hook process\nthat can return the set of files that may have been changed since the\nrequested time. Git can then use this to limit its scan to only those\nfiles and folders that potentially have changes.\n\nDesign\n~~~~~~\n\nA new git hook (query-fsmonitor) must exist and be enabled \n(core.fsmonitor=true) that takes a time_t formatted as a string and\noutputs to stdout all files that have been modified since the requested\ntime.\n\nA new 'fsmonitor' index extension has been added to store the time the\nfsmonitor hook was last queried and a ewah bitmap of the current\n'fsmonitor-dirty' files. Unmarked entries are 'fsmonitor-clean', marked\nentries are 'fsmonitor-dirty.'\n\nAs needed, git will call the query-fsmonitor hook proc for the set of\nchanges since the index was last updated. Git then uses this set of\nfiles along with the list saved in the fsmonitor index extension to flag\nthe potentially dirty index and untracked cache entries. \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. This saves all the work of checking files and folders for\nchanges that are already known to be clean.\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.\n\nThe code is conservative so in case of any error (missing index\nextension, error from hook, etc) it falls back to normal logic of\nchecking everything.\n\nA sample hook is provided in query-fsmonitor.sample to integrate with\nthe cross platform Watchman file watching service\nhttps://facebook.github.io/watchman/\n\n\nPerformance\n~~~~~~~~~~~\n\nThe performance wins of this model are pretty dramatic. Each test was\nrun 3 times and averaged. \"Files\" is the number of files in the working\ndirectory. Tests were done with a cold file system cache as well as\nwith a warm file system cache on a HDD. SSD speeds were typically about\n10x faster than the HDD. Typical real world results would fall\nsomewhere between these extremes. \n\n*--------------------------------------------------------*\n| Repo on HDD | Cache | fsmonitor=false | fsmonitor=true |\n*--------------------------------------------------------*\n| 3K Files | Cold | 0.77s | 0.55s |\n+--------------------------------------------------------+\n| 100K Files | Cold | 38.76s | 2.17s |\n+--------------------------------------------------------+\n| 3M Files | Cold | 421.55s | 18.57s |\n+--------------------------------------------------------+\n| 3K Files | Warm | 0.05s | 0.24s |\n+--------------------------------------------------------+\n| 100K Files | Warm | 1.13s | 0.40s |\n+--------------------------------------------------------+\n| 3M Files | Warm | 59.33s | 4.19s |\n+--------------------------------------------------------+\n\nNote that with the smallest repo, warm times actually increase slightly\nas the overhead of calling the hook, watchman and perl outweighs the\nsavings of not scanning the working directory.\n\n\nOpen Issues\n~~~~~~~~~~~\n\nThe index extension currently has a 32 bit version number, a 64 bit time\nand a 32 bit bitmap size. Do I need to quad-align the version and\nbitmap size in the index extension or can all supported platforms handle\ndereferencing memory that isn't quad aligned?\n\n\nCredits\n~~~~~~~\n\nIdea taken and code refactored from \nhttp://public-inbox.org/git/1466914464-10358-1-git-send-email-novalis@novalis.org/\n\nCurrent version as a fork of GFW on GitHub here: \nhttps://github.com/benpeart/git-for-windows/tree/fsmonitor\n\n\nBen Peart (5):\n dir: make lookup_untracked() available outside of dir.c\n Teach git to optionally utilize a file system monitor to speed up\n detecting new or changed files.\n fsmonitor: add test cases for fsmonitor extension\n Add documentation for the fsmonitor extension. This includes the\n core.fsmonitor setting, the query-fsmonitor hook, and the fsmonitor\n index extension.\n Add a sample query-fsmonitor hook script that integrates with the\n cross platform Watchman file watching service.\n\n Documentation/config.txt | 7 +\n Documentation/githooks.txt | 23 +++\n Documentation/technical/index-format.txt | 18 +++\n Makefile | 1 +\n builtin/update-index.c | 1 +\n cache.h | 5 +\n config.c | 5 +\n dir.c | 15 +-\n dir.h | 5 +\n entry.c | 1 +\n environment.c | 1 +\n fsmonitor.c | 233 +++++++++++++++++++++++++++++++\n fsmonitor.h | 9 ++\n read-cache.c | 28 +++-\n t/t7519-status-fsmonitor.sh | 134 ++++++++++++++++++\n templates/hooks--query-fsmonitor.sample | 27 ++++\n unpack-trees.c | 1 +\n 17 files changed, 511 insertions(+), 3 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n create mode 100644 t/t7519-status-fsmonitor.sh\n create mode 100644 templates/hooks--query-fsmonitor.sample\n\n-- \n2.13.0.windows.1.6.g4597375fc3\n\n"},{"id":"319819","messageId":"20170515191347.1892-2-benpeart@microsoft.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"[PATCH v1 1/5] dir: make lookup_untracked() available outside of dir.c","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T19:13:43Z","receivedAt":"2017-05-15T19:14:17Z","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.\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.6.g4597375fc3\n\n"},{"id":"319820","messageId":"20170515191347.1892-4-benpeart@microsoft.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"[PATCH v1 3/5] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T19:13:45Z","receivedAt":"2017-05-15T19:14:33Z","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\nSigned-off-by: Ben Peart \n---\n t/t7519-status-fsmonitor.sh | 134 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 134 insertions(+)\n create mode 100644 t/t7519-status-fsmonitor.sh\n\ndiff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\nnew file mode 100644\nindex 0000000000..2d63efc27b\n--- /dev/null\n+++ b/t/t7519-status-fsmonitor.sh\n@@ -0,0 +1,134 @@\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+}\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+test_expect_success 'setup' '\n+\tmkdir -p .git/hooks &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\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+\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+\tdirty_repo\n+'\n+\n+cat >.gitignore <<\\EOF\n+.gitignore\n+expect*\n+output*\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+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_i18ncmp expect output\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+\tdirty_repo &&\n+\tgit status >output &&\n+\ttest_i18ncmp expect output\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+\tdirty_repo &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tEOF\n+\tgit add . &&\n+\tgit commit -m \"to reset\" &&\n+\tgit status &&\n+\tgit reset HEAD~1 &&\n+\tgit status >output &&\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 doesnt detect unreported modifications' '\n+\tgit config core.fsmonitor true &&\n+\tgit config core.untrackedcache true &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\t:\n+\tEOF\n+\tclean_repo &&\n+\tgit status &&\n+\t: >untracked &&\n+\techo 2 >dir1/modified &&\n+\tgit status >output &&\n+\ttest_i18ngrep ! \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep ! \"Untracked files:\" output &&\n+\twrite_script .git/hooks/query-fsmonitor<<-\\EOF &&\n+\tprintf \"untracked%s\\0\"\n+\tprintf \"dir1/modified\\0\"\n+\tEOF\n+\tgit status >output &&\n+\ttest_i18ngrep \"Changes not staged for commit:\" output &&\n+\ttest_i18ngrep \"Untracked files:\" output\n+'\n+\n+test_done\n-- \n2.13.0.windows.1.6.g4597375fc3\n\n"},{"id":"319821","messageId":"20170515191347.1892-6-benpeart@microsoft.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"[PATCH v1 5/5] Add a sample query-fsmonitor hook script that integrates with the cross platform Watchman file watching service.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T19:13:47Z","receivedAt":"2017-05-15T19:14:35Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"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 \n---\n templates/hooks--query-fsmonitor.sample | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n create mode 100644 templates/hooks--query-fsmonitor.sample\n\ndiff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--query-fsmonitor.sample\nnew file mode 100644\nindex 0000000000..4bd22f21d8\n--- /dev/null\n+++ b/templates/hooks--query-fsmonitor.sample\n@@ -0,0 +1,27 @@\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 time_t formatted as a string and outputs to\n+# stdout all files that have been modified since the given time.\n+# Paths must be relative to the root of the working tree and\n+# separated by a single NUL.\n+#\n+# To enable this hook, rename this file to \"query-fsmonitor\"\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\\\": $1, \\\"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-- \n2.13.0.windows.1.6.g4597375fc3\n\n"},{"id":"319822","messageId":"20170515191347.1892-3-benpeart@microsoft.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"[PATCH v1 2/5] 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-05-15T19:13:44Z","receivedAt":"2017-05-15T19:14:36Z","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 cach\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---\n Makefile | 1 +\n builtin/update-index.c | 1 +\n cache.h | 5 ++\n config.c | 5 ++\n dir.c | 13 +++\n dir.h | 2 +\n entry.c | 1 +\n environment.c | 1 +\n fsmonitor.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++++\n fsmonitor.h | 9 ++\n read-cache.c | 28 +++++-\n unpack-trees.c | 1 +\n 12 files changed, 298 insertions(+), 2 deletions(-)\n create mode 100644 fsmonitor.c\n create mode 100644 fsmonitor.h\n\ndiff --git a/Makefile b/Makefile\nindex 94cce645a5..89acff1f46 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -761,6 +761,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 40ec032a2d..64aa6e57cd 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+\ttime_t last_update;\n+\tstruct ewah_bitmap *bitmap;\n };\n \n extern struct index_state the_index;\n@@ -767,6 +771,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 d971cc3474..d146c88399 100644\n--- a/config.c\n+++ b/config.c\n@@ -1224,6 +1224,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 platform_core_config(var, value);\n }\ndiff --git a/dir.c b/dir.c\nindex 1b5558fdf9..da428489e2 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1652,6 +1652,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 +1677,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 c57731c468..3758e76159 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -63,6 +63,7 @@ int merge_log_config = -1;\n int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\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..466a0df6a1\n--- /dev/null\n+++ b/fsmonitor.c\n@@ -0,0 +1,233 @@\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+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 = ntohl(*(uint32_t *)index);\n+\tindex += sizeof(uint32_t);\n+\tif (hdr_version != 1)\n+\t\treturn error(\"bad fsmonitor version %d\", hdr_version);\n+\n+\tistate->last_update = (time_t)ntohll(*(uint64_t *)index);\n+\tindex += sizeof(uint64_t);\n+\n+\tewah_size = ntohl(*(uint32_t *)index);\n+\tindex += sizeof(uint32_t);\n+\n+\tistate->bitmap = ewah_new();\n+\tret = ewah_read_mmap(istate->bitmap, index, ewah_size);\n+\tif (ret != ewah_size) {\n+\t\tewah_free(istate->bitmap);\n+\t\tistate->bitmap = 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(1);\n+\tstrbuf_add(sb, &hdr_version, sizeof(uint32_t));\n+\n+\ttm = htonll((uint64_t)istate->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 int update_istate(const char *name, void *is)\n+{\n+\tstruct index_state *istate = is;\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 0;\n+\n+\tdir = find_untracked_cache_dir(istate->untracked, istate->untracked->root, name);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+\n+\treturn 0;\n+}\n+\n+/*\n+ * Call the query-fsmonitor hook passing the time of the last saved results.\n+ */\n+static int query_fsmonitor(time_t last_update, struct strbuf *buffer)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar date[64];\n+\tconst char *argv[3];\n+\n+\tif (!(argv[0] = find_hook(\"query-fsmonitor\")))\n+\t\treturn -1;\n+\n+\tsnprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n+\targv[1] = date;\n+\targv[2] = NULL;\n+\tcp.argv = argv;\n+\tcp.out = -1;\n+\n+\treturn capture_command(&cp, buffer, 1024);\n+}\n+\n+void process_fsmonitor_extension(struct index_state *istate)\n+{\n+\tif (!istate->bitmap)\n+\t\treturn;\n+\n+\tewah_each_bit(istate->bitmap, mark_no_fsmonitor, istate);\n+\tewah_free(istate->bitmap);\n+\tistate->bitmap = NULL;\n+}\n+\n+void refresh_by_fsmonitor(struct index_state *istate)\n+{\n+\tstatic has_run_once = FALSE;\n+\tstruct strbuf buffer = STRBUF_INIT;\n+\tint query_success = 0;\n+\tsize_t bol = 0; /* beginning of line */\n+\ttime_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 = TRUE;\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 = time(NULL);\n+\n+\t/* If we have a last update time, call query-monitor for the set of changes since that time */\n+\tif (istate->last_update) {\n+\t\tquery_success = !query_fsmonitor(istate->last_update, &buffer);\n+\t}\n+\n+\tif (query_success) {\n+\t\t/* Mark all entries returned by the monitor as dirty */\n+\t\tbuf = entry = buffer.buf;\n+\t\tfor (i = 0; i < buffer.len; i++) {\n+\t\t\tif (buf[i] != '\\0')\n+\t\t\t\tcontinue;\n+\t\t\tupdate_istate(buf + bol, istate);\n+\t\t\tbol = i + 1;\n+\t\t}\n+\t\tif (bol < buffer.len)\n+\t\t\tupdate_istate(buf + bol, istate);\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\t}\n+\t}\n+\telse {\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+\n+\t/* Now that we've updated istate, save the last_update time */\n+\tistate->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..28e61602b1\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 dce05c1dde..a081c4a1d6 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 #ifndef NO_PTHREADS\n #include \n@@ -41,11 +42,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@@ -65,6 +67,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@@ -781,6 +784,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@@ -1348,6 +1352,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@@ -1384,8 +1390,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@@ -1395,6 +1404,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@@ -1581,6 +1591,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@@ -1753,6 +1766,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@@ -2358,6 +2372,16 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\tif (err)\n \t\t\treturn -1;\n \t}\n+\tif (!strip_extensions && istate->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))\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.6.g4597375fc3\n\n"},{"id":"319823","messageId":"20170515191347.1892-5-benpeart@microsoft.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"[PATCH v1 4/5] Add documentation for the fsmonitor extension. This includes the core.fsmonitor setting, the query-fsmonitor hook, and the fsmonitor index extension.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T19:13:46Z","receivedAt":"2017-05-15T19:14:38Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"Signed-off-by: Ben Peart \n---\n Documentation/config.txt | 7 +++++++\n Documentation/githooks.txt | 23 +++++++++++++++++++++++\n Documentation/technical/index-format.txt | 18 ++++++++++++++++++\n 3 files changed, 48 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex bc7088b287..a9a58cb8a6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -391,6 +391,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..f7b4b4a844 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+a single argument which is the time in elapsed seconds since midnight,\n+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..b002d23c05 100644\n--- a/Documentation/technical/index-format.txt\n+++ b/Documentation/technical/index-format.txt\n@@ -295,3 +295,21 @@ 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 seconds elapsed since midnight, January 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.6.g4597375fc3\n\n"},{"id":"319830","messageId":"fb609e259c714469b5528888e14c2e3a@exmbdft7.ad.twosigma.com","threadId":"45972","inReplyTo":"20170515191347.1892-6-benpeart@microsoft.com","subject":"RE: [PATCH v1 5/5] Add a sample query-fsmonitor hook script that integrates with the cross platform Watchman file watching service.","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-05-15T19:50:53Z","receivedAt":"2017-05-15T19:51:05Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"\n\n> -----Original Message-----\n> From: Ben Peart [mailto:peartben@gmail.com]\n> Sent: Monday, May 15, 2017 3:14 PM\n> To: git@vger.kernel.org\n> Cc: gitster@pobox.com; benpeart@microsoft.com; pclouds@gmail.com;\n> johannes.schindelin@gmx.de; David Turner ;\n> peff@peff.net\n> Subject: [PATCH v1 5/5] Add a sample query-fsmonitor hook script that\n> integrates with the cross platform Watchman file watching service.\n> \n> To use the script:\n> \n> Download and install Watchman from https://facebook.github.io/watchman/\n> and instruct Watchman to watch your working directory for changes\n> ('watchman watch-project /usr/src/git').\n> \n> Rename the sample integration hook from query-fsmonitor.sample to query-\n> fsmonitor.\n> \n> Configure git to use the extension ('git config core.fsmonitor true') and\n> optionally turn on the untracked cache for optimal performance ('git config\n> core.untrackedcache true').\n> \n> Signed-off-by: Ben Peart \n> Signed-off-by: Johannes Schindelin \n> ---\n> templates/hooks--query-fsmonitor.sample | 27\n> +++++++++++++++++++++++++++\n> 1 file changed, 27 insertions(+)\n> create mode 100644 templates/hooks--query-fsmonitor.sample\n> \n> diff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--\n> query-fsmonitor.sample\n> new file mode 100644\n> index 0000000000..4bd22f21d8\n> --- /dev/null\n> +++ b/templates/hooks--query-fsmonitor.sample\n> @@ -0,0 +1,27 @@\n> +#!/bin/sh\n> +#\n> +# An example hook script to integrate Watchman #\n> +(https://facebook.github.io/watchman/) with git to provide fast # git\n> +status.\n> +#\n> +# The hook is passed a time_t formatted as a string and outputs to #\n> +stdout all files that have been modified since the given time.\n> +# Paths must be relative to the root of the working tree and #\n> +separated by a single NUL.\n> +#\n> +# To enable this hook, rename this file to \"query-fsmonitor\"\n> +\n> +# Convert unix style paths to escaped Windows style paths case \"$(uname\n> +-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 echo\n> +\"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $1,\n> +\\\"fields\\\":[\\\"name\\\"]}]\" | \\ watchman -j | \\ perl -e 'use JSON::PP; my\n> +$o = JSON::PP->new->utf8->decode(join(\"\", <>)); die \"Watchman: $o-\n> >{'error'}.\\nFalling back to scanning...\\n\" if defined($o->{\"error\"});\n> print(join(\"\\0\", @{$o->{\"files\"}}));'\n\nLast time I checked, the argument to 'since' was not a time_t -- it was a \nwatchman clock spec. Have you tested this? Does it work?\n\n"},{"id":"319832","messageId":"268acc85-8fc7-7779-8cb8-f0e88e7d50a5@gmail.com","threadId":"45972","inReplyTo":"fb609e259c714469b5528888e14c2e3a@exmbdft7.ad.twosigma.com","subject":"Re: [PATCH v1 5/5] Add a sample query-fsmonitor hook script that integrates with the cross platform Watchman file watching service.","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-15T20:10:31Z","receivedAt":"2017-05-15T20:10:45Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/15/2017 3:50 PM, David Turner wrote:\n>\n>> -----Original Message-----\n>> From: Ben Peart [mailto:peartben@gmail.com]\n>> Sent: Monday, May 15, 2017 3:14 PM\n>> To: git@vger.kernel.org\n>> Cc: gitster@pobox.com; benpeart@microsoft.com; pclouds@gmail.com;\n>> johannes.schindelin@gmx.de; David Turner ;\n>> peff@peff.net\n>> Subject: [PATCH v1 5/5] Add a sample query-fsmonitor hook script that\n>> integrates with the cross platform Watchman file watching service.\n>>\n>> To use the script:\n>>\n>> Download and install Watchman from https://facebook.github.io/watchman/\n>> and instruct Watchman to watch your working directory for changes\n>> ('watchman watch-project /usr/src/git').\n>>\n>> Rename the sample integration hook from query-fsmonitor.sample to query-\n>> fsmonitor.\n>>\n>> Configure git to use the extension ('git config core.fsmonitor true') and\n>> optionally turn on the untracked cache for optimal performance ('git config\n>> core.untrackedcache true').\n>>\n>> Signed-off-by: Ben Peart \n>> Signed-off-by: Johannes Schindelin \n>> ---\n>> templates/hooks--query-fsmonitor.sample | 27\n>> +++++++++++++++++++++++++++\n>> 1 file changed, 27 insertions(+)\n>> create mode 100644 templates/hooks--query-fsmonitor.sample\n>>\n>> diff --git a/templates/hooks--query-fsmonitor.sample b/templates/hooks--\n>> query-fsmonitor.sample\n>> new file mode 100644\n>> index 0000000000..4bd22f21d8\n>> --- /dev/null\n>> +++ b/templates/hooks--query-fsmonitor.sample\n>> @@ -0,0 +1,27 @@\n>> +#!/bin/sh\n>> +#\n>> +# An example hook script to integrate Watchman #\n>> +(https://facebook.github.io/watchman/) with git to provide fast # git\n>> +status.\n>> +#\n>> +# The hook is passed a time_t formatted as a string and outputs to #\n>> +stdout all files that have been modified since the given time.\n>> +# Paths must be relative to the root of the working tree and #\n>> +separated by a single NUL.\n>> +#\n>> +# To enable this hook, rename this file to \"query-fsmonitor\"\n>> +\n>> +# Convert unix style paths to escaped Windows style paths case \"$(uname\n>> +-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 echo\n>> +\"[\\\"query\\\", \\\"$GIT_WORK_TREE\\\", {\\\"since\\\": $1,\n>> +\\\"fields\\\":[\\\"name\\\"]}]\" | \\ watchman -j | \\ perl -e 'use JSON::PP; my\n>> +$o = JSON::PP->new->utf8->decode(join(\"\", <>)); die \"Watchman: $o-\n>>> {'error'}.\\nFalling back to scanning...\\n\" if defined($o->{\"error\"});\n>> print(join(\"\\0\", @{$o->{\"files\"}}));'\n> Last time I checked, the argument to 'since' was not a time_t -- it was a\n> watchman clock spec. Have you tested this? Does it work?\n>\n\nWatchman also accepts a Unix time value for \"since\" as documented here \n(https://facebook.github.io/watchman/docs/expr/since.html).\n\nYes, this has been tested and works correctly as long as you have a \nrecent version that contains the patch \n(https://github.com/facebook/watchman/commit/67b26a8938336f08918fc7187129b6c1a571f35b) \nthat made sure it was greedy when using the Unix time.\n\n"},{"id":"319837","messageId":"d195af80f27e4fea85a96d6435b36139@exmbdft7.ad.twosigma.com","threadId":"45972","inReplyTo":"20170515191347.1892-3-benpeart@microsoft.com","subject":"RE: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-05-15T21:21:37Z","receivedAt":"2017-05-15T21:21:45Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"\n> -----Original Message-----\n> From: Ben Peart [mailto:peartben@gmail.com]\n> Sent: Monday, May 15, 2017 3:14 PM\n> To: git@vger.kernel.org\n> Cc: gitster@pobox.com; benpeart@microsoft.com; pclouds@gmail.com;\n> johannes.schindelin@gmx.de; David Turner ;\n> peff@peff.net\n> Subject: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to\n> speed up detecting new or changed files.\n\n> @@ -342,6 +344,8 @@ struct index_state {\n> \tstruct hashmap dir_hash;\n> \tunsigned char sha1[20];\n> \tstruct untracked_cache *untracked;\n> +\ttime_t last_update;\n> +\tstruct ewah_bitmap *bitmap;\n\nThe name 'bitmap' doesn't tell the reader much about what it used for.\n\n> +static int update_istate(const char *name, void *is) {\n\nRename to mark_file_dirty? Also why does it take a void pointer? Or return int (rather than void)?\n\n> +void refresh_by_fsmonitor(struct index_state *istate) {\n> +\tstatic has_run_once = FALSE;\n> +\tstruct strbuf buffer = STRBUF_INIT;\n\nRename to query_result? Also I think you're leaking it.\n\n"},{"id":"319852","messageId":"20170516002214.tlqkk4zrwdzcdjha@genre.crustytoothpaste.net","threadId":"45972","inReplyTo":"20170515191347.1892-3-benpeart@microsoft.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-05-16T00:22:14Z","receivedAt":"2017-05-16T00:22:27Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, May 15, 2017 at 03:13:44PM -0400, Ben Peart wrote:\n> +\tistate->last_update = (time_t)ntohll(*(uint64_t *)index);\n> +\tindex += sizeof(uint64_t);\n> +\n> +\tewah_size = ntohl(*(uint32_t *)index);\n> +\tindex += sizeof(uint32_t);\n\nTo answer the question you asked in your cover letter, you cannot write\nthis unless you can guarantee (((uintptr_t)index & 7) == 0) is true.\nOtherwise, this will produce a SIGBUS on SPARC, Alpha, MIPS, and some\nARM systems, and it will perform poorly on PowerPC and other ARM\nsystems[0].\n\nIf you got that pointer from malloc and have only indexed multiples of 8\non it, you're good. But if you're not sure, you probably want to use\nmemcpy. If the compiler can determine that it's not necessary, it will\nomit the copy and perform a direct load.\n\n[0] To be technically correct, all of those systems except SPARC can\nhave unaligned access fixed up automatically, depending on the kernel\nsettings. But such a fixup involves taking a trap into the kernel,\nperforming two aligned loads and bit shifting, and returning to\nuserspace, which performs about as well as you'd expect. For that\nreason, Debian build machines have such fixups turned off and will just\nSIGBUS.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"319854","messageId":"20170516003414.yliltu5fsaudfhyu@sigill.intra.peff.net","threadId":"45972","inReplyTo":"20170516002214.tlqkk4zrwdzcdjha@genre.crustytoothpaste.net","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-16T00:34:15Z","receivedAt":"2017-05-16T00:34:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 16, 2017 at 12:22:14AM +0000, brian m. carlson wrote:\n\n> On Mon, May 15, 2017 at 03:13:44PM -0400, Ben Peart wrote:\n> > +\tistate->last_update = (time_t)ntohll(*(uint64_t *)index);\n> > +\tindex += sizeof(uint64_t);\n> > +\n> > +\tewah_size = ntohl(*(uint32_t *)index);\n> > +\tindex += sizeof(uint32_t);\n> \n> To answer the question you asked in your cover letter, you cannot write\n> this unless you can guarantee (((uintptr_t)index & 7) == 0) is true.\n> Otherwise, this will produce a SIGBUS on SPARC, Alpha, MIPS, and some\n> ARM systems, and it will perform poorly on PowerPC and other ARM\n> systems[0].\n> \n> If you got that pointer from malloc and have only indexed multiples of 8\n> on it, you're good. But if you're not sure, you probably want to use\n> memcpy. If the compiler can determine that it's not necessary, it will\n> omit the copy and perform a direct load.\n\nI think get_be32() does exactly what we want for the ewah_size read. For\nthe last_update one, we don't have a get_be64() yet, but it should be\neasy to make based on the 16/32 versions.\n\n(I note also that time_t is not necessarily 64-bits in the first place,\nbut David said something about this not really being a time_t).\n\n-Peff\n"},{"id":"319861","messageId":"00db0fd9-e873-79f4-0497-0fec6c98d937@gmail.com","threadId":"45972","inReplyTo":"d195af80f27e4fea85a96d6435b36139@exmbdft7.ad.twosigma.com","subject":"Re: [PATCH v1 2/5] 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-05-16T01:15:13Z","receivedAt":"2017-05-16T01:15:20Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\nOn 5/15/2017 5:21 PM, David Turner wrote:\n>\n>> -----Original Message-----\n>> From: Ben Peart [mailto:peartben@gmail.com]\n>> Sent: Monday, May 15, 2017 3:14 PM\n>> To: git@vger.kernel.org\n>> Cc: gitster@pobox.com; benpeart@microsoft.com; pclouds@gmail.com;\n>> johannes.schindelin@gmx.de; David Turner ;\n>> peff@peff.net\n>> Subject: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to\n>> speed up detecting new or changed files.\n>\n>> @@ -342,6 +344,8 @@ struct index_state {\n>> \tstruct hashmap dir_hash;\n>> \tunsigned char sha1[20];\n>> \tstruct untracked_cache *untracked;\n>> +\ttime_t last_update;\n>> +\tstruct ewah_bitmap *bitmap;\n>\n> The name 'bitmap' doesn't tell the reader much about what it used for.\n>\n>> +static int update_istate(const char *name, void *is) {\n>\n> Rename to mark_file_dirty? Also why does it take a void pointer? Or return int (rather than void)?\n>\n\nThanks for the feedback. I'll do some renaming and change the types passed.\n\n\n>> +void refresh_by_fsmonitor(struct index_state *istate) {\n>> +\tstatic has_run_once = FALSE;\n>> +\tstruct strbuf buffer = STRBUF_INIT;\n>\n> Rename to query_result? Also I think you're leaking it.\n>\n\nGood catch! I missed the leak there. Fixed for the next roll.\n"},{"id":"319863","messageId":"2d965a87-36da-23b4-4bc5-97de47f3d7f7@gmail.com","threadId":"45972","inReplyTo":"20170516003414.yliltu5fsaudfhyu@sigill.intra.peff.net","subject":"Re: [PATCH v1 2/5] 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-05-16T01:55:12Z","receivedAt":"2017-05-16T01:55:22Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/15/2017 8:34 PM, Jeff King wrote:\n> On Tue, May 16, 2017 at 12:22:14AM +0000, brian m. carlson wrote:\n>\n>> On Mon, May 15, 2017 at 03:13:44PM -0400, Ben Peart wrote:\n>>> +\tistate->last_update = (time_t)ntohll(*(uint64_t *)index);\n>>> +\tindex += sizeof(uint64_t);\n>>> +\n>>> +\tewah_size = ntohl(*(uint32_t *)index);\n>>> +\tindex += sizeof(uint32_t);\n>>\n>> To answer the question you asked in your cover letter, you cannot write\n>> this unless you can guarantee (((uintptr_t)index & 7) == 0) is true.\n>> Otherwise, this will produce a SIGBUS on SPARC, Alpha, MIPS, and some\n>> ARM systems, and it will perform poorly on PowerPC and other ARM\n>> systems[0].\n>>\n>> If you got that pointer from malloc and have only indexed multiples of 8\n>> on it, you're good. But if you're not sure, you probably want to use\n>> memcpy. If the compiler can determine that it's not necessary, it will\n>> omit the copy and perform a direct load.\n>\n> I think get_be32() does exactly what we want for the ewah_size read. For\n> the last_update one, we don't have a get_be64() yet, but it should be\n> easy to make based on the 16/32 versions.\n\nThanks for the pointers. I'll update this to use the existing get_be32 \nand have created a get_be64 and will use that for the last_update.\n\n>\n> (I note also that time_t is not necessarily 64-bits in the first place,\n> but David said something about this not really being a time_t).\n>\n\nThe in memory representation is a time_t as that is the return value of \ntime(NULL) but it is converted to/from a 64 bit value when written/read \nto the index extension so that the index format is the same no matter \nthe native size of time_t.\n\n> -Peff\n>\n"},{"id":"319866","messageId":"20170516025120.xn5dimkuryl33wfk@sigill.intra.peff.net","threadId":"45972","inReplyTo":"2d965a87-36da-23b4-4bc5-97de47f3d7f7@gmail.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-16T02:51:20Z","receivedAt":"2017-05-16T02:51:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 15, 2017 at 09:55:12PM -0400, Ben Peart wrote:\n\n> > > > +\tistate->last_update = (time_t)ntohll(*(uint64_t *)index);\n> [...]\n> > (I note also that time_t is not necessarily 64-bits in the first place,\n> > but David said something about this not really being a time_t).\n> \n> The in memory representation is a time_t as that is the return value of\n> time(NULL) but it is converted to/from a 64 bit value when written/read to\n> the index extension so that the index format is the same no matter the\n> native size of time_t.\n\nOK. I guess your cast here will truncate on 32-bit systems, but\npresumably not until 2038, so we can perhaps ignore it for now (and\nanyway, time(NULL) will be broken on such a system at that point).\n\n-Peff\n"},{"id":"319888","messageId":"xmqq60h12y94.fsf@gitster.mtv.corp.google.com","threadId":"45972","inReplyTo":"20170515191347.1892-4-benpeart@microsoft.com","subject":"Re: [PATCH v1 3/5] fsmonitor: add test cases for fsmonitor extension","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-16T04:59:03Z","receivedAt":"2017-05-16T04:59:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> Add test cases that ensure status results are correct when using the new\n> fsmonitor extension. Test untracked, modified, and new files by\n> ensuring the results are identical to when not using the extension.\n>\n> Add a test to ensure updates to the index properly mark corresponding\n> entries in the index extension as dirty so that the status is correct\n> after commands that modify the index but don't trigger changes in the\n> working directory.\n>\n> Add a test that verifies that if the fsmonitor extension doesn't tell\n> git about a change, it doesn't discover it on its own. This ensures\n> git is honoring the extension and that we get the performance benefits\n> desired.\n>\n> Signed-off-by: Ben Peart \n> ---\n> t/t7519-status-fsmonitor.sh | 134 ++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 134 insertions(+)\n> create mode 100644 t/t7519-status-fsmonitor.sh\n\nPlease make this executable.\n\n> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n> new file mode 100644\n> index 0000000000..2d63efc27b\n> --- /dev/null\n> +++ b/t/t7519-status-fsmonitor.sh\n> @@ -0,0 +1,134 @@\n> ...\n> +# Ensure commands that call refresh_index() to move the index back in time\n> +# properly invalidate the fsmonitor cache\n> +...\n> +\tgit status >output &&\n> +\tgit -c core.fsmonitor=false status >expect &&\n> +\ttest_i18ncmp expect output\n> +'\n\nHmm. I wonder if we can somehow detect the case where we got the\ncorrect and expected result only because fsmonitor was not in\neffect, even though the test requested it to be used? Not limited\nto this particular test piece, but applies to all of them in this\nfile.\n"},{"id":"319889","messageId":"xmqq1srp2y6y.fsf@gitster.mtv.corp.google.com","threadId":"45972","inReplyTo":"20170515191347.1892-1-benpeart@microsoft.com","subject":"Re: [PATCH v1 0/5] Fast git status via a file system watcher","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-16T05:00:21Z","receivedAt":"2017-05-16T05:00:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> Add documentation for the fsmonitor extension. This includes the\n> core.fsmonitor setting, the query-fsmonitor hook, and the fsmonitor\n> index extension.\n> Add a sample query-fsmonitor hook script that integrates with the\n> cross platform Watchman file watching service.\n\nThese two have looong titles ;-) Accident?\n"},{"id":"319890","messageId":"xmqqwp9h1jkl.fsf@gitster.mtv.corp.google.com","threadId":"45972","inReplyTo":"20170515191347.1892-2-benpeart@microsoft.com","subject":"Re: [PATCH v1 1/5] dir: make lookup_untracked() available outside of dir.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-16T05:01:30Z","receivedAt":"2017-05-16T05:01:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Peart writes:\n\n> Remove the static qualifier from lookup_untracked() and make it\n> available to other modules by exporting it from dir.h.\n\nSurely that is what you did in this patch, but leaves readers in\nsuspense wondering why this helper needs to be available to others\nin the first place ;-) Let's read on.\n\n>\n> Signed-off-by: Ben Peart \n> ---\n> dir.c | 2 +-\n> dir.h | 3 +++\n> 2 files changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 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> {\n> diff --git a/dir.h b/dir.h\n> index 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"},{"id":"319921","messageId":"db527b7e-350a-751a-89e8-5e3312bf3610@gmail.com","threadId":"45972","inReplyTo":"xmqq60h12y94.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 3/5] fsmonitor: add test cases for fsmonitor extension","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2017-05-16T14:28:12Z","receivedAt":"2017-05-16T14:28:26Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/16/2017 12:59 AM, Junio C Hamano wrote:\n> Ben Peart writes:\n>\n>> Add test cases that ensure status results are correct when using the new\n>> fsmonitor extension. Test untracked, modified, and new files by\n>> ensuring the results are identical to when not using the extension.\n>>\n>> Add a test to ensure updates to the index properly mark corresponding\n>> entries in the index extension as dirty so that the status is correct\n>> after commands that modify the index but don't trigger changes in the\n>> working directory.\n>>\n>> Add a test that verifies that if the fsmonitor extension doesn't tell\n>> git about a change, it doesn't discover it on its own. This ensures\n>> git is honoring the extension and that we get the performance benefits\n>> desired.\n>>\n>> Signed-off-by: Ben Peart \n>> ---\n>> t/t7519-status-fsmonitor.sh | 134 ++++++++++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 134 insertions(+)\n>> create mode 100644 t/t7519-status-fsmonitor.sh\n>\n> Please make this executable.\n>\n\nSorry, long time Windows developer so I forgot this extra step. Fixed \nfor next roll.\n\n>> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh\n>> new file mode 100644\n>> index 0000000000..2d63efc27b\n>> --- /dev/null\n>> +++ b/t/t7519-status-fsmonitor.sh\n>> @@ -0,0 +1,134 @@\n>> ...\n>> +# Ensure commands that call refresh_index() to move the index back in time\n>> +# properly invalidate the fsmonitor cache\n>> +...\n>> +\tgit status >output &&\n>> +\tgit -c core.fsmonitor=false status >expect &&\n>> +\ttest_i18ncmp expect output\n>> +'\n>\n> Hmm. I wonder if we can somehow detect the case where we got the\n> correct and expected result only because fsmonitor was not in\n> effect, even though the test requested it to be used? Not limited\n> to this particular test piece, but applies to all of them in this\n> file.\n>\n\nI have tested this manually by editing the test hook proc to output \ninvalid results and ensured that the test failed as a result but adding \nthat to the test script was kind of ugly (all tests end up getting \nduplicated - one ensuring success, one ensuring failure).\n\nOn further reflection, a better idea is to have the test hook proc \noutput a marker file that can be tested for existence. If it exists, \nthe hook was used to update the results, if it doesn't exist, then the \nhook proc wasn't used. A much cleaner solution that doesn't require \nduplicating the tests.\n"},{"id":"319936","messageId":"29122818-71fb-5af9-59b1-03387f014151@gmail.com","threadId":"45972","inReplyTo":"2d965a87-36da-23b4-4bc5-97de47f3d7f7@gmail.com","subject":"Re: [PATCH v1 2/5] 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-05-16T17:17:56Z","receivedAt":"2017-05-16T17:18:03Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/15/2017 9:55 PM, Ben Peart wrote:\n>\n>\n> On 5/15/2017 8:34 PM, Jeff King wrote:\n>> On Tue, May 16, 2017 at 12:22:14AM +0000, brian m. carlson wrote:\n>>\n>>> On Mon, May 15, 2017 at 03:13:44PM -0400, Ben Peart wrote:\n>>>> + istate->last_update = (time_t)ntohll(*(uint64_t *)index);\n>>>> + index += sizeof(uint64_t);\n>>>> +\n>>>> + ewah_size = ntohl(*(uint32_t *)index);\n>>>> + index += sizeof(uint32_t);\n>>>\n>>> To answer the question you asked in your cover letter, you cannot write\n>>> this unless you can guarantee (((uintptr_t)index & 7) == 0) is true.\n>>> Otherwise, this will produce a SIGBUS on SPARC, Alpha, MIPS, and some\n>>> ARM systems, and it will perform poorly on PowerPC and other ARM\n>>> systems[0].\n>>>\n>>> If you got that pointer from malloc and have only indexed multiples of 8\n>>> on it, you're good. But if you're not sure, you probably want to use\n>>> memcpy. If the compiler can determine that it's not necessary, it will\n>>> omit the copy and perform a direct load.\n>>\n>> I think get_be32() does exactly what we want for the ewah_size read. For\n>> the last_update one, we don't have a get_be64() yet, but it should be\n>> easy to make based on the 16/32 versions.\n>\n> Thanks for the pointers. I'll update this to use the existing get_be32\n> and have created a get_be64 and will use that for the last_update.\n>\n\nOK, now I'm confused as to the best path for adding a get_be64. This \none is trivial:\n\n#define get_be64(p)\tntohll(*(uint64_t *)(p))\n\nbut should the unaligned version be:\n\n#define get_be64(p)\t( \\\n\t(*((unsigned char *)(p) + 0) << 56) | \\\n\t(*((unsigned char *)(p) + 1) << 48) | \\\n\t(*((unsigned char *)(p) + 2) << 40) | \\\n\t(*((unsigned char *)(p) + 3) << 32) | \\\n\t(*((unsigned char *)(p) + 4) << 24) | \\\n\t(*((unsigned char *)(p) + 5) << 16) | \\\n\t(*((unsigned char *)(p) + 6) << 8) | \\\n\t(*((unsigned char *)(p) + 7) << 0) )\n\nor would it be better to do it like this:\n\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\nor with a static inline function like git_bswap64:\n\nor something else entirely?\n\nI'm not sure why the different styles in this one file and which I \nshould be emulating.\n\n\n>>\n>> (I note also that time_t is not necessarily 64-bits in the first place,\n>> but David said something about this not really being a time_t).\n>>\n>\n> The in memory representation is a time_t as that is the return value of\n> time(NULL) but it is converted to/from a 64 bit value when written/read\n> to the index extension so that the index format is the same no matter\n> the native size of time_t.\n>\n>> -Peff\n>>\n"},{"id":"319945","messageId":"20170516174948.arqbk533pihm6x46@sigill.intra.peff.net","threadId":"45972","inReplyTo":"29122818-71fb-5af9-59b1-03387f014151@gmail.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-16T17:49:48Z","receivedAt":"2017-05-16T17:50:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 16, 2017 at 01:17:56PM -0400, Ben Peart wrote:\n\n> > Thanks for the pointers. I'll update this to use the existing get_be32\n> > and have created a get_be64 and will use that for the last_update.\n> \n> OK, now I'm confused as to the best path for adding a get_be64. This one is\n> trivial:\n> \n> #define get_be64(p)\tntohll(*(uint64_t *)(p))\n> \n> but should the unaligned version be:\n> \n> #define get_be64(p)\t( \\\n> \t(*((unsigned char *)(p) + 0) << 56) | \\\n> \t(*((unsigned char *)(p) + 1) << 48) | \\\n> \t(*((unsigned char *)(p) + 2) << 40) | \\\n> \t(*((unsigned char *)(p) + 3) << 32) | \\\n> \t(*((unsigned char *)(p) + 4) << 24) | \\\n> \t(*((unsigned char *)(p) + 5) << 16) | \\\n> \t(*((unsigned char *)(p) + 6) << 8) | \\\n> \t(*((unsigned char *)(p) + 7) << 0) )\n> \n> or would it be better to do it like this:\n> \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\nI'd imagine the compiler would generate quite similar code between the\ntwo, and the second is much shorter and easier to read, so I'd probably\nprefer it.\n\n> or with a static inline function like git_bswap64:\n\nTry \"git log -Sinline compat/bswap.h\", which turns up the history of why\nit went from a macro to an inline function.\n\nThe get_be macros are simple enough that they can remain as macros,\nthough I'd have no objection personally to them being inline functions.\nI'd expect modern compilers to be able to optimize similarly, and it\nremoves the restriction that you can't call the macro with an argument\nthat has side effects.\n\n-Peff\n"},{"id":"319953","messageId":"134ea57f-3a64-f7b5-67dd-8b14ff3cc04a@kdbg.org","threadId":"45972","inReplyTo":"29122818-71fb-5af9-59b1-03387f014151@gmail.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-05-16T19:13:58Z","receivedAt":"2017-05-16T19:14:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 16.05.2017 um 19:17 schrieb Ben Peart:\n> OK, now I'm confused as to the best path for adding a get_be64. This \n> one is trivial:\n> \n> #define get_be64(p) ntohll(*(uint64_t *)(p))\n\nI cringe when I see a cast like this. Unless you can guarantee that p is \nchar* (bare or signed or unsigned), you fall pray to strict aliasing \nviolations, aka undefined behavior. And I'm not even mentioning correct \nalignment, yet.\n\n-- Hannes\n"},{"id":"319967","messageId":"473c4b47-06a7-cb55-6d67-e335fa5b5a5b@google.com","threadId":"45972","inReplyTo":"20170515191347.1892-3-benpeart@microsoft.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-05-16T21:41:08Z","receivedAt":"2017-05-16T21:41:17Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"I'm not very familiar with this part of the code - here is a partial review.\n\nFirstly, if someone invokes update-index, I wonder if it's better just \nto do a full refresh (e.g. by deleting the last_update time from the index).\n\nAlso, the change to unpack-trees.c doesn't match my mental model. I \nnotice that it is in a function related to sparse checkout, but if the \nworking tree changes for whatever reason, it seems simpler to just let \nthe hook do its thing. As far as I can tell, it is fine to have files \noverzealously marked as FSMONITOR_DIRTY.\n\nOn 05/15/2017 12:13 PM, Ben Peart wrote:\n> diff --git a/cache.h b/cache.h\n> index 40ec032a2d..64aa6e57cd 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> +\ttime_t last_update;\n> +\tstruct ewah_bitmap *bitmap;\n\nHere a bitmap is introduced, presumably corresponding to the entries in \n\"struct cache_entry **cache\", but there is also a CE_FSMONITOR_DIRTY \nthat can be set in each \"struct cache_entry\". This seems redundant and \nprobably at least worth explaining in a comment.\n\n> +/*\n> + * Call the query-fsmonitor hook passing the time of the last saved results.\n> + */\n> +static int query_fsmonitor(time_t last_update, struct strbuf *buffer)\n> +{\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tchar date[64];\n> +\tconst char *argv[3];\n> +\n> +\tif (!(argv[0] = find_hook(\"query-fsmonitor\")))\n> +\t\treturn -1;\n> +\n> +\tsnprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n> +\targv[1] = date;\n> +\targv[2] = NULL;\n> +\tcp.argv = argv;\n> +\tcp.out = -1;\n> +\n> +\treturn capture_command(&cp, buffer, 1024);\n> +}\n\nOutput argument could probably be named better.\n\nAlso, would the output of this command be very large? If yes, it might \nbe better to process it little by little instead of buffering the whole \nthing first.\n\n> +void write_fsmonitor_extension(struct strbuf *sb, struct index_state* istate);\n\nSpace before * (in the .h and .c files).\n\n"},{"id":"320015","messageId":"45f4f321-bccf-b64f-0d20-af7603c9e8f8@gmail.com","threadId":"45972","inReplyTo":"473c4b47-06a7-cb55-6d67-e335fa5b5a5b@google.com","subject":"Re: [PATCH v1 2/5] 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-05-17T03:35:36Z","receivedAt":"2017-05-17T03:35:51Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/16/2017 5:41 PM, Jonathan Tan wrote:\n> I'm not very familiar with this part of the code - here is a partial\n> review.\n>\n> Firstly, if someone invokes update-index, I wonder if it's better just\n> to do a full refresh (e.g. by deleting the last_update time from the\n> index).\n\nA full refresh can be very expensive when the working directory is large \n(the specific case this patch series is trying to improve). Instead, \nthe code does the minimal update required to keep things fast but still \nreturn correct results.\n\n>\n> Also, the change to unpack-trees.c doesn't match my mental model. I\n> notice that it is in a function related to sparse checkout, but if the\n> working tree changes for whatever reason, it seems simpler to just let\n> the hook do its thing. As far as I can tell, it is fine to have files\n> overzealously marked as FSMONITOR_DIRTY.\n\nThe case this (and the others like it) is solving is when the index is \nupdated but there may not be any change to the associated file in the \nworking directory. When this occurs, the hook won't indicate any change \nhas happened so the index and working directory could be out of sync. \nTo be sure this doesn't happen, the index entry is marked \nCE_FSMONITOR_DIRTY to ensure the file is checked.\n\nThis is pretty simple to demonstrate - a simple \"git reset HEAD~1\" will \ndo it as a mixed reset updates the index but doesn't touch the files in \nthe working directory.\n\n>\n> On 05/15/2017 12:13 PM, Ben Peart wrote:\n>> diff --git a/cache.h b/cache.h\n>> index 40ec032a2d..64aa6e57cd 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\n>> mode)\n>> #define CACHE_TREE_CHANGED (1 << 5)\n>> #define SPLIT_INDEX_ORDERED (1 << 6)\n>> #define UNTRACKED_CHANGED (1 << 7)\n>> +#define FSMONITOR_CHANGED (1 << 8)\n>>\n>> struct split_index;\n>> struct untracked_cache;\n>> @@ -342,6 +344,8 @@ struct index_state {\n>> struct hashmap dir_hash;\n>> unsigned char sha1[20];\n>> struct untracked_cache *untracked;\n>> + time_t last_update;\n>> + struct ewah_bitmap *bitmap;\n>\n> Here a bitmap is introduced, presumably corresponding to the entries in\n> \"struct cache_entry **cache\", but there is also a CE_FSMONITOR_DIRTY\n> that can be set in each \"struct cache_entry\". This seems redundant and\n> probably at least worth explaining in a comment.\n>\n\nThe ewah bitmap is loaded from the index extension and saved until it \ncan be processed after the untracked cache has been loaded and \ninitialized in post_read_index_from(). I'm not opposed to documenting \nthat to make it clearer but I've just followed the same pattern the \nuntracked cache, and split index extensions use which don't specifically \ndocument it either.\n\n>> +/*\n>> + * Call the query-fsmonitor hook passing the time of the last saved\n>> results.\n>> + */\n>> +static int query_fsmonitor(time_t last_update, struct strbuf *buffer)\n>> +{\n>> + struct child_process cp = CHILD_PROCESS_INIT;\n>> + char date[64];\n>> + const char *argv[3];\n>> +\n>> + if (!(argv[0] = find_hook(\"query-fsmonitor\")))\n>> + return -1;\n>> +\n>> + snprintf(date, sizeof(date), \"%\" PRIuMAX, (uintmax_t)last_update);\n>> + argv[1] = date;\n>> + argv[2] = NULL;\n>> + cp.argv = argv;\n>> + cp.out = -1;\n>> +\n>> + return capture_command(&cp, buffer, 1024);\n>> +}\n>\n> Output argument could probably be named better.\n\nI agree. I've renamed it query_result for the next iteration.\n\n>\n> Also, would the output of this command be very large? If yes, it might\n> be better to process it little by little instead of buffering the whole\n> thing first.\n>\n\nThe output is usually quite small as it is is the list of files modified \nin the working directory since the last command that requested the \nupdated list.\n\n>> +void write_fsmonitor_extension(struct strbuf *sb, struct index_state*\n>> istate);\n>\n> Space before * (in the .h and .c files).\n>\n\nThanks, missed that. I'll fix it for the next iteration.\n\n"},{"id":"320082","messageId":"0a3f38b7-788b-b364-3fef-83191e4b9bea@gmail.com","threadId":"45972","inReplyTo":"134ea57f-3a64-f7b5-67dd-8b14ff3cc04a@kdbg.org","subject":"Re: [PATCH v1 2/5] 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-05-17T14:26:08Z","receivedAt":"2017-05-17T14:26:14Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 5/16/2017 3:13 PM, Johannes Sixt wrote:\n> Am 16.05.2017 um 19:17 schrieb Ben Peart:\n>> OK, now I'm confused as to the best path for adding a get_be64. This\n>> one is trivial:\n>>\n>> #define get_be64(p) ntohll(*(uint64_t *)(p))\n>\n> I cringe when I see a cast like this. Unless you can guarantee that p is\n> char* (bare or signed or unsigned), you fall pray to strict aliasing\n> violations, aka undefined behavior. And I'm not even mentioning correct\n> alignment, yet.\n>\n> -- Hannes\n\nNote, this macro is only used where the CPU architecture is OK with \nunaligned memory access. You can see it in context with many similar \nmacros and casts in bswap.h. It's outside the scope of this patch \nseries to fix them all. Perhaps a separate patch series?\n"},{"id":"320097","messageId":"2e5ea82d-3b8f-f508-e2af-5f193241a573@kdbg.org","threadId":"45972","inReplyTo":"0a3f38b7-788b-b364-3fef-83191e4b9bea@gmail.com","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-05-17T18:15:03Z","receivedAt":"2017-05-17T18:15:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.05.2017 um 16:26 schrieb Ben Peart:\n> On 5/16/2017 3:13 PM, Johannes Sixt wrote:\n>> Am 16.05.2017 um 19:17 schrieb Ben Peart:\n>>> OK, now I'm confused as to the best path for adding a get_be64. This\n>>> one is trivial:\n>>>\n>>> #define get_be64(p) ntohll(*(uint64_t *)(p))\n>>\n>> I cringe when I see a cast like this. Unless you can guarantee that p is\n>> char* (bare or signed or unsigned), you fall pray to strict aliasing\n>> violations, aka undefined behavior. And I'm not even mentioning correct\n>> alignment, yet.\n> \n> Note, this macro is only used where the CPU architecture is OK with \n> unaligned memory access.\n\nI'm not worried about the unaligned memory access: It either works, or \nwe get a SIGBUS. The undefined behavior is more worrisome because the \ncode may work or not, and we can never be sure which it is.\n\n-- Hannes\n"},{"id":"320135","messageId":"20170518045207.gd26sq5qdbxg6vnm@sigill.intra.peff.net","threadId":"45972","inReplyTo":"2e5ea82d-3b8f-f508-e2af-5f193241a573@kdbg.org","subject":"Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-18T04:52:07Z","receivedAt":"2017-05-18T04:52:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 17, 2017 at 08:15:03PM +0200, Johannes Sixt wrote:\n\n> Am 17.05.2017 um 16:26 schrieb Ben Peart:\n> > On 5/16/2017 3:13 PM, Johannes Sixt wrote:\n> > > Am 16.05.2017 um 19:17 schrieb Ben Peart:\n> > > > OK, now I'm confused as to the best path for adding a get_be64. This\n> > > > one is trivial:\n> > > > \n> > > > #define get_be64(p) ntohll(*(uint64_t *)(p))\n> > > \n> > > I cringe when I see a cast like this. Unless you can guarantee that p is\n> > > char* (bare or signed or unsigned), you fall pray to strict aliasing\n> > > violations, aka undefined behavior. And I'm not even mentioning correct\n> > > alignment, yet.\n> > \n> > Note, this macro is only used where the CPU architecture is OK with\n> > unaligned memory access.\n> \n> I'm not worried about the unaligned memory access: It either works, or we\n> get a SIGBUS. The undefined behavior is more worrisome because the code may\n> work or not, and we can never be sure which it is.\n\nI don't think there's much we can do, though. That's how all of the\nget_be* macros are designed to work (and there's really no point in\nusing them on something that isn't a char pointer).\n\nI agree it would be nice to have some type safety there if we can get\nit, though. I wonder if:\n\n static inline uint32_t get_be32(unsigned char *p)\n {\n\treturn ntohl(*(unsigned int *)p);\n }\n\nwould generate the same code. It does mean we may have problems between\nsigned/unsigned buffers, though.\n\n-Peff\n"}]}