{"thread":{"id":"42080","subject":"[PATCH v5 01/15] read-cache.c: fix constness of verify_hdr()","startedAt":"2016-04-19T23:27:54Z","lastAt":"2016-04-25T20:53:06Z","messageCount":35,"participants":["David Turner","Duy Nguyen","Johannes Schindelin","Nguyễn Thái Ngọc Duy"],"isPatch":true,"patchVersion":5,"patchTotal":15},"messages":[{"id":"283878","messageId":"1461108489-29376-1-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":null,"subject":"[PATCH v5 00/15] index-helper/watchman","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:54Z","receivedAt":"2016-04-19T23:27:54Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"This version includes the following changes since v4:\n1. The last patch has been removed; it's pretty much always a good\nidea to wait for the index-helper\n\n2. Documentation for index-helper --kill and --autorun. Documentation\nfor update-index --watchman. Documentation for index-format for WAMA.\nDocumentation for index-helper suggesting running update-index --watchman.\n\n3. index-helper autorun moved to read-cache so it's only run on relevant\ncommands.\n\n4. Tests: Broken && chain fixed; removed a subshell\n\n5. A couple of fd leaks fixed.\n\n6. Cruft removed.\n\n7. New perf numbers.\n\nDavid Turner (5):\n unpack-trees: preserve index extensions\n index-helper: kill mode\n index-helper: don't run if already running\n index-helper: autorun mode\n index-helper: optionally automatically run\n\nNguyễn Thái Ngọc Duy (10):\n read-cache.c: fix constness of verify_hdr()\n read-cache: allow to keep mmap'd memory after reading\n index-helper: new daemon for caching index and related stuff\n index-helper: add --strict\n daemonize(): set a flag before exiting the main process\n index-helper: add --detach\n read-cache: add watchman 'WAMA' extension\n Add watchman support to reduce index refresh cost\n index-helper: use watchman to avoid refreshing index with lstat()\n update-index: enable/disable watchman support\n\n .gitignore | 1 +\n Documentation/config.txt | 4 +\n Documentation/git-index-helper.txt | 75 +++++\n Documentation/git-update-index.txt | 6 +\n Documentation/technical/index-format.txt | 22 ++\n Makefile | 17 ++\n builtin/gc.c | 2 +-\n builtin/update-index.c | 11 +\n cache.h | 16 +-\n config.c | 5 +\n configure.ac | 8 +\n daemon.c | 2 +-\n dir.c | 23 +-\n dir.h | 6 +\n environment.c | 3 +\n git-compat-util.h | 1 +\n index-helper.c | 454 ++++++++++++++++++++++++++++\n read-cache.c | 488 ++++++++++++++++++++++++++++++-\n setup.c | 4 +-\n t/t7063-status-untracked-cache.sh | 22 ++\n t/t7900-index-helper.sh | 68 +++++\n t/test-lib-functions.sh | 4 +\n unpack-trees.c | 1 +\n watchman-support.c | 135 +++++++++\n watchman-support.h | 7 +\n 25 files changed, 1364 insertions(+), 21 deletions(-)\n create mode 100644 Documentation/git-index-helper.txt\n create mode 100644 index-helper.c\n create mode 100755 t/t7900-index-helper.sh\n create mode 100644 watchman-support.c\n create mode 100644 watchman-support.h\n\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283875","messageId":"1461108489-29376-2-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 01/15] read-cache.c: fix constness of verify_hdr()","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:55Z","receivedAt":"2016-04-19T23:27:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n read-cache.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex d9fb78b..16cc487 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1345,7 +1345,7 @@ struct ondisk_cache_entry_extended {\n \t\t\t ondisk_cache_entry_extended_size(ce_namelen(ce)) : \\\n \t\t\t ondisk_cache_entry_size(ce_namelen(ce)))\n \n-static int verify_hdr(struct cache_header *hdr, unsigned long size)\n+static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n {\n \tgit_SHA_CTX c;\n \tunsigned char sha1[20];\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283877","messageId":"1461108489-29376-3-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:56Z","receivedAt":"2016-04-19T23:27:56Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nLater, we will introduce git index-helper to share this memory with\nother git processes.\n\nSince the memory will be shared, it will never be unmapped (although\nthe kernel may of course choose to page it out).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n cache.h | 3 +++\n read-cache.c | 13 ++++++++++++-\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex b829410..4180e2b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -333,11 +333,14 @@ struct index_state {\n \tstruct split_index *split_index;\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n+\t\t keep_mmap : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\n \tunsigned char sha1[20];\n \tstruct untracked_cache *untracked;\n+\tvoid *mmap;\n+\tsize_t mmap_size;\n };\n \n extern struct index_state the_index;\ndiff --git a/read-cache.c b/read-cache.c\nindex 16cc487..7e387e9 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1574,6 +1574,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tmmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tif (mmap == MAP_FAILED)\n \t\tdie_errno(\"unable to map index file\");\n+\tif (istate->keep_mmap) {\n+\t\tistate->mmap = mmap;\n+\t\tistate->mmap_size = mmap_size;\n+\t}\n \tclose(fd);\n \n \thdr = mmap;\n@@ -1626,10 +1630,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \t\tsrc_offset += 8;\n \t\tsrc_offset += extsize;\n \t}\n-\tmunmap(mmap, mmap_size);\n+\tif (!istate->keep_mmap)\n+\t\tmunmap(mmap, mmap_size);\n \treturn istate->cache_nr;\n \n unmap:\n+\tistate->mmap = NULL;\n \tmunmap(mmap, mmap_size);\n \tdie(\"index file corrupt\");\n }\n@@ -1655,6 +1661,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\tdiscard_index(split_index->base);\n \telse\n \t\tsplit_index->base = xcalloc(1, sizeof(*split_index->base));\n+\tsplit_index->base->keep_mmap = istate->keep_mmap;\n \tret = do_read_index(split_index->base,\n \t\t\t git_path(\"sharedindex.%s\",\n \t\t\t\t sha1_to_hex(split_index->base_sha1)), 1);\n@@ -1698,6 +1705,10 @@ int discard_index(struct index_state *istate)\n \tfree(istate->cache);\n \tistate->cache = NULL;\n \tistate->cache_alloc = 0;\n+\tif (istate->keep_mmap && istate->mmap) {\n+\t\tmunmap(istate->mmap, istate->mmap_size);\n+\t\tistate->mmap = NULL;\n+\t}\n \tdiscard_split_index(istate);\n \tfree_untracked_cache(istate->untracked);\n \tistate->untracked = NULL;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283881","messageId":"1461108489-29376-4-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 03/15] index-helper: new daemon for caching index and related stuff","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:57Z","receivedAt":"2016-04-19T23:27:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nInstead of reading the index from disk and worrying about disk\ncorruption, the index is cached in memory (memory bit-flips happen\ntoo, but hopefully less often). The result is faster read. Read time\nis reduced by 70%.\n\nThe biggest gain is not having to verify the trailing SHA-1, which\ntakes lots of time especially on large index files. But this also\nopens doors for further optimiztions:\n\n - we could create an in-memory format that's essentially the memory\n dump of the index to eliminate most of parsing/allocation\n overhead. The mmap'd memory can be used straight away. Experiment\n [1] shows we could reduce read time by 88%.\n\n - we could cache non-index info such as name hash\n\nShared memory is done by storing files in a per-repository temporary\ndirectory. This is more portable than shm (which requires\nposix-realtime and has various quirks on OS X). It might even work on\nWindows, although this has not been tested. The shared memory file's\nname folows the template \"shm--\" where is the\ntrailing SHA-1 of the index file. is \"index\" for cached index\nfiles (and might later be \"name-hash\" for name-hash cache). If such\nshared memory exists, it contains the same index content as on\ndisk. The content is already validated by the daemon and git won't\nvalidate it again (except comparing the trailing SHA-1s).\n\nWe keep this daemon's logic as thin as possible. The \"brain\" stays in\ngit. So the daemon can read and validate stuff, but that's all it's\nallowed to do. It does not add/create new information. It doesn't even\naccept direct updates from git.\n\nGit can poke the daemon via unix domain sockets to tell it to refresh\nthe index cache, or to keep it alive some more minutes. It can't give\nany real index data directly to the daemon. Real data goes to disk\nfirst, then the daemon reads and verifies it from there. Poking only\nhappens for $GIT_DIR/index, not temporary index files.\n\n$GIT_DIR/index-helper.sock is the socket for the daemon process. The\ndaemon reads from the socket and executes commands.\n\nNamed pipes were considered for portability reasons, but then commands\nthat need replies from the daemon would have open their own pipes,\nsince a named pipe should only have one reader. Unix domain sockets\ndon't have this problem.\n\nOn webkit.git with index format v2, duplicating 8 times to 1.5m\nentries and 236MB in size:\n\n(vanilla) 0.50 s: read_index_from .git/index\n(index-helper) 0.18 s: read_index_from .git/index\n\nInterestingly with index v4, we get less out of index-helper. It makes\nsense as v4 requires more processing after loading the index:\n\n(vanilla) 0.37 s: read_index_from .git/index\n(index-helper) 0.22 s: read_index_from .git/index\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/247268/focus=248771\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \nSigned-off-by: Ramsay Jones \n---\n .gitignore | 1 +\n Documentation/git-index-helper.txt | 47 +++++++\n Makefile | 5 +\n cache.h | 2 +\n git-compat-util.h | 1 +\n index-helper.c | 276 +++++++++++++++++++++++++++++++++++++\n read-cache.c | 120 ++++++++++++++--\n t/t7900-index-helper.sh | 23 ++++\n 8 files changed, 466 insertions(+), 9 deletions(-)\n create mode 100644 Documentation/git-index-helper.txt\n create mode 100644 index-helper.c\n create mode 100755 t/t7900-index-helper.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 5087ce1..b92f122 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -71,6 +71,7 @@\n /git-http-fetch\n /git-http-push\n /git-imap-send\n+/git-index-helper\n /git-index-pack\n /git-init\n /git-init-db\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nnew file mode 100644\nindex 0000000..77687c0\n--- /dev/null\n+++ b/Documentation/git-index-helper.txt\n@@ -0,0 +1,47 @@\n+git-index-helper(1)\n+===================\n+\n+NAME\n+----\n+git-index-helper - A simple cache daemon for speeding up index file access\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git index-helper' [options]\n+\n+DESCRIPTION\n+-----------\n+Keep the index file in memory for faster access. This daemon is per\n+repository.\n+\n+OPTIONS\n+-------\n+\n+--exit-after=::\n+\tExit if the cached index is not accessed for ``\n+\tseconds. Specify 0 to wait forever. Default is 600.\n+\n+NOTES\n+-----\n+\n+$GIT_DIR/index-helper.sock a Unix domain socket that the daemon reads\n+commands from. The directory will also contain files named\n+\"shm-index-\". These are used as backing stores for shared\n+memory. Normally the daemon will clean up these files when it exits\n+or when they are no longer relevant. But if it crashes, some objects\n+could remain there and they can be safely deleted with \"rm\"\n+command. The following commands are used to control the daemon:\n+\n+\"refresh\"::\n+\tReread the index.\n+\n+\"poke\":\n+\tLet the daemon know the index is to be read. It keeps the\n+\tdaemon alive longer, unless `--exit-after=0` is used.\n+\n+All commands and replies are terminated by a 0 byte.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 2742a69..c8be0e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1433,6 +1433,10 @@ ifdef HAVE_DEV_TTY\n \tBASIC_CFLAGS += -DHAVE_DEV_TTY\n endif\n \n+ifndef NO_MMAP\n+\tPROGRAM_OBJS += index-helper.o\n+endif\n+\n ifdef DIR_HAS_BSD_GROUP_SEMANTICS\n \tCOMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS\n endif\n@@ -2159,6 +2163,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@+\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n+\t@echo NO_MMAP=\\''$(subst ','\\'',$(subst ','\\'',$(NO_MMAP)))'\\' >>$@+\n ifdef TEST_OUTPUT_DIRECTORY\n \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n endif\ndiff --git a/cache.h b/cache.h\nindex 4180e2b..43fb314 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -334,6 +334,8 @@ struct index_state {\n \tstruct cache_time timestamp;\n \tunsigned name_hash_initialized : 1,\n \t\t keep_mmap : 1,\n+\t\t from_shm : 1,\n+\t\t to_shm : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex c07e0c1..56945a7 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -513,6 +513,7 @@ static inline int ends_with(const char *str, const char *suffix)\n #define PROT_READ 1\n #define PROT_WRITE 2\n #define MAP_PRIVATE 1\n+#define MAP_SHARED 2\n #endif\n \n #define mmap git_mmap\ndiff --git a/index-helper.c b/index-helper.c\nnew file mode 100644\nindex 0000000..d64e49b\n--- /dev/null\n+++ b/index-helper.c\n@@ -0,0 +1,276 @@\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"sigchain.h\"\n+#include \"strbuf.h\"\n+#include \"exec_cmd.h\"\n+#include \"split-index.h\"\n+#include \"lockfile.h\"\n+#include \"cache.h\"\n+#include \"unix-socket.h\"\n+\n+struct shm {\n+\tunsigned char sha1[20];\n+\tvoid *shm;\n+\tsize_t size;\n+};\n+\n+static struct shm shm_index;\n+static struct shm shm_base_index;\n+\n+static void release_index_shm(struct shm *is)\n+{\n+\tif (!is->shm)\n+\t\treturn;\n+\tmunmap(is->shm, is->size);\n+\tunlink(git_path(\"shm-index-%s\", sha1_to_hex(is->sha1)));\n+\tis->shm = NULL;\n+}\n+\n+static void cleanup_shm(void)\n+{\n+\trelease_index_shm(&shm_index);\n+\trelease_index_shm(&shm_base_index);\n+}\n+\n+static void cleanup(void)\n+{\n+\tunlink(git_path(\"index-helper.sock\"));\n+\tcleanup_shm();\n+}\n+\n+static void cleanup_on_signal(int sig)\n+{\n+\t/* We ignore sigpipes -- that's just a client being broken. */\n+\tif (sig == SIGPIPE)\n+\t\treturn;\n+\tcleanup();\n+\tsigchain_pop(sig);\n+\traise(sig);\n+}\n+\n+static int shared_mmap_create(int file_flags, int file_mode, size_t size,\n+\t\t\t void **new_mmap, int mmap_prot, int mmap_flags,\n+\t\t\t const char *path)\n+{\n+\tint fd = -1;\n+\tint ret = -1;\n+\n+\tfd = open(path, file_flags, file_mode);\n+\n+\tif (fd < 0)\n+\t\tgoto done;\n+\n+\tif (ftruncate(fd, size))\n+\t\tgoto done;\n+\n+\t*new_mmap = mmap(NULL, size, mmap_prot, mmap_flags, fd, 0);\n+\n+\tif (*new_mmap == MAP_FAILED) {\n+\t\t*new_mmap = NULL;\n+\t\tgoto done;\n+\t}\n+\tmadvise(new_mmap, size, MADV_WILLNEED);\n+\n+\tret = 0;\n+done:\n+\tif (fd > 0)\n+\t\tclose(fd);\n+\treturn ret;\n+}\n+\n+static void share_index(struct index_state *istate, struct shm *is)\n+{\n+\tvoid *new_mmap;\n+\tif (istate->mmap_size <= 20 ||\n+\t hashcmp(istate->sha1,\n+\t\t (unsigned char *)istate->mmap + istate->mmap_size - 20) ||\n+\t !hashcmp(istate->sha1, is->sha1) ||\n+\t shared_mmap_create(O_CREAT | O_EXCL | O_RDWR, 0700,\n+\t\t\t istate->mmap_size, &new_mmap,\n+\t\t\t PROT_READ | PROT_WRITE, MAP_SHARED,\n+\t\t\t git_path(\"shm-index-%s\",\n+\t\t\t\t\tsha1_to_hex(istate->sha1))) < 0)\n+\t\treturn;\n+\n+\trelease_index_shm(is);\n+\tis->size = istate->mmap_size;\n+\tis->shm = new_mmap;\n+\thashcpy(is->sha1, istate->sha1);\n+\n+\tmemcpy(new_mmap, istate->mmap, istate->mmap_size - 20);\n+\n+\t/*\n+\t * The trailing hash must be written last after everything is\n+\t * written. It's the indication that the shared memory is now\n+\t * ready.\n+\t * The memory barrier here matches read-cache.c:try_shm.\n+\t */\n+\t__sync_synchronize();\n+\n+\thashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);\n+}\n+\n+static void share_the_index(void)\n+{\n+\tif (the_index.split_index && the_index.split_index->base)\n+\t\tshare_index(the_index.split_index->base, &shm_base_index);\n+\tshare_index(&the_index, &shm_index);\n+\tdiscard_index(&the_index);\n+}\n+\n+static void set_socket_blocking_flag(int fd, int make_nonblocking)\n+{\n+\tint flags;\n+\n+\tflags = fcntl(fd, F_GETFL, NULL);\n+\n+\tif (flags < 0)\n+\t\tdie(_(\"fcntl failed\"));\n+\n+\tif (make_nonblocking)\n+\t\tflags |= O_NONBLOCK;\n+\telse\n+\t\tflags &= ~O_NONBLOCK;\n+\n+\tif (fcntl(fd, F_SETFL, flags) < 0)\n+\t\tdie(_(\"fcntl failed\"));\n+}\n+\n+static void refresh(void)\n+{\n+\tdiscard_index(&the_index);\n+\tthe_index.keep_mmap = 1;\n+\tthe_index.to_shm = 1;\n+\tif (read_cache() < 0)\n+\t\tdie(_(\"could not read index\"));\n+\tshare_the_index();\n+}\n+\n+#ifndef NO_MMAP\n+\n+static void loop(int fd, int idle_in_seconds)\n+{\n+\tstruct timeval timeout;\n+\tstruct timeval *timeout_p;\n+\n+\twhile (1) {\n+\t\tfd_set readfds;\n+\t\tint result, client_fd;\n+\t\tstruct strbuf command = STRBUF_INIT;\n+\n+\t\t/* need to reset timer in case select() decremented it */\n+\t\tif (idle_in_seconds) {\n+\t\t\ttimeout.tv_usec = 0;\n+\t\t\ttimeout.tv_sec = idle_in_seconds;\n+\t\t\ttimeout_p = &timeout;\n+\t\t} else {\n+\t\t\ttimeout_p = NULL;\n+\t\t}\n+\n+\t\t/* Wait for a request */\n+\t\tFD_ZERO(&readfds);\n+\t\tFD_SET(fd, &readfds);\n+\t\tresult = select(fd + 1, &readfds, NULL, NULL, timeout_p);\n+\t\tif (result < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\t/*\n+\t\t\t\t * This can lead to an overlong keepalive,\n+\t\t\t\t * but that is better than a premature exit.\n+\t\t\t\t */\n+\t\t\t\tcontinue;\n+\t\t\tdie_errno(_(\"select() failed\"));\n+\t\t}\n+\t\tif (result == 0)\n+\t\t\t/* timeout */\n+\t\t\tbreak;\n+\n+\t\tclient_fd = accept(fd, NULL, NULL);\n+\t\tif (client_fd < 0)\n+\t\t\t/*\n+\t\t\t * An error here is unlikely -- it probably\n+\t\t\t * indicates that the connecting process has\n+\t\t\t * already dropped the connection.\n+\t\t\t */\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * Our connection to the client is blocking since a client\n+\t\t * can always be killed by SIGINT or similar.\n+\t\t */\n+\t\tset_socket_blocking_flag(client_fd, 0);\n+\n+\t\tif (strbuf_getwholeline_fd(&command, client_fd, '\\0') == 0) {\n+\t\t\tif (!strcmp(command.buf, \"refresh\")) {\n+\t\t\t\trefresh();\n+\t\t\t} else if (!strcmp(command.buf, \"poke\")) {\n+\t\t\t\t/*\n+\t\t\t\t * Just a poke to keep us\n+\t\t\t\t * alive, nothing to do.\n+\t\t\t\t */\n+\t\t\t} else {\n+\t\t\t\twarning(\"BUG: Bogus command %s\", command.buf);\n+\t\t\t}\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * No command from client. Probably it's just\n+\t\t\t * a liveness check. Just close up.\n+\t\t\t */\n+\t\t}\n+\t\tclose(client_fd);\n+\t\tstrbuf_release(&command);\n+\t}\n+\n+\tclose(fd);\n+}\n+\n+#else\n+\n+static void loop(int fd, int idle_in_seconds)\n+{\n+\tdie(_(\"index-helper is not supported on this platform\"));\n+}\n+\n+#endif\n+\n+static const char * const usage_text[] = {\n+\tN_(\"git index-helper [options]\"),\n+\tNULL\n+};\n+\n+int main(int argc, char **argv)\n+{\n+\tconst char *prefix;\n+\tint idle_in_seconds = 600;\n+\tint fd;\n+\tstruct strbuf socket_path = STRBUF_INIT;\n+\tstruct option options[] = {\n+\t\tOPT_INTEGER(0, \"exit-after\", &idle_in_seconds,\n+\t\t\t N_(\"exit if not used after some seconds\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_extract_argv0_path(argv[0]);\n+\tgit_setup_gettext();\n+\n+\tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n+\t\tusage_with_options(usage_text, options);\n+\n+\tprefix = setup_git_directory();\n+\tif (parse_options(argc, (const char **)argv, prefix,\n+\t\t\t options, usage_text, 0))\n+\t\tdie(_(\"too many arguments\"));\n+\n+\tatexit(cleanup);\n+\tsigchain_push_common(cleanup_on_signal);\n+\n+\tstrbuf_git_path(&socket_path, \"index-helper.sock\");\n+\n+\tfd = unix_stream_listen(socket_path.buf);\n+\tif (fd < 0)\n+\t\tdie_errno(_(\"could not set up index-helper socket\"));\n+\n+\tloop(fd, idle_in_seconds);\n+\n+\tclose(fd);\n+\treturn 0;\n+}\ndiff --git a/read-cache.c b/read-cache.c\nindex 7e387e9..af2101f 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 \"unix-socket.h\"\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\n \t\t\t\t\t unsigned int options);\n@@ -1541,6 +1542,95 @@ static void post_read_index_from(struct index_state *istate)\n \ttweak_untracked_cache(istate);\n }\n \n+static int poke_daemon(struct index_state *istate,\n+\t\t const struct stat *st, int refresh_cache)\n+{\n+\tint fd;\n+\tint ret = 0;\n+\tconst char *socket_path;\n+\n+\t/* if this is from index-helper, do not poke itself (recursively) */\n+\tif (istate->to_shm)\n+\t\treturn 0;\n+\n+\tsocket_path = git_path(\"index-helper.sock\");\n+\tif (!socket_path)\n+\t\treturn -1;\n+\n+\tfd = unix_stream_connect(socket_path);\n+\tif (refresh_cache) {\n+\t\tret = write_in_full(fd, \"refresh\", 8) != 8;\n+\t} else {\n+\t\tret = write_in_full(fd, \"poke\", 5) != 5;\n+\t}\n+\n+\tclose(fd);\n+\treturn ret;\n+}\n+\n+static int is_main_index(struct index_state *istate)\n+{\n+\treturn istate == &the_index ||\n+\t\t(the_index.split_index &&\n+\t\t istate == the_index.split_index->base);\n+}\n+\n+/*\n+ * Try to open and verify a cached shm index if available. Return 0 if\n+ * succeeds (istate->mmap and istate->mmap_size are updated). Return\n+ * negative otherwise.\n+ */\n+static int try_shm(struct index_state *istate)\n+{\n+\tvoid *new_mmap = NULL;\n+\tsize_t old_size = istate->mmap_size;\n+\tssize_t new_size;\n+\tconst unsigned char *sha1;\n+\tstruct stat st;\n+\tint fd = -1;\n+\n+\tif (!is_main_index(istate) ||\n+\t old_size <= 20 ||\n+\t stat(git_path(\"index-helper.sock\"), &st))\n+\t\treturn -1;\n+\tif (poke_daemon(istate, &st, 0))\n+\t\treturn -1;\n+\tsha1 = (unsigned char *)istate->mmap + old_size - 20;\n+\n+\tfd = open(git_path(\"shm-index-%s\", sha1_to_hex(sha1)), O_RDONLY);\n+\tif (fd < 0)\n+\t\tgoto fail;\n+\n+\tif (fstat(fd, &st))\n+\t\tgoto fail;\n+\n+\tnew_size = st.st_size;\n+\tnew_mmap = mmap(NULL, new_size, PROT_READ, MAP_SHARED, fd, 0);\n+\tif (new_size <= 20 ||\n+\t hashcmp((unsigned char *)istate->mmap + old_size - 20,\n+\t\t (unsigned char *)new_mmap + new_size - 20)) {\n+\t\tif (new_mmap)\n+\t\t\tmunmap(new_mmap, new_size);\n+\t\tgoto fail;\n+\t}\n+\n+\t/* The memory barrier here matches index-helper.c:share_index. */\n+\t__sync_synchronize();\n+\n+\tmunmap(istate->mmap, istate->mmap_size);\n+\tistate->mmap = new_mmap;\n+\tistate->mmap_size = new_size;\n+\tistate->from_shm = 1;\n+\tclose(fd);\n+\treturn 0;\n+\n+fail:\n+\tif (fd >= 0)\n+\t\tclose(fd);\n+\tpoke_daemon(istate, &st, 1);\n+\treturn -1;\n+}\n+\n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n {\n@@ -1555,6 +1645,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tif (istate->initialized)\n \t\treturn istate->cache_nr;\n \n+\tistate->from_shm = 0;\n \tistate->timestamp.sec = 0;\n \tistate->timestamp.nsec = 0;\n \tfd = open(path, O_RDONLY);\n@@ -1574,15 +1665,17 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tmmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);\n \tif (mmap == MAP_FAILED)\n \t\tdie_errno(\"unable to map index file\");\n-\tif (istate->keep_mmap) {\n-\t\tistate->mmap = mmap;\n-\t\tistate->mmap_size = mmap_size;\n-\t}\n \tclose(fd);\n \n-\thdr = mmap;\n-\tif (verify_hdr(hdr, mmap_size) < 0)\n+\tistate->mmap = mmap;\n+\tistate->mmap_size = mmap_size;\n+\tif (try_shm(istate) &&\n+\t verify_hdr(istate->mmap, istate->mmap_size) < 0)\n \t\tgoto unmap;\n+\thdr = mmap = istate->mmap;\n+\tmmap_size = istate->mmap_size;\n+\tif (!istate->keep_mmap)\n+\t\tistate->mmap = NULL;\n \n \thashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 20);\n \tistate->version = ntohl(hdr->hdr_version);\n@@ -1662,6 +1755,8 @@ int read_index_from(struct index_state *istate, const char *path)\n \telse\n \t\tsplit_index->base = xcalloc(1, sizeof(*split_index->base));\n \tsplit_index->base->keep_mmap = istate->keep_mmap;\n+\tsplit_index->base->to_shm = istate->to_shm;\n+\tsplit_index->base->from_shm = istate->from_shm;\n \tret = do_read_index(split_index->base,\n \t\t\t git_path(\"sharedindex.%s\",\n \t\t\t\t sha1_to_hex(split_index->base_sha1)), 1);\n@@ -1712,6 +1807,8 @@ int discard_index(struct index_state *istate)\n \tdiscard_split_index(istate);\n \tfree_untracked_cache(istate->untracked);\n \tistate->untracked = NULL;\n+\tistate->from_shm = 0;\n+\tistate->to_shm = 0;\n \treturn 0;\n }\n \n@@ -2138,9 +2235,14 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n \t\treturn ret;\n \tassert((flags & (COMMIT_LOCK | CLOSE_LOCK)) !=\n \t (COMMIT_LOCK | CLOSE_LOCK));\n-\tif (flags & COMMIT_LOCK)\n-\t\treturn commit_locked_index(lock);\n-\telse if (flags & CLOSE_LOCK)\n+\tif (flags & COMMIT_LOCK) {\n+\t\tstruct stat st;\n+\t\tret = commit_locked_index(lock);\n+\t\tif (!ret && is_main_index(istate) &&\n+\t\t !stat(git_path(\"index-helper.sock\"), &st))\n+\t\t\tpoke_daemon(istate, &st, 1);\n+\t\treturn ret;\n+\t} else if (flags & CLOSE_LOCK)\n \t\treturn close_lock_file(lock);\n \telse\n \t\treturn ret;\ndiff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh\nnew file mode 100755\nindex 0000000..114c112\n--- /dev/null\n+++ b/t/t7900-index-helper.sh\n@@ -0,0 +1,23 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2016, Twitter, Inc\n+#\n+\n+test_description='git-index-helper\n+\n+Testing git index-helper\n+'\n+\n+. ./test-lib.sh\n+\n+test -n \"$NO_MMAP\" && {\n+\tskip_all='skipping index-helper tests: no mmap'\n+\ttest_done\n+}\n+\n+test_expect_success 'index-helper smoke test' '\n+\tgit index-helper --exit-after 1 &&\n+\ttest_path_is_missing .git/index-helper.sock\n+'\n+\n+test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283876","messageId":"1461108489-29376-5-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 04/15] index-helper: add --strict","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:58Z","receivedAt":"2016-04-19T23:27:58Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nThere are \"holes\" in the index-helper approach because the shared\nmemory is not verified again by git. If $USER is compromised, shared\nmemory could be modified. But anyone who could do this could already\nmodify $GIT_DIR/index. A more realistic risk is some bugs in\nindex-helper that produce corrupt shared memory. --strict is added to\navoid that.\n\nStrictly speaking there's still a very small gap where corrupt shared\nmemory could still be read by git: after we write the trailing SHA-1 in\nthe shared memory (thus signaling \"this shm is ready\") and before\nverify_shm() detects an error.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 9 +++++++\n cache.h | 1 +\n index-helper.c | 48 ++++++++++++++++++++++++++++++++++++++\n read-cache.c | 9 ++++---\n 4 files changed, 64 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex 77687c0..a5c058f 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -22,6 +22,15 @@ OPTIONS\n \tExit if the cached index is not accessed for ``\n \tseconds. Specify 0 to wait forever. Default is 600.\n \n+--strict::\n+--no-strict::\n+\tStrict mode makes index-helper verify the shared memory after\n+\tit's created. If the result does not match what's read from\n+\t$GIT_DIR/index, the shared memory is destroyed. This makes\n+\tindex-helper take more than double the amount of time required\n+\tfor reading an index, but because it will happen in the\n+\tbackground, it's not noticable. `--strict` is enabled by default.\n+\n NOTES\n -----\n \ndiff --git a/cache.h b/cache.h\nindex 43fb314..4b678e9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -336,6 +336,7 @@ struct index_state {\n \t\t keep_mmap : 1,\n \t\t from_shm : 1,\n \t\t to_shm : 1,\n+\t\t always_verify_trailing_sha1 : 1,\n \t\t initialized : 1;\n \tstruct hashmap name_hash;\n \tstruct hashmap dir_hash;\ndiff --git a/index-helper.c b/index-helper.c\nindex d64e49b..e8d6b64 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -16,6 +16,7 @@ struct shm {\n \n static struct shm shm_index;\n static struct shm shm_base_index;\n+static int to_verify = 1;\n \n static void release_index_shm(struct shm *is)\n {\n@@ -110,11 +111,56 @@ static void share_index(struct index_state *istate, struct shm *is)\n \thashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);\n }\n \n+static int verify_shm(void)\n+{\n+\tint i;\n+\tstruct index_state istate;\n+\tmemset(&istate, 0, sizeof(istate));\n+\tistate.always_verify_trailing_sha1 = 1;\n+\tistate.to_shm = 1;\n+\ti = read_index(&istate);\n+\tif (i != the_index.cache_nr)\n+\t\tgoto done;\n+\tfor (; i < the_index.cache_nr; i++) {\n+\t\tstruct cache_entry *base, *ce;\n+\t\t/* namelen is checked separately */\n+\t\tconst unsigned int ondisk_flags =\n+\t\t\tCE_STAGEMASK | CE_VALID | CE_EXTENDED_FLAGS;\n+\t\tunsigned int ce_flags, base_flags, ret;\n+\t\tbase = the_index.cache[i];\n+\t\tce = istate.cache[i];\n+\t\tif (ce->ce_namelen != base->ce_namelen ||\n+\t\t strcmp(ce->name, base->name)) {\n+\t\t\twarning(\"mismatch at entry %d\", i);\n+\t\t\tbreak;\n+\t\t}\n+\t\tce_flags = ce->ce_flags;\n+\t\tbase_flags = base->ce_flags;\n+\t\t/* only on-disk flags matter */\n+\t\tce->ce_flags &= ondisk_flags;\n+\t\tbase->ce_flags &= ondisk_flags;\n+\t\tret = memcmp(&ce->ce_stat_data, &base->ce_stat_data,\n+\t\t\t offsetof(struct cache_entry, name) -\n+\t\t\t offsetof(struct cache_entry, ce_stat_data));\n+\t\tce->ce_flags = ce_flags;\n+\t\tbase->ce_flags = base_flags;\n+\t\tif (ret) {\n+\t\t\twarning(\"mismatch at entry %d\", i);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+done:\n+\tdiscard_index(&istate);\n+\treturn i == the_index.cache_nr;\n+}\n+\n static void share_the_index(void)\n {\n \tif (the_index.split_index && the_index.split_index->base)\n \t\tshare_index(the_index.split_index->base, &shm_base_index);\n \tshare_index(&the_index, &shm_index);\n+\tif (to_verify && !verify_shm())\n+\t\tcleanup_shm();\n \tdiscard_index(&the_index);\n }\n \n@@ -246,6 +292,8 @@ int main(int argc, char **argv)\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"exit-after\", &idle_in_seconds,\n \t\t\t N_(\"exit if not used after some seconds\")),\n+\t\tOPT_BOOL(0, \"strict\", &to_verify,\n+\t\t\t \"verify shared memory after creating\"),\n \t\tOPT_END()\n \t};\n \ndiff --git a/read-cache.c b/read-cache.c\nindex af2101f..5df5b21 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1669,9 +1669,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \n \tistate->mmap = mmap;\n \tistate->mmap_size = mmap_size;\n-\tif (try_shm(istate) &&\n-\t verify_hdr(istate->mmap, istate->mmap_size) < 0)\n-\t\tgoto unmap;\n+\tif (try_shm(istate)) {\n+\t\tif (verify_hdr(istate->mmap, istate->mmap_size) < 0)\n+\t\t\tgoto unmap;\n+\t} else if (istate->always_verify_trailing_sha1 &&\n+\t\t verify_hdr(istate->mmap, istate->mmap_size) < 0)\n+\t\t\tgoto unmap;\n \thdr = mmap = istate->mmap;\n \tmmap_size = istate->mmap_size;\n \tif (!istate->keep_mmap)\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283884","messageId":"1461108489-29376-6-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 05/15] daemonize(): set a flag before exiting the main process","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:27:59Z","receivedAt":"2016-04-19T23:27:59Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nThis allows signal handlers and atexit functions to realize this\nsituation and not clean up.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n builtin/gc.c | 2 +-\n cache.h | 2 +-\n daemon.c | 2 +-\n setup.c | 4 +++-\n 4 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c583aad..37180de 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -385,7 +385,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\t * failure to daemonize is ok, we'll continue\n \t\t\t * in foreground\n \t\t\t */\n-\t\t\tdaemonized = !daemonize();\n+\t\t\tdaemonized = !daemonize(NULL);\n \t\t}\n \t} else\n \t\tadd_repack_all_option();\ndiff --git a/cache.h b/cache.h\nindex 4b678e9..0aeb994 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -530,7 +530,7 @@ extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n extern int init_db(const char *template_dir, unsigned int flags);\n \n extern void sanitize_stdfds(void);\n-extern int daemonize(void);\n+extern int daemonize(int *);\n \n #define alloc_nr(x) (((x)+16)*3/2)\n \ndiff --git a/daemon.c b/daemon.c\nindex 8d45c33..a5cf954 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1365,7 +1365,7 @@ int main(int argc, char **argv)\n \t\treturn execute();\n \n \tif (detach) {\n-\t\tif (daemonize())\n+\t\tif (daemonize(NULL))\n \t\t\tdie(\"--detach not supported on this platform\");\n \t} else\n \t\tsanitize_stdfds();\ndiff --git a/setup.c b/setup.c\nindex de1a2a7..9adf13f 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1017,7 +1017,7 @@ void sanitize_stdfds(void)\n \t\tclose(fd);\n }\n \n-int daemonize(void)\n+int daemonize(int *daemonized)\n {\n #ifdef NO_POSIX_GOODIES\n \terrno = ENOSYS;\n@@ -1029,6 +1029,8 @@ int daemonize(void)\n \t\tcase -1:\n \t\t\tdie_errno(\"fork failed\");\n \t\tdefault:\n+\t\t\tif (daemonized)\n+\t\t\t\t*daemonized = 1;\n \t\t\texit(0);\n \t}\n \tif (setsid() == -1)\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283879","messageId":"1461108489-29376-7-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 06/15] index-helper: add --detach","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:00Z","receivedAt":"2016-04-19T23:28:00Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nWe detach after creating and opening the socket, because otherwise\nwe might return control to the shell before index-helper is ready to\naccept commands. This might lead to flaky tests.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 3 +++\n index-helper.c | 9 +++++++--\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex a5c058f..f6aa525 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -31,6 +31,9 @@ OPTIONS\n \tfor reading an index, but because it will happen in the\n \tbackground, it's not noticable. `--strict` is enabled by default.\n \n+--detach::\n+\tDetach from the shell.\n+\n NOTES\n -----\n \ndiff --git a/index-helper.c b/index-helper.c\nindex e8d6b64..80b1dbe 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -16,7 +16,7 @@ struct shm {\n \n static struct shm shm_index;\n static struct shm shm_base_index;\n-static int to_verify = 1;\n+static int daemonized, to_verify = 1;\n \n static void release_index_shm(struct shm *is)\n {\n@@ -35,6 +35,8 @@ static void cleanup_shm(void)\n \n static void cleanup(void)\n {\n+\tif (daemonized)\n+\t\treturn;\n \tunlink(git_path(\"index-helper.sock\"));\n \tcleanup_shm();\n }\n@@ -286,7 +288,7 @@ static const char * const usage_text[] = {\n int main(int argc, char **argv)\n {\n \tconst char *prefix;\n-\tint idle_in_seconds = 600;\n+\tint idle_in_seconds = 600, detach = 0;\n \tint fd;\n \tstruct strbuf socket_path = STRBUF_INIT;\n \tstruct option options[] = {\n@@ -294,6 +296,7 @@ int main(int argc, char **argv)\n \t\t\t N_(\"exit if not used after some seconds\")),\n \t\tOPT_BOOL(0, \"strict\", &to_verify,\n \t\t\t \"verify shared memory after creating\"),\n+\t\tOPT_BOOL(0, \"detach\", &detach, \"detach the process\"),\n \t\tOPT_END()\n \t};\n \n@@ -317,6 +320,8 @@ int main(int argc, char **argv)\n \tif (fd < 0)\n \t\tdie_errno(_(\"could not set up index-helper socket\"));\n \n+\tif (detach && daemonize(&daemonized))\n+\t\tdie_errno(_(\"unable to detach\"));\n \tloop(fd, idle_in_seconds);\n \n \tclose(fd);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283882","messageId":"1461108489-29376-8-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 07/15] read-cache: add watchman 'WAMA' extension","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:01Z","receivedAt":"2016-04-19T23:28:01Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nThe extension contains a bitmap, one bit for each entry in the\nindex. If the n-th bit is zero, the n-th entry is considered\nunchanged, we can ce_mark_uptodate() it without refreshing. If the bit\nis non-zero and we found out the corresponding file is clean after\nrefresh, we can clear the bit.\n\nIn addition, there's a list of directories in the untracked-cache\nto invalidate (because they have new or modified entries).\n\nThe 'skipping refresh' bit is not in this patch yet as we would need\nwatchman. More details in later patches.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Documentation/technical/index-format.txt | 22 ++++++\n cache.h | 4 ++\n dir.h | 3 +\n read-cache.c | 117 ++++++++++++++++++++++++++++++-\n 4 files changed, 144 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt\nindex ade0b0c..86ed3a6 100644\n--- a/Documentation/technical/index-format.txt\n+++ b/Documentation/technical/index-format.txt\n@@ -295,3 +295,25 @@ The remaining data of each directory block is grouped by type:\n in the previous ewah bitmap.\n \n - One NUL.\n+\n+== Watchman cache\n+\n+ The watchman cache tracks files for which watchman has told us about\n+ changes. The signature for this extension is { 'W', 'A', 'M', 'A' }.\n+\n+ The extension starts with\n+\n+ - A NUL-terminated string: the watchman vector clock at the last\n+ time we heard from watchman.\n+\n+ - 32-bit bitmap size: the size of the CE_WATCHMAN_DIRTY bitmap\n+\n+ - 32-bit untracked cache entry count: the number of dirty untracked\n+ cache entries\n+\n+ - An ewah bitmap, the n-th bit indicates whether the n-th index entry\n+ is CE_WATCHMAN_DIRTY.\n+\n+ - a list of N NUL-terminated strings. Each is a directory that should\n+ be marked dirty in the untracked cache because watchman has told us\n+ about an update to a file in it.\ndiff --git a/cache.h b/cache.h\nindex 0aeb994..f4f7eef 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -182,6 +182,8 @@ struct cache_entry {\n #define CE_VALID (0x8000)\n #define CE_STAGESHIFT 12\n \n+#define CE_WATCHMAN_DIRTY (0x0001)\n+\n /*\n * Range 0xFFFF0FFF in ce_flags is divided into\n * two parts: in-memory flags and on-disk ones.\n@@ -320,6 +322,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 WATCHMAN_CHANGED\t(1 << 8)\n \n struct split_index;\n struct untracked_cache;\n@@ -344,6 +347,7 @@ struct index_state {\n \tstruct untracked_cache *untracked;\n \tvoid *mmap;\n \tsize_t mmap_size;\n+\tchar *last_update;\n };\n \n extern struct index_state the_index;\ndiff --git a/dir.h b/dir.h\nindex 3ec3fb0..3d540de 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -142,6 +142,9 @@ struct untracked_cache {\n \tint gitignore_invalidated;\n \tint dir_invalidated;\n \tint dir_opened;\n+\t/* watchman invalidation data */\n+\tunsigned int use_watchman : 1;\n+\tstruct string_list invalid_untracked;\n };\n \n struct dir_struct {\ndiff --git a/read-cache.c b/read-cache.c\nindex 5df5b21..050668f 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -19,6 +19,7 @@\n #include \"split-index.h\"\n #include \"utf8.h\"\n #include \"unix-socket.h\"\n+#include \"ewah/ewok.h\"\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\n \t\t\t\t\t unsigned int options);\n@@ -41,11 +42,13 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\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_WATCHMAN 0x57414D41\t /* \"WAMA\" */\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 | \\\n+\t\t WATCHMAN_CHANGED)\n \n struct index_state the_index;\n static const char *alternate_index_output;\n@@ -1220,8 +1223,13 @@ 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\tif (ce->ce_flags & CE_WATCHMAN_DIRTY) {\n+\t\t\t\tce->ce_flags &= ~CE_WATCHMAN_DIRTY;\n+\t\t\t\tistate->cache_changed |= WATCHMAN_CHANGED;\n+\t\t\t}\n \t\t\tcontinue;\n+\t\t}\n \t\tif (!new) {\n \t\t\tconst char *fmt;\n \n@@ -1365,6 +1373,94 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n \treturn 0;\n }\n \n+static void mark_no_watchman(size_t pos, void *data)\n+{\n+\tstruct index_state *istate = data;\n+\tassert(pos < istate->cache_nr);\n+\tistate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;\n+}\n+\n+static int read_watchman_ext(struct index_state *istate, const void *data,\n+\t\t\t unsigned long sz)\n+{\n+\tstruct ewah_bitmap *bitmap;\n+\tint ret, len;\n+\tuint32_t bitmap_size;\n+\tuint32_t untracked_nr;\n+\n+\tif (memchr(data, 0, sz) == NULL)\n+\t\treturn error(\"invalid extension\");\n+\n+\tlen = strlen(data) + 1;\n+\tmemcpy(&bitmap_size, (const char *)data + len, 4);\n+\tmemcpy(&untracked_nr, (const char *)data + len + 4, 4);\n+\tuntracked_nr = ntohl(untracked_nr);\n+\tbitmap_size = ntohl(bitmap_size);\n+\n+\tbitmap = ewah_new();\n+\tret = ewah_read_mmap(bitmap, (const char *)data + len + 8, bitmap_size);\n+\tif (ret != bitmap_size) {\n+\t\tewah_free(bitmap);\n+\t\treturn error(\"failed to parse ewah bitmap reading watchman index extension\");\n+\t}\n+\tistate->last_update = xstrdup(data);\n+\tewah_each_bit(bitmap, mark_no_watchman, istate);\n+\tewah_free(bitmap);\n+\n+\t/*\n+\t * TODO: update the untracked cache from the untracked data in this\n+\t * extension.\n+\t */\n+\treturn 0;\n+}\n+\n+static int untracked_entry_append(struct string_list_item *item, void *sbvoid)\n+{\n+\tstruct strbuf *sb = sbvoid;\n+\n+\tstrbuf_addstr(sb, item->string);\n+\tstrbuf_addch(sb, 0);\n+\treturn 0;\n+}\n+\n+void write_watchman_ext(struct strbuf *sb, struct index_state* istate)\n+{\n+\tstruct ewah_bitmap *bitmap;\n+\tint i;\n+\tint ewah_start;\n+\tint ewah_size = 0;\n+\tint fixup = 0;\n+\n+\tstrbuf_add(sb, istate->last_update, strlen(istate->last_update) + 1);\n+\tfixup = sb->len;\n+\tstrbuf_add(sb, &ewah_size, 4); /* we'll fix this up later */\n+\tif (istate->untracked) {\n+\t\tuint32_t nr = istate->untracked->invalid_untracked.nr;\n+\t\tnr = htonl(nr);\n+\t\tstrbuf_add(sb, &nr, 4);\n+\t} else {\n+\t\t/* zero */\n+\t\tstrbuf_add(sb, &ewah_size, 4);\n+\t}\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_WATCHMAN_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 = sb->len - ewah_start;\n+\tewah_size = htonl(ewah_size);\n+\tmemcpy(sb->buf + fixup, &ewah_size, 4);\n+\n+\tif (istate->untracked)\n+\t\tfor_each_string_list(&istate->untracked->invalid_untracked,\n+\t\t\t\t untracked_entry_append, sb);\n+}\n+\n static int read_index_extension(struct index_state *istate,\n \t\t\t\tconst char *ext, void *data, unsigned long sz)\n {\n@@ -1382,6 +1478,11 @@ 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+\n+\tcase CACHE_EXT_WATCHMAN:\n+\t\tread_watchman_ext(istate, data, sz);\n+\t\tbreak;\n+\n \tdefault:\n \t\tif (*ext < 'A' || 'Z' < *ext)\n \t\t\treturn error(\"index uses %.4s extension, which we do not understand\",\n@@ -1812,6 +1913,8 @@ int discard_index(struct index_state *istate)\n \tistate->untracked = NULL;\n \tistate->from_shm = 0;\n \tistate->to_shm = 0;\n+\tfree(istate->last_update);\n+\tistate->last_update = NULL;\n \treturn 0;\n }\n \n@@ -2209,6 +2312,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->last_update) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\twrite_watchman_ext(&sb, istate);\n+\t\terr = write_index_ext_header(&c, newfd, CACHE_EXT_WATCHMAN, 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;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283883","messageId":"1461108489-29376-9-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 08/15] Add watchman support to reduce index refresh cost","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:02Z","receivedAt":"2016-04-19T23:28:02Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nThe previous patch has the logic to clear bits in 'WAMA' bitmap. This\npatch has logic to set bits as told by watchman. The missing bit,\n_using_ these bits, are not here yet.\n\nA lot of this code is written by David Turner originally, mostly from\n[1]. I'm just copying and polishing it a bit.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/248006\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Makefile | 12 +++++\n cache.h | 1 +\n config.c | 5 ++\n configure.ac | 8 ++++\n environment.c | 3 ++\n watchman-support.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++\n watchman-support.h | 7 +++\n 7 files changed, 171 insertions(+)\n create mode 100644 watchman-support.c\n create mode 100644 watchman-support.h\n\ndiff --git a/Makefile b/Makefile\nindex c8be0e7..65ab0f4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -451,6 +451,7 @@ MSGFMT = msgfmt\n CURL_CONFIG = curl-config\n PTHREAD_LIBS = -lpthread\n PTHREAD_CFLAGS =\n+WATCHMAN_LIBS =\n GCOV = gcov\n \n export TCL_PATH TCLTK_PATH\n@@ -1416,6 +1417,13 @@ else\n \tLIB_OBJS += thread-utils.o\n endif\n \n+ifdef USE_WATCHMAN\n+\tLIB_H += watchman-support.h\n+\tLIB_OBJS += watchman-support.o\n+\tWATCHMAN_LIBS = -lwatchman\n+\tBASIC_CFLAGS += -DUSE_WATCHMAN\n+endif\n+\n ifdef HAVE_PATHS_H\n \tBASIC_CFLAGS += -DHAVE_PATHS_H\n endif\n@@ -2025,6 +2033,9 @@ git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \\\n \t$(VCSSVN_LIB)\n \n+git-index-helper$X: index-helper.o GIT-LDFLAGS $(GITLIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) $(WATCHMAN_LIBS)\n+\n $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)\n \t$(QUIET_LNCP)$(RM) $@ && \\\n \tln $< $@ 2>/dev/null || \\\n@@ -2164,6 +2175,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo NO_MMAP=\\''$(subst ','\\'',$(subst ','\\'',$(NO_MMAP)))'\\' >>$@+\n+\t@echo USE_WATCHMAN=\\''$(subst ','\\'',$(subst ','\\'',$(USE_WATCHMAN)))'\\' >>$@+\n ifdef TEST_OUTPUT_DIRECTORY\n \t@echo TEST_OUTPUT_DIRECTORY=\\''$(subst ','\\'',$(subst ','\\'',$(TEST_OUTPUT_DIRECTORY)))'\\' >>$@+\n endif\ndiff --git a/cache.h b/cache.h\nindex f4f7eef..37f211b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -687,6 +687,7 @@ extern char *git_replace_ref_base;\n \n extern int fsync_object_files;\n extern int core_preload_index;\n+extern int core_watchman_sync_timeout;\n extern int core_apply_sparse_checkout;\n extern int precomposed_unicode;\n extern int protect_hfs;\ndiff --git a/config.c b/config.c\nindex 9ba40bc..e6dc141 100644\n--- a/config.c\n+++ b/config.c\n@@ -882,6 +882,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.watchmansynctimeout\")) {\n+\t\tcore_watchman_sync_timeout = git_config_int(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"core.createobject\")) {\n \t\tif (!strcmp(value, \"rename\"))\n \t\t\tobject_creation_mode = OBJECT_CREATION_USES_RENAMES;\ndiff --git a/configure.ac b/configure.ac\nindex 0cd9f46..334d63b 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -1099,6 +1099,14 @@ AC_COMPILE_IFELSE([BSD_SYSCTL_SRC],\n \tHAVE_BSD_SYSCTL=])\n GIT_CONF_SUBST([HAVE_BSD_SYSCTL])\n \n+#\n+# Check for watchman client library\n+\n+AC_CHECK_LIB([watchman], [watchman_connect],\n+\t[USE_WATCHMAN=YesPlease],\n+\t[USE_WATCHMAN=])\n+GIT_CONF_SUBST([USE_WATCHMAN])\n+\n ## Other checks.\n # Define USE_PIC if you need the main git objects to be built with -fPIC\n # in order to build and link perl/Git.so. x86-64 seems to need this.\ndiff --git a/environment.c b/environment.c\nindex 6dec9d0..35e03c7 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -94,6 +94,9 @@ int core_preload_index = 1;\n */\n int ignore_untracked_cache_config;\n \n+int core_watchman_sync_timeout = 300;\n+\n+\n /* This is set by setup_git_dir_gently() and/or git_default_config() */\n char *git_work_tree_cfg;\n static char *work_tree;\ndiff --git a/watchman-support.c b/watchman-support.c\nnew file mode 100644\nindex 0000000..b168e88\n--- /dev/null\n+++ b/watchman-support.c\n@@ -0,0 +1,135 @@\n+#include \"cache.h\"\n+#include \"watchman-support.h\"\n+#include \"strbuf.h\"\n+#include \"dir.h\"\n+#include \n+\n+static struct watchman_query *make_query(const char *last_update)\n+{\n+\tstruct watchman_query *query = watchman_query();\n+\twatchman_query_set_fields(query, WATCHMAN_FIELD_NAME |\n+\t\t\t\t\t WATCHMAN_FIELD_EXISTS |\n+\t\t\t\t\t WATCHMAN_FIELD_NEWER);\n+\twatchman_query_set_empty_on_fresh(query, 1);\n+\tquery->sync_timeout = core_watchman_sync_timeout;\n+\tif (*last_update)\n+\t\twatchman_query_set_since_oclock(query, last_update);\n+\treturn query;\n+}\n+\n+static struct watchman_query_result* query_watchman(\n+\tstruct index_state *istate, struct watchman_connection *connection,\n+\tconst char *fs_path, const char *last_update)\n+{\n+\tstruct watchman_error wm_error;\n+\tstruct watchman_query *query;\n+\tstruct watchman_expression *expr;\n+\tstruct watchman_query_result *result;\n+\n+\tquery = make_query(last_update);\n+\texpr = watchman_true_expression();\n+\tresult = watchman_do_query(connection, fs_path, query, expr, &wm_error);\n+\twatchman_free_query(query);\n+\twatchman_free_expression(expr);\n+\n+\tif (!result)\n+\t\twarning(\"Watchman query error: %s (at %s)\",\n+\t\t\twm_error.message,\n+\t\t\t*last_update ? last_update : \"the beginning\");\n+\n+\treturn result;\n+}\n+\n+static void update_index(struct index_state *istate,\n+\t\t\t struct watchman_query_result *result)\n+{\n+\tint i;\n+\n+\tif (result->is_fresh_instance) {\n+\t\t/* let refresh clear them later */\n+\t\tfor (i = 0; i < istate->cache_nr; i++)\n+\t\t\tistate->cache[i]->ce_flags |= CE_WATCHMAN_DIRTY;\n+\t\tgoto done;\n+\t}\n+\n+\tfor (i = 0; i < result->nr; i++) {\n+\t\tstruct watchman_stat *wm = result->stats + i;\n+\t\tint pos;\n+\n+\t\tif (S_ISDIR(wm->mode) ||\n+\t\t !strncmp(wm->name, \".git/\", 5) ||\n+\t\t strstr(wm->name, \"/.git/\"))\n+\t\t\tcontinue;\n+\n+\t\tpos = index_name_pos(istate, wm->name, strlen(wm->name));\n+\t\tif (pos < 0) {\n+\t\t\tif (istate->untracked) {\n+\t\t\t\tchar *name = xstrdup(wm->name);\n+\t\t\t\tchar *dname = dirname(name);\n+\n+\t\t\t\t/*\n+\t\t\t\t * dirname() returns '.' for the root,\n+\t\t\t\t * but we call it ''.\n+\t\t\t\t */\n+\t\t\t\tif (dname[0] == '.' && dname[1] == 0)\n+\t\t\t\t\tstring_list_append(&istate->untracked->invalid_untracked, \"\");\n+\t\t\t\telse\n+\t\t\t\t\tstring_list_append(&istate->untracked->invalid_untracked,\n+\t\t\t\t\t\t\t dname);\n+\t\t\t\tfree(name);\n+\t\t\t}\n+\t\t\tcontinue;\n+\t\t}\n+\t\t/* FIXME: ignore staged entries and gitlinks too? */\n+\n+\t\tistate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;\n+\t}\n+\n+done:\n+\tfree(istate->last_update);\n+\tistate->last_update = xstrdup(result->clock);\n+\tistate->cache_changed |= WATCHMAN_CHANGED;\n+\tif (istate->untracked)\n+\t\tstring_list_remove_duplicates(&istate->untracked->invalid_untracked, 0);\n+}\n+\n+int check_watchman(struct index_state *istate)\n+{\n+\tstruct watchman_error wm_error;\n+\tstruct watchman_connection *connection;\n+\tstruct watchman_query_result *result;\n+\tconst char *fs_path;\n+\tstruct timeval timeout;\n+\t/*\n+\t * Convert core_watchman_sync_timeout, in milliseconds, to\n+\t * struct timeval, in seconds and microseconds.\n+\t */\n+\n+\tfs_path = get_git_work_tree();\n+\tif (!fs_path)\n+\t\treturn -1;\n+\n+\ttimeout.tv_sec = core_watchman_sync_timeout / 1000;\n+\ttimeout.tv_usec = (core_watchman_sync_timeout % 1000) * 1000;\n+\tconnection = watchman_connect(timeout, &wm_error);\n+\n+\tif (!connection) {\n+\t\twarning(\"Watchman watch error: %s\", wm_error.message);\n+\t\treturn -1;\n+\t}\n+\n+\tif (watchman_watch(connection, fs_path, &wm_error)) {\n+\t\twarning(\"Watchman watch error: %s\", wm_error.message);\n+\t\twatchman_connection_close(connection);\n+\t\treturn -1;\n+\t}\n+\n+\n+\tresult = query_watchman(istate, connection, fs_path, istate->last_update);\n+\twatchman_connection_close(connection);\n+\tif (!result)\n+\t\treturn -1;\n+\tupdate_index(istate, result);\n+\twatchman_free_query_result(result);\n+\treturn 0;\n+}\ndiff --git a/watchman-support.h b/watchman-support.h\nnew file mode 100644\nindex 0000000..ee1ef2c\n--- /dev/null\n+++ b/watchman-support.h\n@@ -0,0 +1,7 @@\n+#ifndef WATCHMAN_SUPPORT_H\n+#define WATCHMAN_SUPPORT_H\n+\n+struct index_state;\n+int check_watchman(struct index_state *index);\n+\n+#endif /* WATCHMAN_SUPPORT_H */\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283880","messageId":"1461108489-29376-10-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 09/15] index-helper: use watchman to avoid refreshing index with lstat()","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:03Z","receivedAt":"2016-04-19T23:28:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nWatchman is hidden behind index-helper. Before git tries to read the\nindex from shm, it notifies index-helper through the socket and waits\nfor index-helper to prepare a file for sharing memory (with\nMAP_SHARED). index-helper then contacts watchman, updates 'WAMA'\nextension and put it in a separate file and wakes git up with a reply\nto git's socket.\n\nGit uses this extension to not lstat unchanged entries. Git only\ntrusts the 'WAMA' extension when it's received from the separate file,\nnot from disk. Unmarked entries are \"clean\". Marked entries are dirty\nfrom watchman point of view. If it finds out some entries are\n'watchman-dirty', but are really unchanged (e.g. the file was changed,\nthen reverted back), then Git will clear the marking in 'WAMA' before\nwriting it down.\n\nHiding watchman behind index-helper means you need both daemons. You\ncan't run watchman alone. Not so good. But on the other hand, 'git'\nbinary is not linked to watchman/json libraries, which is good for\npackaging. Core git package will run fine without watchman-related\npackages. If they need watchman, they can install git-index-helper and\ndependencies.\n\nThis also lets us trust anything in the untracked cache that we haven't\nmarked invalid, saving those stat() calls.\n\nAnother reason for tying watchman to index-helper is, when used with\nuntracked cache, we need to keep track of $GIT_WORK_TREE file\nlisting. That kind of list can be kept in index-helper.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 6 +\n cache.h | 2 +\n dir.c | 23 +++-\n dir.h | 3 +\n index-helper.c | 86 +++++++++++++--\n read-cache.c | 220 ++++++++++++++++++++++++++++++++++---\n 6 files changed, 315 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex f6aa525..b1fa49e 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -52,6 +52,12 @@ command. The following commands are used to control the daemon:\n \tLet the daemon know the index is to be read. It keeps the\n \tdaemon alive longer, unless `--exit-after=0` is used.\n \n+\"poke \":\n+\tLike \"poke\", but replies with \"OK\". If the index has the\n+\twatchman extension, index-helper queries watchman, then\n+\tprepares a shared memory object with the watchman index\n+\textension before replying.\n+\n All commands and replies are terminated by a 0 byte.\n \n GIT\ndiff --git a/cache.h b/cache.h\nindex 37f211b..4cc89bb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -558,6 +558,7 @@ extern int daemonize(int *);\n \n /* Initialize and use the cache information */\n struct lock_file;\n+extern int verify_index(const struct index_state *);\n extern int read_index(struct index_state *);\n extern int read_index_preload(struct index_state *, const struct pathspec *pathspec);\n extern int do_read_index(struct index_state *istate, const char *path,\n@@ -565,6 +566,7 @@ extern int do_read_index(struct index_state *istate, const char *path,\n extern int read_index_from(struct index_state *, const char *path);\n extern int is_index_unborn(struct index_state *);\n extern int read_index_unmerged(struct index_state *);\n+extern void write_watchman_ext(struct strbuf *sb, struct index_state* istate);\n #define COMMIT_LOCK\t\t(1 << 0)\n #define CLOSE_LOCK\t\t(1 << 1)\n extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);\ndiff --git a/dir.c b/dir.c\nindex 69e0be6..5058b29 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -597,9 +597,9 @@ 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-\t\t\t\t\t\t struct untracked_cache_dir *dir,\n-\t\t\t\t\t\t const char *name, int len)\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 {\n \tint first, last;\n \tstruct untracked_cache_dir *d;\n@@ -1726,6 +1726,17 @@ static int valid_cached_dir(struct dir_struct *dir,\n \tif (!untracked)\n \t\treturn 0;\n \n+\tif (dir->untracked->use_watchman) {\n+\t\t/*\n+\t\t * With watchman, 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@@ -1739,6 +1750,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;\n@@ -2625,8 +2637,10 @@ static void free_untracked(struct untracked_cache_dir *ucd)\n \n void free_untracked_cache(struct untracked_cache *uc)\n {\n-\tif (uc)\n+\tif (uc) {\n \t\tfree_untracked(uc->root);\n+\t\tstring_list_clear(&uc->invalid_untracked, 0);\n+\t}\n \tfree(uc);\n }\n \n@@ -2775,6 +2789,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n \t\treturn NULL;\n \n \tuc = xcalloc(1, sizeof(*uc));\n+\tstring_list_init(&uc->invalid_untracked, 1);\n \tstrbuf_init(&uc->ident, ident_len);\n \tstrbuf_add(&uc->ident, ident, ident_len);\n \tload_sha1_stat(&uc->ss_info_exclude, &ouc->info_exclude_stat,\ndiff --git a/dir.h b/dir.h\nindex 3d540de..8fd3f9e 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -315,4 +315,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_cache(struct index_state *istate);\n void remove_untracked_cache(struct index_state *istate);\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\ndiff --git a/index-helper.c b/index-helper.c\nindex 80b1dbe..9d85a7c 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -7,15 +7,18 @@\n #include \"lockfile.h\"\n #include \"cache.h\"\n #include \"unix-socket.h\"\n+#include \"watchman-support.h\"\n \n struct shm {\n \tunsigned char sha1[20];\n \tvoid *shm;\n \tsize_t size;\n+\tpid_t pid;\n };\n \n static struct shm shm_index;\n static struct shm shm_base_index;\n+static struct shm shm_watchman;\n static int daemonized, to_verify = 1;\n \n static void release_index_shm(struct shm *is)\n@@ -27,10 +30,21 @@ static void release_index_shm(struct shm *is)\n \tis->shm = NULL;\n }\n \n+static void release_watchman_shm(struct shm *is)\n+{\n+\tif (!is->shm)\n+\t\treturn;\n+\tmunmap(is->shm, is->size);\n+\tunlink(git_path(\"shm-watchman-%s-%\" PRIuMAX,\n+\t\t\tsha1_to_hex(is->sha1), (uintmax_t)is->pid));\n+\tis->shm = NULL;\n+}\n+\n static void cleanup_shm(void)\n {\n \trelease_index_shm(&shm_index);\n \trelease_index_shm(&shm_base_index);\n+\trelease_watchman_shm(&shm_watchman);\n }\n \n static void cleanup(void)\n@@ -161,9 +175,10 @@ static void share_the_index(void)\n \tif (the_index.split_index && the_index.split_index->base)\n \t\tshare_index(the_index.split_index->base, &shm_base_index);\n \tshare_index(&the_index, &shm_index);\n-\tif (to_verify && !verify_shm())\n+\tif (to_verify && !verify_shm()) {\n \t\tcleanup_shm();\n-\tdiscard_index(&the_index);\n+\t\tdiscard_index(&the_index);\n+\t}\n }\n \n static void set_socket_blocking_flag(int fd, int make_nonblocking)\n@@ -196,6 +211,54 @@ static void refresh(void)\n \n #ifndef NO_MMAP\n \n+#ifdef USE_WATCHMAN\n+static void share_watchman(struct index_state *istate,\n+\t\t\t struct shm *is, pid_t pid)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tvoid *shm;\n+\n+\twrite_watchman_ext(&sb, istate);\n+\tif (!shared_mmap_create(O_CREAT | O_EXCL | O_RDWR, 0700, sb.len + 20,\n+\t\t\t\t&shm, PROT_READ | PROT_WRITE, MAP_SHARED,\n+\t\t\t\tgit_path(\"shm-watchman-%s-%\" PRIuMAX,\n+\t\t\t\t\t sha1_to_hex(istate->sha1),\n+\t\t\t\t\t (uintmax_t)pid))) {\n+\t\tis->size = sb.len + 20;\n+\t\tis->shm = shm;\n+\t\tis->pid = pid;\n+\t\thashcpy(is->sha1, istate->sha1);\n+\n+\t\tmemcpy(shm, sb.buf, sb.len);\n+\t\thashcpy((unsigned char *)shm + is->size - 20, is->sha1);\n+\t}\n+\tstrbuf_release(&sb);\n+}\n+\n+\n+static void prepare_with_watchman(pid_t pid)\n+{\n+\t/*\n+\t * TODO: with the help of watchman, maybe we could detect if\n+\t * $GIT_DIR/index is updated.\n+\t */\n+\tif (check_watchman(&the_index))\n+\t\treturn;\n+\n+\tshare_watchman(&the_index, &shm_watchman, pid);\n+}\n+\n+static void prepare_index(pid_t pid)\n+{\n+\tif (shm_index.shm == NULL)\n+\t\trefresh();\n+\trelease_watchman_shm(&shm_watchman);\n+\tif (the_index.last_update)\n+\t\tprepare_with_watchman(pid);\n+}\n+\n+#endif\n+\n static void loop(int fd, int idle_in_seconds)\n {\n \tstruct timeval timeout;\n@@ -250,11 +313,20 @@ static void loop(int fd, int idle_in_seconds)\n \t\tif (strbuf_getwholeline_fd(&command, client_fd, '\\0') == 0) {\n \t\t\tif (!strcmp(command.buf, \"refresh\")) {\n \t\t\t\trefresh();\n-\t\t\t} else if (!strcmp(command.buf, \"poke\")) {\n-\t\t\t\t/*\n-\t\t\t\t * Just a poke to keep us\n-\t\t\t\t * alive, nothing to do.\n-\t\t\t\t */\n+\t\t\t} else if (starts_with(command.buf, \"poke\")) {\n+\t\t\t\tif (command.buf[4] == ' ') {\n+#ifdef USE_WATCHMAN\n+\t\t\t\t\tpid_t client_pid = strtoull(command.buf + 5, NULL, 10);\n+\t\t\t\t\tprepare_index(client_pid);\n+#endif\n+\t\t\t\t\tif (write_in_full(client_fd, \"OK\", 3) != 3)\n+\t\t\t\t\t\twarning(_(\"client write failed\"));\n+\t\t\t\t} else {\n+\t\t\t\t\t/*\n+\t\t\t\t\t * Just a poke to keep us\n+\t\t\t\t\t * alive, nothing to do.\n+\t\t\t\t\t */\n+\t\t\t\t}\n \t\t\t} else {\n \t\t\t\twarning(\"BUG: Bogus command %s\", command.buf);\n \t\t\t}\ndiff --git a/read-cache.c b/read-cache.c\nindex 050668f..fb0168e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1233,7 +1233,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \t\tif (!new) {\n \t\t\tconst char *fmt;\n \n-\t\t\tif (really && cache_errno == EINVAL) {\n+\t\t\tif (really || cache_errno == EINVAL) {\n \t\t\t\t/* If we are doing --really-refresh that\n \t\t\t\t * means the index is not valid anymore.\n \t\t\t\t */\n@@ -1373,11 +1373,76 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)\n \treturn 0;\n }\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+\tint component_len;\n+\tconst char *end;\n+\tstruct untracked_cache_dir *dir;\n+\n+\tif (!*name)\n+\t\treturn ucd;\n+\n+\tend = strchr(name, '/');\n+\tif (end)\n+\t\tcomponent_len = end - name;\n+\telse\n+\t\tcomponent_len = strlen(name);\n+\n+\tdir = lookup_untracked(uc, ucd, name, component_len);\n+\tif (dir)\n+\t\treturn find_untracked_cache_dir(uc, dir, name + component_len + 1);\n+\n+\treturn NULL;\n+}\n+\n static void mark_no_watchman(size_t pos, void *data)\n {\n \tstruct index_state *istate = data;\n+\tstruct cache_entry *ce = istate->cache[pos];\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tchar *c;\n+\tstruct untracked_cache_dir *dir;\n+\n \tassert(pos < istate->cache_nr);\n-\tistate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;\n+\tce->ce_flags |= CE_WATCHMAN_DIRTY;\n+\n+\tif (!istate->untracked || !istate->untracked->root)\n+\t\treturn;\n+\n+\tstrbuf_add(&sb, ce->name, ce_namelen(ce));\n+\n+\tfor (c = sb.buf + sb.len - 1; c > sb.buf; c--) {\n+\t\tif (*c == '/') {\n+\t\t\tstrbuf_setlen(&sb, c - sb.buf);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (c == sb.buf) {\n+\t\tstrbuf_setlen(&sb, 0);\n+\t}\n+\n+\tdir = find_untracked_cache_dir(istate->untracked,\n+\t\t\t\t istate->untracked->root, sb.buf);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+\n+\tstrbuf_release(&sb);\n+}\n+\n+static int mark_untracked_invalid(struct string_list_item *item, void *uc)\n+{\n+\tstruct untracked_cache *untracked = uc;\n+\tstruct untracked_cache_dir *dir;\n+\n+\tdir = find_untracked_cache_dir(untracked, untracked->root,\n+\t\t\t\t item->string);\n+\tif (dir)\n+\t\tdir->valid = 0;\n+\n+\treturn 0;\n }\n \n static int read_watchman_ext(struct index_state *istate, const void *data,\n@@ -1407,10 +1472,24 @@ static int read_watchman_ext(struct index_state *istate, const void *data,\n \tewah_each_bit(bitmap, mark_no_watchman, istate);\n \tewah_free(bitmap);\n \n-\t/*\n-\t * TODO: update the untracked cache from the untracked data in this\n-\t * extension.\n-\t */\n+\tif (istate->untracked && istate->untracked->root) {\n+\t\tint i;\n+\t\tconst char *untracked;\n+\n+\t\tuntracked = (const char *)data + len + 8 + bitmap_size;\n+\t\tfor (i = 0; i < untracked_nr; ++i) {\n+\t\t\tint len = strlen(untracked);\n+\t\t\tstring_list_append(&istate->untracked->invalid_untracked,\n+\t\t\t\t\t untracked);\n+\t\t\tuntracked += len + 1;\n+\t\t}\n+\n+\t\tfor_each_string_list(&istate->untracked->invalid_untracked,\n+\t\t\t mark_untracked_invalid, istate->untracked);\n+\n+\t\tif (untracked_nr)\n+\t\t\tistate->cache_changed |= WATCHMAN_CHANGED;\n+\t}\n \treturn 0;\n }\n \n@@ -1643,26 +1722,67 @@ static void post_read_index_from(struct index_state *istate)\n \ttweak_untracked_cache(istate);\n }\n \n+static int poke_and_wait_for_reply(int fd)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf reply = STRBUF_INIT;\n+\tint ret = -1;\n+\tfd_set fds;\n+\tstruct timeval timeout;\n+\n+\ttimeout.tv_usec = 0;\n+\ttimeout.tv_sec = 1;\n+\n+\tif (fd < 0)\n+\t\treturn -1;\n+\n+\tstrbuf_addf(&buf, \"poke %d\", getpid());\n+\tif (write_in_full(fd, buf.buf, buf.len + 1) != buf.len + 1)\n+\t\tgoto done_poke;\n+\n+\t/* Now wait for a reply */\n+\tFD_ZERO(&fds);\n+\tFD_SET(fd, &fds);\n+\tif (select(fd + 1, &fds, NULL, NULL, &timeout) == 0)\n+\t\t/* No reply, giving up */\n+\t\tgoto done_poke;\n+\n+\tif (strbuf_getwholeline_fd(&reply, fd, 0))\n+\t\tgoto done_poke;\n+\n+\tif (!starts_with(reply.buf, \"OK\"))\n+\t\tgoto done_poke;\n+\n+\tret = 0;\n+done_poke:\n+\tclose(fd);\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&reply);\n+\n+\treturn ret;\n+}\n+\n static int poke_daemon(struct index_state *istate,\n \t\t const struct stat *st, int refresh_cache)\n {\n \tint fd;\n-\tint ret = 0;\n-\tconst char *socket_path;\n+\tint ret = -1;\n \n \t/* if this is from index-helper, do not poke itself (recursively) */\n \tif (istate->to_shm)\n \t\treturn 0;\n \n-\tsocket_path = git_path(\"index-helper.sock\");\n-\tif (!socket_path)\n+\tfd = unix_stream_connect(git_path(\"index-helper.sock\"));\n+\tif (fd < 0) {\n+\t\twarning(\"Failed to connect to index-helper socket\");\n+\t\tunlink(git_path(\"index-helper.sock\"));\n \t\treturn -1;\n+\t}\n \n-\tfd = unix_stream_connect(socket_path);\n \tif (refresh_cache) {\n \t\tret = write_in_full(fd, \"refresh\", 8) != 8;\n \t} else {\n-\t\tret = write_in_full(fd, \"poke\", 5) != 5;\n+\t\tret = poke_and_wait_for_reply(fd);\n \t}\n \n \tclose(fd);\n@@ -1732,6 +1852,74 @@ fail:\n \treturn -1;\n }\n \n+static void refresh_by_watchman(struct index_state *istate)\n+{\n+\tvoid *shm = NULL;\n+\tint length;\n+\tint i;\n+\tstruct stat st;\n+\tint fd = -1;\n+\tconst char *path = git_path(\"shm-watchman-%s-%\"PRIuMAX,\n+\t\t\t\t sha1_to_hex(istate->sha1),\n+\t\t\t\t (uintmax_t)getpid());\n+\n+\tfd = open(path, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn;\n+\n+\t/*\n+\t * This watchman data is just for us -- no need to keep it\n+\t * around once we've got it open.\n+\t */\n+\tunlink(path);\n+\n+\tif (fstat(fd, &st) < 0)\n+\t\tgoto done;\n+\n+\tlength = st.st_size;\n+\tshm = mmap(NULL, length, PROT_READ, MAP_SHARED, fd, 0);\n+\n+\tif (shm == MAP_FAILED)\n+\t\tgoto done;\n+\n+\tclose(fd);\n+\tfd = -1;\n+\n+\tif (length <= 20 ||\n+\t hashcmp(istate->sha1, (unsigned char *)shm + length - 20) ||\n+\t /*\n+\t * No need to clear CE_WATCHMAN_DIRTY set by 'WAMA' on\n+\t * disk. Watchman can only set more, not clear any, so\n+\t * this is OR mask.\n+\t */\n+\t read_watchman_ext(istate, shm, length - 20))\n+\t\tgoto done;\n+\n+\t/*\n+\t * Now that we've marked the invalid entries in the\n+\t * untracked-cache itself, we can erase them from the list of\n+\t * entries to be processed and mark the untracked cache for\n+\t * watchman usage.\n+\t */\n+\tif (istate->untracked) {\n+\t\tstring_list_clear(&istate->untracked->invalid_untracked, 0);\n+\t\tistate->untracked->use_watchman = 1;\n+\t}\n+\n+\tfor (i = 0; i < istate->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = istate->cache[i];\n+\t\tif (ce_stage(ce) || (ce->ce_flags & CE_WATCHMAN_DIRTY))\n+\t\t\tcontinue;\n+\t\tce_mark_uptodate(ce);\n+\t}\n+done:\n+\tif (shm)\n+\t\tmunmap(shm, length);\n+\n+\tif (fd >= 0)\n+\t\tclose(fd);\n+}\n+\n /* remember to discard_cache() before reading a different cache! */\n int do_read_index(struct index_state *istate, const char *path, int must_exist)\n {\n@@ -1851,7 +2039,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \tsplit_index = istate->split_index;\n \tif (!split_index || is_null_sha1(split_index->base_sha1)) {\n \t\tpost_read_index_from(istate);\n-\t\treturn ret;\n+\t\tgoto done;\n \t}\n \n \tif (split_index->base)\n@@ -1872,6 +2060,10 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\t sha1_to_hex(split_index->base->sha1));\n \tmerge_base_index(istate);\n \tpost_read_index_from(istate);\n+\n+done:\n+\tif (ret > 0 && istate->from_shm && istate->last_update)\n+\t\trefresh_by_watchman(istate);\n \treturn ret;\n }\n \n@@ -2173,7 +2365,7 @@ out:\n \treturn 0;\n }\n \n-static int verify_index(const struct index_state *istate)\n+int verify_index(const struct index_state *istate)\n {\n \treturn verify_index_from(istate, get_index_file());\n }\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283885","messageId":"1461108489-29376-11-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 10/15] update-index: enable/disable watchman support","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:04Z","receivedAt":"2016-04-19T23:28:04Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 3 +++\n Documentation/git-update-index.txt | 6 ++++++\n builtin/update-index.c | 11 +++++++++++\n 3 files changed, 20 insertions(+)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex b1fa49e..dd039a1 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -15,6 +15,9 @@ DESCRIPTION\n Keep the index file in memory for faster access. This daemon is per\n repository.\n \n+If you want the index-helper to accelerate untracked file checking,\n+run git update-index --watchman before using it.\n+\n OPTIONS\n -------\n \ndiff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt\nindex c6cbed1..6736487 100644\n--- a/Documentation/git-update-index.txt\n+++ b/Documentation/git-update-index.txt\n@@ -19,6 +19,7 @@ SYNOPSIS\n \t [--ignore-submodules]\n \t [--[no-]split-index]\n \t [--[no-|test-|force-]untracked-cache]\n+\t [--[no-]watchman]\n \t [--really-refresh] [--unresolve] [--again | -g]\n \t [--info-only] [--index-info]\n \t [-z] [--stdin] [--index-version ]\n@@ -176,6 +177,11 @@ may not support it yet.\n --no-untracked-cache::\n \tEnable or disable untracked cache feature. Please use\n \t`--test-untracked-cache` before enabling it.\n+\n+--watchman::\n+--no-watchman::\n+\tEnable or disable watchman support. This is, at present,\n+\tonly useful with git index-helper.\n +\n These options take effect whatever the value of the `core.untrackedCache`\n configuration variable (see linkgit:git-config[1]). But a warning is\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 1c94ca5..7c08e1c 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -914,6 +914,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n {\n \tint newfd, entries, has_errors = 0, nul_term_line = 0;\n \tenum uc_mode untracked_cache = UC_UNSPECIFIED;\n+\tint use_watchman = -1;\n \tint read_from_stdin = 0;\n \tint prefix_length = prefix ? strlen(prefix) : 0;\n \tint preferred_index_format = 0;\n@@ -1012,6 +1013,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"test if the filesystem supports untracked cache\"), UC_TEST),\n \t\tOPT_SET_INT(0, \"force-untracked-cache\", &untracked_cache,\n \t\t\t N_(\"enable untracked cache without testing the filesystem\"), UC_FORCE),\n+\t\tOPT_BOOL(0, \"watchman\", &use_watchman,\n+\t\t\tN_(\"use or not use watchman to reduce refresh cost\")),\n \t\tOPT_END()\n \t};\n \n@@ -1149,6 +1152,14 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Bug: bad untracked_cache value: %d\", untracked_cache);\n \t}\n \n+\tif (use_watchman > 0) {\n+\t\tthe_index.last_update = xstrdup(\"\");\n+\t\tthe_index.cache_changed |= WATCHMAN_CHANGED;\n+\t} else if (!use_watchman) {\n+\t\tthe_index.last_update = NULL;\n+\t\tthe_index.cache_changed |= WATCHMAN_CHANGED;\n+\t}\n+\n \tif (active_cache_changed) {\n \t\tif (newfd < 0) {\n \t\t\tif (refresh_args.flags & REFRESH_QUIET)\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283890","messageId":"1461108489-29376-12-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 11/15] unpack-trees: preserve index extensions","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:05Z","receivedAt":"2016-04-19T23:28:05Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Make git checkout (and other unpack_tree operations) preserve the\nuntracked cache and watchman status. This is valuable for two reasons:\n\n1. Often, an unpack_tree operation will not touch large parts of the\nworking tree, and thus most of the untracked cache will continue to be\nvalid.\n\n2. Even if the untracked cache were entirely invalidated by such an\noperation, the user has signaled their intention to have such a cache,\nand we don't want to throw it away.\n\nThe same logic applies to the watchman state.\n\nSigned-off-by: David Turner \n---\n cache.h | 1 +\n read-cache.c | 8 ++++++++\n t/t7063-status-untracked-cache.sh | 22 ++++++++++++++++++++++\n t/test-lib-functions.sh | 4 ++++\n unpack-trees.c | 1 +\n 5 files changed, 36 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex 4cc89bb..49fa128 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -571,6 +571,7 @@ extern void write_watchman_ext(struct strbuf *sb, struct index_state* istate);\n #define CLOSE_LOCK\t\t(1 << 1)\n extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);\n extern int discard_index(struct index_state *);\n+extern void move_index_extensions(struct index_state *dst, struct index_state *src);\n extern int unmerged_index(const struct index_state *);\n extern int verify_path(const char *path);\n extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);\ndiff --git a/read-cache.c b/read-cache.c\nindex fb0168e..65f22f9 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2745,3 +2745,11 @@ void stat_validity_update(struct stat_validity *sv, int fd)\n \t\tfill_stat_data(sv->sd, &st);\n \t}\n }\n+\n+void move_index_extensions(struct index_state *dst, struct index_state *src)\n+{\n+\tdst->untracked = src->untracked;\n+\tsrc->untracked = NULL;\n+\tdst->last_update = src->last_update;\n+\tsrc->last_update = NULL;\n+}\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex a971884..083516d 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -646,4 +646,26 @@ test_expect_success 'test ident field is working' '\n \ttest_cmp ../expect ../err\n '\n \n+test_expect_success 'untracked cache survives a checkout' '\n+\tgit commit --allow-empty -m empty &&\n+\ttest-dump-untracked-cache >../before &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tgit checkout -b other_branch &&\n+\ttest-dump-untracked-cache >../after &&\n+\ttest_cmp ../before ../after &&\n+\ttest_commit test &&\n+\ttest-dump-untracked-cache >../before &&\n+\tgit checkout master &&\n+\ttest-dump-untracked-cache >../after &&\n+\ttest_cmp ../before ../after\n+'\n+\n+test_expect_success 'untracked cache survives a commit' '\n+\ttest-dump-untracked-cache >../before &&\n+\tgit add done/two &&\n+\tgit commit -m commit &&\n+\ttest-dump-untracked-cache >../after &&\n+\ttest_cmp ../before ../after\n+'\n+\n test_done\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 8d99eb3..e974b5b 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -186,6 +186,10 @@ test_commit () {\n \t\ttest_tick\n \tfi &&\n \tgit commit $signoff -m \"$1\" &&\n+\tif [ \"$(git config core.bare)\" = false ]\n+\tthen\n+\t git update-index --force-untracked-cache\n+\tfi\n \tgit tag \"${4:-$1}\"\n }\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 9f55cc2..fc90eb3 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1215,6 +1215,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t\t\t\t\t WRITE_TREE_SILENT |\n \t\t\t\t\t\t WRITE_TREE_REPAIR);\n \t\t}\n+\t\tmove_index_extensions(&o->result, o->dst_index);\n \t\tdiscard_index(o->dst_index);\n \t\t*o->dst_index = o->result;\n \t} else {\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283886","messageId":"1461108489-29376-13-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 12/15] index-helper: kill mode","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:06Z","receivedAt":"2016-04-19T23:28:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Add a new command (and command-line arg) to allow index-helpers to\nexit cleanly.\n\nThis is mainly useful for tests.\n\nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 3 +++\n index-helper.c | 31 ++++++++++++++++++++++++++++++-\n t/t7900-index-helper.sh | 9 +++++++++\n 3 files changed, 42 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex dd039a1..7d80638 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -37,6 +37,9 @@ OPTIONS\n --detach::\n \tDetach from the shell.\n \n+--kill::\n+\tKill any running index-helper and clean up the socket\n+\n NOTES\n -----\n \ndiff --git a/index-helper.c b/index-helper.c\nindex 9d85a7c..6af01c9 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -327,6 +327,8 @@ static void loop(int fd, int idle_in_seconds)\n \t\t\t\t\t * alive, nothing to do.\n \t\t\t\t\t */\n \t\t\t\t}\n+\t\t\t} else if (!strcmp(command.buf, \"die\")) {\n+\t\t\t\tbreak;\n \t\t\t} else {\n \t\t\t\twarning(\"BUG: Bogus command %s\", command.buf);\n \t\t\t}\n@@ -357,10 +359,29 @@ static const char * const usage_text[] = {\n \tNULL\n };\n \n+static void request_kill(void)\n+{\n+\tint fd = unix_stream_connect(git_path(\"index-helper.sock\"));\n+\n+\tif (fd >= 0) {\n+\t\twrite_in_full(fd, \"die\", 4);\n+\t\tclose(fd);\n+\t}\n+\n+\t/*\n+\t * The child will try to do this anyway, but we want to be\n+\t * ready to launch a new daemon immediately after this command\n+\t * returns.\n+\t */\n+\n+\tunlink(git_path(\"index-helper.sock\"));\n+\treturn;\n+}\n+\n int main(int argc, char **argv)\n {\n \tconst char *prefix;\n-\tint idle_in_seconds = 600, detach = 0;\n+\tint idle_in_seconds = 600, detach = 0, kill = 0;\n \tint fd;\n \tstruct strbuf socket_path = STRBUF_INIT;\n \tstruct option options[] = {\n@@ -369,6 +390,7 @@ int main(int argc, char **argv)\n \t\tOPT_BOOL(0, \"strict\", &to_verify,\n \t\t\t \"verify shared memory after creating\"),\n \t\tOPT_BOOL(0, \"detach\", &detach, \"detach the process\"),\n+\t\tOPT_BOOL(0, \"kill\", &kill, \"request that existing index helper processes exit\"),\n \t\tOPT_END()\n \t};\n \n@@ -383,6 +405,13 @@ int main(int argc, char **argv)\n \t\t\t options, usage_text, 0))\n \t\tdie(_(\"too many arguments\"));\n \n+\tif (kill) {\n+\t\tif (detach)\n+\t\t\tdie(_(\"--kill doesn't want any other options\"));\n+\t\trequest_kill();\n+\t\treturn 0;\n+\t}\n+\n \tatexit(cleanup);\n \tsigchain_push_common(cleanup_on_signal);\n \ndiff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh\nindex 114c112..e71b5af 100755\n--- a/t/t7900-index-helper.sh\n+++ b/t/t7900-index-helper.sh\n@@ -20,4 +20,13 @@ test_expect_success 'index-helper smoke test' '\n \ttest_path_is_missing .git/index-helper.sock\n '\n \n+test_expect_success 'index-helper creates usable path file and can be killed' '\n+\ttest_when_finished \"git index-helper --kill\" &&\n+\ttest_path_is_missing .git/index-helper.sock &&\n+\tgit index-helper --detach &&\n+\ttest -S .git/index-helper.sock &&\n+\tgit index-helper --kill &&\n+\ttest_path_is_missing .git/index-helper.sock\n+'\n+\n test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283889","messageId":"1461108489-29376-14-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 13/15] index-helper: don't run if already running","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:07Z","receivedAt":"2016-04-19T23:28:07Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Signed-off-by: David Turner \n---\n index-helper.c | 7 +++++++\n t/t7900-index-helper.sh | 9 +++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/index-helper.c b/index-helper.c\nindex 6af01c9..8fcb76e 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -412,6 +412,13 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\t/* check that no other copy is running */\n+\tfd = unix_stream_connect(git_path(\"index-helper.sock\"));\n+\tif (fd > 0)\n+\t\tdie(_(\"Already running\"));\n+\tif (errno != ECONNREFUSED && errno != ENOENT)\n+\t\tdie_errno(_(\"Unexpected error checking socket\"));\n+\n \tatexit(cleanup);\n \tsigchain_push_common(cleanup_on_signal);\n \ndiff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh\nindex e71b5af..7159971 100755\n--- a/t/t7900-index-helper.sh\n+++ b/t/t7900-index-helper.sh\n@@ -29,4 +29,13 @@ test_expect_success 'index-helper creates usable path file and can be killed' '\n \ttest_path_is_missing .git/index-helper.sock\n '\n \n+test_expect_success 'index-helper will not start if already running' '\n+\ttest_when_finished \"git index-helper --kill\" &&\n+\tgit index-helper --detach &&\n+\ttest -S .git/index-helper.sock &&\n+\ttest_must_fail git index-helper 2>err &&\n+\ttest -S .git/index-helper.sock &&\n+\tgrep \"Already running\" err\n+'\n+\n test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283888","messageId":"1461108489-29376-15-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 14/15] index-helper: autorun mode","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:08Z","receivedAt":"2016-04-19T23:28:08Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Soon, we'll want to automatically start index-helper, so we need\na mode that silently exits if it can't start up (either because\nit's not in a git dir, or because another one is already running).\n\nSigned-off-by: David Turner \n---\n Documentation/git-index-helper.txt | 4 ++++\n index-helper.c | 29 +++++++++++++++++++++++------\n t/t7900-index-helper.sh | 8 ++++++++\n 3 files changed, 35 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt\nindex 7d80638..91d7a36 100644\n--- a/Documentation/git-index-helper.txt\n+++ b/Documentation/git-index-helper.txt\n@@ -40,6 +40,10 @@ OPTIONS\n --kill::\n \tKill any running index-helper and clean up the socket\n \n+--autorun::\n+\tIf index-helper is not already running, start it. Else, do\n+\tnothing.\n+\n NOTES\n -----\n \ndiff --git a/index-helper.c b/index-helper.c\nindex 8fcb76e..3d884da 100644\n--- a/index-helper.c\n+++ b/index-helper.c\n@@ -381,8 +381,9 @@ static void request_kill(void)\n int main(int argc, char **argv)\n {\n \tconst char *prefix;\n-\tint idle_in_seconds = 600, detach = 0, kill = 0;\n+\tint idle_in_seconds = 600, detach = 0, kill = 0, autorun = 0;\n \tint fd;\n+\tint nongit;\n \tstruct strbuf socket_path = STRBUF_INIT;\n \tstruct option options[] = {\n \t\tOPT_INTEGER(0, \"exit-after\", &idle_in_seconds,\n@@ -391,6 +392,7 @@ int main(int argc, char **argv)\n \t\t\t \"verify shared memory after creating\"),\n \t\tOPT_BOOL(0, \"detach\", &detach, \"detach the process\"),\n \t\tOPT_BOOL(0, \"kill\", &kill, \"request that existing index helper processes exit\"),\n+\t\tOPT_BOOL(0, \"autorun\", &autorun, \"this is an automatic run of git index-helper, so certain errors can be solved by silently exiting\"),\n \t\tOPT_END()\n \t};\n \n@@ -400,7 +402,14 @@ int main(int argc, char **argv)\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(usage_text, options);\n \n-\tprefix = setup_git_directory();\n+\tprefix = setup_git_directory_gently(&nongit);\n+\tif (nongit) {\n+\t\tif (autorun)\n+\t\t\texit(0);\n+\t\telse\n+\t\t\tdie(_(\"not a git repository\"));\n+\t}\n+\n \tif (parse_options(argc, (const char **)argv, prefix,\n \t\t\t options, usage_text, 0))\n \t\tdie(_(\"too many arguments\"));\n@@ -414,10 +423,18 @@ int main(int argc, char **argv)\n \n \t/* check that no other copy is running */\n \tfd = unix_stream_connect(git_path(\"index-helper.sock\"));\n-\tif (fd > 0)\n-\t\tdie(_(\"Already running\"));\n-\tif (errno != ECONNREFUSED && errno != ENOENT)\n-\t\tdie_errno(_(\"Unexpected error checking socket\"));\n+\tif (fd > 0) {\n+\t\tif (autorun)\n+\t\t\texit(0);\n+\t\telse\n+\t\t\tdie(_(\"Already running\"));\n+\t}\n+\tif (errno != ECONNREFUSED && errno != ENOENT) {\n+\t\tif (autorun)\n+\t\t\treturn 0;\n+\t\telse\n+\t\t\tdie_errno(_(\"Unexpected error checking socket\"));\n+\t}\n \n \tatexit(cleanup);\n \tsigchain_push_common(cleanup_on_signal);\ndiff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh\nindex 7159971..aa6e5fc 100755\n--- a/t/t7900-index-helper.sh\n+++ b/t/t7900-index-helper.sh\n@@ -38,4 +38,12 @@ test_expect_success 'index-helper will not start if already running' '\n \tgrep \"Already running\" err\n '\n \n+test_expect_success 'index-helper is quiet with --autorun' '\n+\ttest_when_finished \"git index-helper --kill\" &&\n+\tgit index-helper --kill &&\n+\tgit index-helper --detach &&\n+\ttest -S .git/index-helper.sock &&\n+\tgit index-helper --autorun\n+'\n+\n test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283887","messageId":"1461108489-29376-16-git-send-email-dturner@twopensource.com","threadId":"42080","inReplyTo":"1461108489-29376-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v5 15/15] index-helper: optionally automatically run","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-19T23:28:09Z","receivedAt":"2016-04-19T23:28:09Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Introduce a new config option, indexhelper.autorun, to automatically\nrun git index-helper before starting up a builtin git command. This\nenables users to keep index-helper running without manual\nintervention.\n\nSigned-off-by: David Turner \n---\n Documentation/config.txt | 4 ++++\n read-cache.c | 37 +++++++++++++++++++++++++++++++++++--\n t/t7900-index-helper.sh | 19 +++++++++++++++++++\n 3 files changed, 58 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2cd6bdd..8ec8824 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1852,6 +1852,10 @@ index.version::\n \tSpecify the version with which new index files should be\n \tinitialized. This does not affect existing repositories.\n \n+indexhelper.autorun::\n+\tAutomatically run git index-helper when any builtin git\n+\tcommand is run inside a repository.\n+\n init.templateDir::\n \tSpecify the directory from which templates will be copied.\n \t(See the \"TEMPLATE DIRECTORY\" section of linkgit:git-init[1].)\ndiff --git a/read-cache.c b/read-cache.c\nindex 65f22f9..a73487e 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -20,6 +20,7 @@\n #include \"utf8.h\"\n #include \"unix-socket.h\"\n #include \"ewah/ewok.h\"\n+#include \"run-command.h\"\n \n static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,\n \t\t\t\t\t unsigned int options);\n@@ -1722,6 +1723,33 @@ static void post_read_index_from(struct index_state *istate)\n \ttweak_untracked_cache(istate);\n }\n \n+static int want_auto_index_helper(void)\n+{\n+\tint want = -1;\n+\tconst char *value = NULL;\n+\tconst char *key = \"indexhelper.autorun\";\n+\n+\tif (git_config_key_is_valid(key) &&\n+\t !git_config_get_value(key, &value)) {\n+\t\tint b = git_config_maybe_bool(key, value);\n+\t\twant = b >= 0 ? b : 0;\n+\t}\n+\treturn want;\n+}\n+\n+static void autorun_index_helper(void)\n+{\n+\tconst char *argv[] = {\"git-index-helper\", \"--detach\", \"--autorun\", NULL};\n+\tif (want_auto_index_helper() <= 0)\n+\t\treturn;\n+\n+\ttrace_argv_printf(argv, \"trace: auto index-helper:\");\n+\n+\tif (run_command_v_opt(argv,\n+\t\t\t RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT))\n+\t\twarning(_(\"You specified indexhelper.autorun, but running git-index-helper failed.\"));\n+}\n+\n static int poke_and_wait_for_reply(int fd)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -1776,6 +1804,7 @@ static int poke_daemon(struct index_state *istate,\n \tif (fd < 0) {\n \t\twarning(\"Failed to connect to index-helper socket\");\n \t\tunlink(git_path(\"index-helper.sock\"));\n+\t\tautorun_index_helper();\n \t\treturn -1;\n \t}\n \n@@ -1811,9 +1840,13 @@ static int try_shm(struct index_state *istate)\n \tint fd = -1;\n \n \tif (!is_main_index(istate) ||\n-\t old_size <= 20 ||\n-\t stat(git_path(\"index-helper.sock\"), &st))\n+\t old_size <= 20)\n \t\treturn -1;\n+\n+\tif (stat(git_path(\"index-helper.sock\"), &st)) {\n+\t\tautorun_index_helper();\n+\t\treturn -1;\n+\t}\n \tif (poke_daemon(istate, &st, 0))\n \t\treturn -1;\n \tsha1 = (unsigned char *)istate->mmap + old_size - 20;\ndiff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh\nindex aa6e5fc..2d3ce3c 100755\n--- a/t/t7900-index-helper.sh\n+++ b/t/t7900-index-helper.sh\n@@ -16,6 +16,9 @@ test -n \"$NO_MMAP\" && {\n }\n \n test_expect_success 'index-helper smoke test' '\n+\t# We need an existing commit so that the index exists (otherwise,\n+\t# the index-helper will not be autostarted)\n+\ttest_commit x &&\n \tgit index-helper --exit-after 1 &&\n \ttest_path_is_missing .git/index-helper.sock\n '\n@@ -46,4 +49,20 @@ test_expect_success 'index-helper is quiet with --autorun' '\n \tgit index-helper --autorun\n '\n \n+test_expect_success 'index-helper autorun works' '\n+\trm -f .git/index-helper.sock &&\n+\tgit status &&\n+\ttest_path_is_missing .git/index-helper.sock &&\n+\ttest_config indexhelper.autorun true &&\n+\tgit status &&\n+\ttest -S .git/index-helper.sock &&\n+\tgit status 2>err &&\n+\ttest -S .git/index-helper.sock &&\n+\t! grep -q . err &&\n+\tgit index-helper --kill &&\n+\ttest_config indexhelper.autorun false &&\n+\tgit status &&\n+\ttest_path_is_missing .git/index-helper.sock\n+'\n+\n test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"283892","messageId":"CACsJy8D34SKcA0dsG0ZrsOZiOFLwgEuG-TJ914t4qFV1xZGnew@mail.gmail.com","threadId":"42080","inReplyTo":"1461108489-29376-11-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 10/15] update-index: enable/disable watchman support","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-19T23:45:24Z","receivedAt":"2016-04-19T23:45:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 6:28 AM, David Turner wrote:\n> + if (use_watchman > 0) {\n> + the_index.last_update = xstrdup(\"\");\n> + the_index.cache_changed |= WATCHMAN_CHANGED;\n> + } else if (!use_watchman) {\n> + the_index.last_update = NULL;\n> + the_index.cache_changed |= WATCHMAN_CHANGED;\n> + }\n> +\n\nWe probably should warn people if index-helper is not built with\nwatchman support, which makes this knob completely useless. If\nwatchman fails to start, that's a separate problem..\n-- \nDuy\n"},{"id":"283893","messageId":"CACsJy8Bcb=-V0wc8En2SCSz8jPefEL4qibxJsLsTB-c94x9y3Q@mail.gmail.com","threadId":"42080","inReplyTo":"1461108489-29376-7-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 06/15] index-helper: add --detach","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-19T23:50:54Z","receivedAt":"2016-04-19T23:50:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 6:28 AM, David Turner wrote:\n> @@ -317,6 +320,8 @@ int main(int argc, char **argv)\n> if (fd < 0)\n> die_errno(_(\"could not set up index-helper socket\"));\n>\n> + if (detach && daemonize(&daemonized))\n> + die_errno(_(\"unable to detach\"));\n\nAt the least, I think we need to redirect both stdout and stderr to a\nfile, so we can catch errors. The watchman patch uses warning() to\nreport errors, for example. And there is always a chance of die().\n\nThen we need to report the errors back. I faced the same problem with\ndaemonizing git-gc, but I'm not sure if we can do exactly the same\nhere like in commit 329e6e8 (gc: save log from daemonized gc --auto\nand print it next time - 2015-09-19)\n-- \nDuy\n"},{"id":"283894","messageId":"CACsJy8CM409OH12w3EdVP3UjXoURbWNuqb_coQ=AagdCs+ctaQ@mail.gmail.com","threadId":"42080","inReplyTo":"1461108489-29376-10-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 09/15] index-helper: use watchman to avoid refreshing index with lstat()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T00:15:37Z","receivedAt":"2016-04-20T00:15:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Continuing my comment from the --use-watchman patch about watchman not\nbeing supported...\n\nOn Wed, Apr 20, 2016 at 6:28 AM, David Turner wrote:\n> +static int poke_and_wait_for_reply(int fd)\n> +{\n> + struct strbuf buf = STRBUF_INIT;\n> + struct strbuf reply = STRBUF_INIT;\n> + int ret = -1;\n> + fd_set fds;\n> + struct timeval timeout;\n> +\n> + timeout.tv_usec = 0;\n> + timeout.tv_sec = 1;\n> +\n> + if (fd < 0)\n> + return -1;\n> +\n> + strbuf_addf(&buf, \"poke %d\", getpid());\n> + if (write_in_full(fd, buf.buf, buf.len + 1) != buf.len + 1)\n> + goto done_poke;\n> +\n> + /* Now wait for a reply */\n> + FD_ZERO(&fds);\n> + FD_SET(fd, &fds);\n> + if (select(fd + 1, &fds, NULL, NULL, &timeout) == 0)\n> + /* No reply, giving up */\n> + goto done_poke;\n> +\n> + if (strbuf_getwholeline_fd(&reply, fd, 0))\n> + goto done_poke;\n> +\n> + if (!starts_with(reply.buf, \"OK\"))\n> + goto done_poke;\n\n... while we could simply check USE_WATCHMAN macro and reject in\nupdate-index, a better solution is sending \"poke %d watchman\" and\nreturning \"OK watchman\" (vs \"OK\") when watchman is supported and\nactive. If the user already requests watchman and index-helper returns\njust \"OK\" then we can warn the user the reason of possible performance\ndegradation. It's related to the error reporting, but I don't think\nyou can send straight errors over unix socket. It's possible but it's\na bit more complicated.\n\n> +static void refresh_by_watchman(struct index_state *istate)\n> +{\n> + void *shm = NULL;\n> + int length;\n> + int i;\n> + struct stat st;\n> + int fd = -1;\n> + const char *path = git_path(\"shm-watchman-%s-%\"PRIuMAX,\n> + sha1_to_hex(istate->sha1),\n> + (uintmax_t)getpid());\n> +\n> + fd = open(path, O_RDONLY);\n> + if (fd < 0)\n> + return;\n> +\n> + /*\n> + * This watchman data is just for us -- no need to keep it\n> + * around once we've got it open.\n> + */\n> + unlink(path);\n\nThis will not play well when multiple processes read and refresh the\nindex at the same time. They could refresh non-overlapping\nsubdirectories, and I think it's perfectly ok for them to do so\n(writing index down is a different matter). I don't have a good answer\nfor this. Perhaps if shm-watchman-%s-%d file is small enough (and it\nshould be, we store it in the index), then we can just send the\ncontent straight over unix socket. I didn't have this option with my\nsignal-based communication model.\n\nThis is really extra. But if we know in advance that git does not need\nrefresh(), then we should be able to tell index-helper not to waste\ncycles contacting watchman and preparing shm-watchman-%s-%d (the poke\nline gets more parameters). Either that, or we decouple watchman\nrequests from read_cache() requests. Only when refresh_index() is\ncalled that we send something to request shm-watchman-%s-%d. The same\nfor read_directory() (i.e. untracked cache stuff). Hmm?\n\nNow that I think of it, with watchman backing us, we probably should\njust do nothing in update_index_if_able() (or write_locked_index()\nwhen we know only stat info is changed) when watchman is active. The\npurpose of update_index_if_able() is to avoid costly refresh, but we\ncan already avoid that with watchman. And updating big index files is\nalways costly (even though it should cost less with split-index). Of\ncourse this can only be done if watchman (inotify to be precise) can\ncover whole worktree. I'm not sure how watchman behaves when there's\nnot enough inotify resource to cover full worktree.\n-- \nDuy\n"},{"id":"283898","messageId":"1461114098.5540.158.camel@twopensource.com","threadId":"42080","inReplyTo":"CACsJy8CM409OH12w3EdVP3UjXoURbWNuqb_coQ=AagdCs+ctaQ@mail.gmail.com","subject":"Re: [PATCH v5 09/15] index-helper: use watchman to avoid refreshing index with lstat()","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T01:01:38Z","receivedAt":"2016-04-20T01:01:38Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 07:15 +0700, Duy Nguyen wrote:\n> Continuing my comment from the --use-watchman patch about watchman\n> not\n> being supported...\n> \n> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n> dturner@twopensource.com> wrote:\n> > +static int poke_and_wait_for_reply(int fd)\n> > +{\n> > + struct strbuf buf = STRBUF_INIT;\n> > + struct strbuf reply = STRBUF_INIT;\n> > + int ret = -1;\n> > + fd_set fds;\n> > + struct timeval timeout;\n> > +\n> > + timeout.tv_usec = 0;\n> > + timeout.tv_sec = 1;\n> > +\n> > + if (fd < 0)\n> > + return -1;\n> > +\n> > + strbuf_addf(&buf, \"poke %d\", getpid());\n> > + if (write_in_full(fd, buf.buf, buf.len + 1) != buf.len + 1)\n> > + goto done_poke;\n> > +\n> > + /* Now wait for a reply */\n> > + FD_ZERO(&fds);\n> > + FD_SET(fd, &fds);\n> > + if (select(fd + 1, &fds, NULL, NULL, &timeout) == 0)\n> > + /* No reply, giving up */\n> > + goto done_poke;\n> > +\n> > + if (strbuf_getwholeline_fd(&reply, fd, 0))\n> > + goto done_poke;\n> > +\n> > + if (!starts_with(reply.buf, \"OK\"))\n> > + goto done_poke;\n> \n> ... while we could simply check USE_WATCHMAN macro and reject in\n> update-index, a better solution is sending \"poke %d watchman\" and\n> returning \"OK watchman\" (vs \"OK\") when watchman is supported and\n> active. If the user already requests watchman and index-helper\n> returns\n> just \"OK\" then we can warn the user the reason of possible\n> performance\n> degradation. It's related to the error reporting, but I don't think\n> you can send straight errors over unix socket. It's possible but it's\n> a bit more complicated.\n\nDo you mean that we should do this here? Or in update-index -\n-watchman? If the former, I agree. If the latter, I'm not sure; maybe\nyou'll be setting up your index before you've started the index helper?\n\n> > +static void refresh_by_watchman(struct index_state *istate)\n> > +{\n> > + void *shm = NULL;\n> > + int length;\n> > + int i;\n> > + struct stat st;\n> > + int fd = -1;\n> > + const char *path = git_path(\"shm-watchman-%s-%\"PRIuMAX,\n> > + sha1_to_hex(istate->sha1),\n> > + (uintmax_t)getpid());\n> > +\n> > + fd = open(path, O_RDONLY);\n> > + if (fd < 0)\n> > + return;\n> > +\n> > + /*\n> > + * This watchman data is just for us -- no need to keep it\n> > + * around once we've got it open.\n> > + */\n> > + unlink(path);\n> \n> This will not play well when multiple processes read and refresh the\n> index at the same time. \n\nMultiple processes will have different pids, right? And the pid is\nincluded in the filename. Am I missing something?\n\n> This is really extra. But if we know in advance that git does not \n> need refresh(), then we should be able to tell index-helper not to \n> waste cycles contacting watchman and preparing shm-watchman-%s-%d \n> (the poke line gets more parameters). Either that, or we decouple \n> watchman requests from read_cache() requests. Only when \n> refresh_index() is called that we send something to request shm-\n> watchman-%s-%d. The same for read_directory() (i.e. untracked cache \n> stuff). Hmm?\n\nIt's true that we could decouple watchman requests. I'll look and see\nif that's reasonable.\n\n> Now that I think of it, with watchman backing us, we probably should\n> just do nothing in update_index_if_able() (or write_locked_index()\n> when we know only stat info is changed) when watchman is active. The\n> purpose of update_index_if_able() is to avoid costly refresh, but we\n> can already avoid that with watchman. And updating big index files is\n> always costly (even though it should cost less with split-index). \n\nThat sounds like a change we could make in a separate series. It's not\na bad idea, but if our goal is to get the basic version out, we should\nstart there.\n\n> Of\n> course this can only be done if watchman (inotify to be precise) can\n> cover whole worktree. I'm not sure how watchman behaves when there's\n> not enough inotify resource to cover full worktree.\n\nIt will detect this case and will either manually recrawl (in the event\nof a max_queued_events overflow) or return an error (in the event of\ntoo many watched directories).\n"},{"id":"283899","messageId":"1461114267.5540.161.camel@twopensource.com","threadId":"42080","inReplyTo":"CACsJy8Bcb=-V0wc8En2SCSz8jPefEL4qibxJsLsTB-c94x9y3Q@mail.gmail.com","subject":"Re: [PATCH v5 06/15] index-helper: add --detach","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T01:04:27Z","receivedAt":"2016-04-20T01:04:27Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:\n> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n> dturner@twopensource.com> wrote:\n> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)\n> > if (fd < 0)\n> > die_errno(_(\"could not set up index-helper\n> > socket\"));\n> > \n> > + if (detach && daemonize(&daemonized))\n> > + die_errno(_(\"unable to detach\"));\n> \n> At the least, I think we need to redirect both stdout and stderr to a\n> file, so we can catch errors. The watchman patch uses warning() to\n> report errors, for example. And there is always a chance of die().\n> \n> Then we need to report the errors back. I faced the same problem with\n> daemonizing git-gc, but I'm not sure if we can do exactly the same\n> here like in commit 329e6e8 (gc: save log from daemonized gc --auto\n> and print it next time - 2015-09-19)\n\nI'll add in code to log errors. I'm not sure where it would make sense\nto report the errors. Generally, for errors during a client operation,\nwe would like to report them to the client, but the client might have\nalready disconnected. I guess in that case it's OK if they just go to\nthe log? The client could warn on a timeout while waiting for index\n-helper and direct people to the log.\n"},{"id":"283919","messageId":"alpine.DEB.2.20.1604201059530.2826@virtualbox","threadId":"42080","inReplyTo":"1461108489-29376-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-20T09:01:23Z","receivedAt":"2016-04-20T09:01:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Dave,\n\nOn Tue, 19 Apr 2016, David Turner wrote:\n\n> unmap:\n> +\tistate->mmap = NULL;\n> \tmunmap(mmap, mmap_size);\n> \tdie(\"index file corrupt\");\n> }\n> [...]\n> @@ -1698,6 +1705,10 @@ int discard_index(struct index_state *istate)\n> \tfree(istate->cache);\n> \tistate->cache = NULL;\n> \tistate->cache_alloc = 0;\n> +\tif (istate->keep_mmap && istate->mmap) {\n> +\t\tmunmap(istate->mmap, istate->mmap_size);\n> +\t\tistate->mmap = NULL;\n> +\t}\n> \tdiscard_split_index(istate);\n\nJust curious: any reason why the first hunk munmap()s after resetting the\nfield to NULL and the second hunk does it in the opposite order?\n\nCiao,\nDscho\n"},{"id":"283921","messageId":"CACsJy8D229ARqEjH29Gqd=u8KijGA_Fzk0cbUNRXB9GA0ittMg@mail.gmail.com","threadId":"42080","inReplyTo":"1461108489-29376-3-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T09:26:34Z","receivedAt":"2016-04-20T09:26:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 6:27 AM, David Turner wrote:\n> From: Nguyễn Thái Ngọc Duy \n>\n> Later, we will introduce git index-helper to share this memory with\n> other git processes.\n>\n> Since the memory will be shared, it will never be unmapped (although\n> the kernel may of course choose to page it out).\n\nThis is not entirely true. We do need to keep the mmap'd memory for\nlonger, even after read_index_from() finishes. But we do not share the\nexact same address space to other processes (memcpy is used in\nindex-helper.c:share_index()). We could even discard_index() at the\nend of share_index(), but I think we keep it anyway just in case\nanother process asks, or when index is not updated.\n-- \nDuy\n"},{"id":"283922","messageId":"CACsJy8DbA6Es3-Kb5XXad0+6Mm2VkMtbNCo6rmEio_180L-G0w@mail.gmail.com","threadId":"42080","inReplyTo":"1461114267.5540.161.camel@twopensource.com","subject":"Re: [PATCH v5 06/15] index-helper: add --detach","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T09:33:16Z","receivedAt":"2016-04-20T09:33:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 8:04 AM, David Turner wrote:\n> On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:\n>> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n>> dturner@twopensource.com> wrote:\n>> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)\n>> > if (fd < 0)\n>> > die_errno(_(\"could not set up index-helper\n>> > socket\"));\n>> >\n>> > + if (detach && daemonize(&daemonized))\n>> > + die_errno(_(\"unable to detach\"));\n>>\n>> At the least, I think we need to redirect both stdout and stderr to a\n>> file, so we can catch errors. The watchman patch uses warning() to\n>> report errors, for example. And there is always a chance of die().\n>>\n>> Then we need to report the errors back. I faced the same problem with\n>> daemonizing git-gc, but I'm not sure if we can do exactly the same\n>> here like in commit 329e6e8 (gc: save log from daemonized gc --auto\n>> and print it next time - 2015-09-19)\n>\n> I'll add in code to log errors. I'm not sure where it would make sense\n> to report the errors. Generally, for errors during a client operation,\n> we would like to report them to the client, but the client might have\n> already disconnected. I guess in that case it's OK if they just go to\n> the log? The client could warn on a timeout while waiting for index\n> -helper and direct people to the log.\n\nYeah if the client already disconnects, we have no way but saving the\nerrors somewhere. index-helper can pick up from the log and report to\nthe next client, if you want to keep it simple. If we're building a\nmore complicated protocol on top of unix socket, I suggest you use\npkt-line to wrap/unwrap messages. Bonus point, we can trace what's\nsending/receiving with GIT_TRACE_PACKET.\n-- \nDuy\n"},{"id":"283923","messageId":"CACsJy8CijNvJJzjxh5c-2bxJtEHzUNmR_iTPNLTqHGFQjOP9Eg@mail.gmail.com","threadId":"42080","inReplyTo":"1461114098.5540.158.camel@twopensource.com","subject":"Re: [PATCH v5 09/15] index-helper: use watchman to avoid refreshing index with lstat()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T09:36:22Z","receivedAt":"2016-04-20T09:36:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 8:01 AM, David Turner wrote:\n> On Wed, 2016-04-20 at 07:15 +0700, Duy Nguyen wrote:\n>> Continuing my comment from the --use-watchman patch about watchman\n>> not\n>> being supported...\n>>\n>> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n>> dturner@twopensource.com> wrote:\n>> > +static int poke_and_wait_for_reply(int fd)\n>> > +{\n>> > + struct strbuf buf = STRBUF_INIT;\n>> > + struct strbuf reply = STRBUF_INIT;\n>> > + int ret = -1;\n>> > + fd_set fds;\n>> > + struct timeval timeout;\n>> > +\n>> > + timeout.tv_usec = 0;\n>> > + timeout.tv_sec = 1;\n>> > +\n>> > + if (fd < 0)\n>> > + return -1;\n>> > +\n>> > + strbuf_addf(&buf, \"poke %d\", getpid());\n>> > + if (write_in_full(fd, buf.buf, buf.len + 1) != buf.len + 1)\n>> > + goto done_poke;\n>> > +\n>> > + /* Now wait for a reply */\n>> > + FD_ZERO(&fds);\n>> > + FD_SET(fd, &fds);\n>> > + if (select(fd + 1, &fds, NULL, NULL, &timeout) == 0)\n>> > + /* No reply, giving up */\n>> > + goto done_poke;\n>> > +\n>> > + if (strbuf_getwholeline_fd(&reply, fd, 0))\n>> > + goto done_poke;\n>> > +\n>> > + if (!starts_with(reply.buf, \"OK\"))\n>> > + goto done_poke;\n>>\n>> ... while we could simply check USE_WATCHMAN macro and reject in\n>> update-index, a better solution is sending \"poke %d watchman\" and\n>> returning \"OK watchman\" (vs \"OK\") when watchman is supported and\n>> active. If the user already requests watchman and index-helper\n>> returns\n>> just \"OK\" then we can warn the user the reason of possible\n>> performance\n>> degradation. It's related to the error reporting, but I don't think\n>> you can send straight errors over unix socket. It's possible but it's\n>> a bit more complicated.\n>\n> Do you mean that we should do this here? Or in update-index -\n> -watchman? If the former, I agree. If the latter, I'm not sure; maybe\n> you'll be setting up your index before you've started the index helper?\n\nHere is better than update-index because we can't know what\nindex-helper is capable of (the USE_WATCHMAN macro is more like a\nsuggestion)\n\n>> > +static void refresh_by_watchman(struct index_state *istate)\n>> > +{\n>> > + void *shm = NULL;\n>> > + int length;\n>> > + int i;\n>> > + struct stat st;\n>> > + int fd = -1;\n>> > + const char *path = git_path(\"shm-watchman-%s-%\"PRIuMAX,\n>> > + sha1_to_hex(istate->sha1),\n>> > + (uintmax_t)getpid());\n>> > +\n>> > + fd = open(path, O_RDONLY);\n>> > + if (fd < 0)\n>> > + return;\n>> > +\n>> > + /*\n>> > + * This watchman data is just for us -- no need to keep it\n>> > + * around once we've got it open.\n>> > + */\n>> > + unlink(path);\n>>\n>> This will not play well when multiple processes read and refresh the\n>> index at the same time.\n>\n> Multiple processes will have different pids, right? And the pid is\n> included in the filename. Am I missing something?\n\nAhhh! I thought that pid was index-helper's. Silly me.\n\n>> Now that I think of it, with watchman backing us, we probably should\n>> just do nothing in update_index_if_able() (or write_locked_index()\n>> when we know only stat info is changed) when watchman is active. The\n>> purpose of update_index_if_able() is to avoid costly refresh, but we\n>> can already avoid that with watchman. And updating big index files is\n>> always costly (even though it should cost less with split-index).\n>\n> That sounds like a change we could make in a separate series. It's not\n> a bad idea, but if our goal is to get the basic version out, we should\n> start there.\n\nAgreed. More optimizations can always wait till later. We just need a\ngood foundation first.\n-- \nDuy\n"},{"id":"283924","messageId":"1461146394-5135-1-git-send-email-pclouds@gmail.com","threadId":"42080","inReplyTo":"1461108489-29376-16-git-send-email-dturner@twopensource.com","subject":"[PATCH 16/15] Add tracing to measure where most of the time is spent","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T09:59:54Z","receivedAt":"2016-04-20T09:59:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"All the known heavy code blocks are measured (except object database\naccess). This should help identify if an optimization is effective or\nnot. An unoptimized git-status would give something like below (92% of\ntime is accounted). To sum up the effort of making git scale better:\n\n - read cache line is being addressed by index-helper\n - preload/refresh index by watchman\n - read packed refs by lmdb backend\n - diff-index by rebuilding cache-tree\n - read directory by untracked cache and watchman\n - write index by split index\n - name hash potientially by index-helper\n\nread-cache.c:2075 performance: 0.004058570 s: read cache .../index\npreload-index.c:104 performance: 0.004419864 s: preload index\nread-cache.c:1265 performance: 0.000185224 s: refresh index\nrefs/files-backend.c:1100 performance: 0.001935788 s: read packed refs\ndiff-lib.c:240 performance: 0.000144132 s: diff-files\ndiff-lib.c:506 performance: 0.013592000 s: diff-index\nname-hash.c:128 performance: 0.000614177 s: initialize name hash\ndir.c:2030 performance: 0.015814103 s: read directory\nread-cache.c:2565 performance: 0.004052343 s: write index, changed mask = 2\ntrace.c:420 performance: 0.048365509 s: git command: './git' 'status'\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n This is worth merging, I think.\n\n diff-lib.c | 4 ++++\n dir.c | 2 ++\n name-hash.c | 2 ++\n preload-index.c | 2 ++\n read-cache.c | 11 +++++++++++\n refs/files-backend.c | 2 ++\n 6 files changed, 23 insertions(+)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex bc49c70..7af7f9a 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -90,6 +90,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \tint diff_unmerged_stage = revs->max_count;\n \tunsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)\n \t\t\t ? CE_MATCH_RACY_IS_DIRTY : 0);\n+\tuint64_t start = getnanotime();\n \n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"i/\", \"w/\");\n \n@@ -236,6 +237,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t}\n \tdiffcore_std(&revs->diffopt);\n \tdiff_flush(&revs->diffopt);\n+\ttrace_performance_since(start, \"diff-files\");\n \treturn 0;\n }\n \n@@ -491,6 +493,7 @@ static int diff_cache(struct rev_info *revs,\n int run_diff_index(struct rev_info *revs, int cached)\n {\n \tstruct object_array_entry *ent;\n+\tuint64_t start = getnanotime();\n \n \tent = revs->pending.objects;\n \tif (diff_cache(revs, ent->item->oid.hash, ent->name, cached))\n@@ -500,6 +503,7 @@ int run_diff_index(struct rev_info *revs, int cached)\n \tdiffcore_fix_diff_index(&revs->diffopt);\n \tdiffcore_std(&revs->diffopt);\n \tdiff_flush(&revs->diffopt);\n+\ttrace_performance_since(start, \"diff-index\");\n \treturn 0;\n }\n \ndiff --git a/dir.c b/dir.c\nindex 91003e5..1ebf9fe 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1991,6 +1991,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru\n {\n \tstruct path_simplify *simplify;\n \tstruct untracked_cache_dir *untracked;\n+\tuint64_t start = getnanotime();\n \n \t/*\n \t * Check out create_simplify()\n@@ -2026,6 +2027,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru\n \tfree_simplify(simplify);\n \tqsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);\n \tqsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);\n+\ttrace_performance_since(start, \"read directory %.*s\", len, path);\n \tif (dir->untracked) {\n \t\tstatic struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);\n \t\ttrace_printf_key(&trace_untracked_stats,\ndiff --git a/name-hash.c b/name-hash.c\nindex 6d9f23e..b3966d8 100644\n--- a/name-hash.c\n+++ b/name-hash.c\n@@ -115,6 +115,7 @@ static int cache_entry_cmp(const struct cache_entry *ce1,\n static void lazy_init_name_hash(struct index_state *istate)\n {\n \tint nr;\n+\tuint64_t start = getnanotime();\n \n \tif (istate->name_hash_initialized)\n \t\treturn;\n@@ -124,6 +125,7 @@ static void lazy_init_name_hash(struct index_state *istate)\n \tfor (nr = 0; nr < istate->cache_nr; nr++)\n \t\thash_index_entry(istate, istate->cache[nr]);\n \tistate->name_hash_initialized = 1;\n+\ttrace_performance_since(start, \"initialize name hash\");\n }\n \n void add_name_hash(struct index_state *istate, struct cache_entry *ce)\ndiff --git a/preload-index.c b/preload-index.c\nindex c1fe3a3..7bb4809 100644\n--- a/preload-index.c\n+++ b/preload-index.c\n@@ -72,6 +72,7 @@ static void preload_index(struct index_state *index,\n {\n \tint threads, i, work, offset;\n \tstruct thread_data data[MAX_PARALLEL];\n+\tuint64_t start = getnanotime();\n \n \tif (!core_preload_index)\n \t\treturn;\n@@ -100,6 +101,7 @@ static void preload_index(struct index_state *index,\n \t\tif (pthread_join(p->pthread, NULL))\n \t\t\tdie(\"unable to join threaded lstat\");\n \t}\n+\ttrace_performance_since(start, \"preload index\");\n }\n #endif\n \ndiff --git a/read-cache.c b/read-cache.c\nindex a73487e..3a79581 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1187,6 +1187,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \tconst char *typechange_fmt;\n \tconst char *added_fmt;\n \tconst char *unmerged_fmt;\n+\tuint64_t start = getnanotime();\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@@ -1261,6 +1262,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,\n \n \t\treplace_index_entry(istate, i, new);\n \t}\n+\ttrace_performance_since(start, \"refresh index\");\n \treturn has_errors;\n }\n \n@@ -2062,12 +2064,15 @@ int read_index_from(struct index_state *istate, const char *path)\n {\n \tstruct split_index *split_index;\n \tint ret;\n+\tuint64_t start;\n \n \t/* istate->initialized covers both .git/index and .git/sharedindex.xxx */\n \tif (istate->initialized)\n \t\treturn istate->cache_nr;\n \n+\tstart = getnanotime();\n \tret = do_read_index(istate, path, 0);\n+\ttrace_performance_since(start, \"read cache %s\", path);\n \n \tsplit_index = istate->split_index;\n \tif (!split_index || is_null_sha1(split_index->base_sha1)) {\n@@ -2082,6 +2087,7 @@ int read_index_from(struct index_state *istate, const char *path)\n \tsplit_index->base->keep_mmap = istate->keep_mmap;\n \tsplit_index->base->to_shm = istate->to_shm;\n \tsplit_index->base->from_shm = istate->from_shm;\n+\tstart = getnanotime();\n \tret = do_read_index(split_index->base,\n \t\t\t git_path(\"sharedindex.%s\",\n \t\t\t\t sha1_to_hex(split_index->base_sha1)), 1);\n@@ -2093,6 +2099,9 @@ int read_index_from(struct index_state *istate, const char *path)\n \t\t sha1_to_hex(split_index->base->sha1));\n \tmerge_base_index(istate);\n \tpost_read_index_from(istate);\n+\ttrace_performance_since(start, \"read cache %s\",\n+\t\t\t\tgit_path(\"sharedindex.%s\",\n+\t\t\t\t\t sha1_to_hex(split_index->base_sha1)));\n \n done:\n \tif (ret > 0 && istate->from_shm && istate->last_update)\n@@ -2437,6 +2446,7 @@ static int do_write_index(struct index_state *istate, int newfd,\n \tint entries = istate->cache_nr;\n \tstruct stat st;\n \tstruct strbuf previous_name_buf = STRBUF_INIT, *previous_name;\n+\tuint64_t start = getnanotime();\n \n \tfor (i = removed = extended = 0; i < entries; i++) {\n \t\tif (cache[i]->ce_flags & CE_REMOVE)\n@@ -2552,6 +2562,7 @@ static int do_write_index(struct index_state *istate, int newfd,\n \t\treturn -1;\n \tistate->timestamp.sec = (unsigned int)st.st_mtime;\n \tistate->timestamp.nsec = ST_MTIME_NSEC(st);\n+\ttrace_performance_since(start, \"write index, changed mask = %x\", istate->cache_changed);\n \treturn 0;\n }\n \ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex ea78ce9..b88913f 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1048,6 +1048,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)\n \tstruct ref_entry *last = NULL;\n \tstruct strbuf line = STRBUF_INIT;\n \tenum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;\n+\tuint64_t start = getnanotime();\n \n \twhile (strbuf_getwholeline(&line, f, '\\n') != EOF) {\n \t\tunsigned char sha1[20];\n@@ -1096,6 +1097,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)\n \t}\n \n \tstrbuf_release(&line);\n+\ttrace_performance_since(start, \"read packed refs\");\n }\n \n /*\n-- \n2.8.0.rc0.210.gd302cd2\n"},{"id":"283927","messageId":"alpine.DEB.2.20.1604201349590.2826@virtualbox","threadId":"42080","inReplyTo":"1461108489-29376-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v5 03/15] index-helper: new daemon for caching index and related stuff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-20T12:17:06Z","receivedAt":"2016-04-20T12:17:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Dave,\n\n(apologies in advance if I may bring up anything that has been discussed\nin earlier iterations; I simply was too busy with the rebase--helper\nproject to even look.)\n\nOn Tue, 19 Apr 2016, David Turner wrote:\n\n> Shared memory is done by storing files in a per-repository temporary\n> directory. This is more portable than shm (which requires\n> posix-realtime and has various quirks on OS X). It might even work on\n> Windows, although this has not been tested. The shared memory file's\n> name folows the template \"shm--\" where is the\n\ns/folows/follows/\n\nAnd: now that it is no longer shared memory, should we not do away with\nthe \"shm-\" prefix?\n\n> trailing SHA-1 of the index file. is \"index\" for cached index\n> files (and might later be \"name-hash\" for name-hash cache). If such\n> shared memory exists, it contains the same index content as on\n> disk. The content is already validated by the daemon and git won't\n> validate it again (except comparing the trailing SHA-1s).\n\nExcellent. I was worrying about the penalty of validating.\n\n> We keep this daemon's logic as thin as possible. The \"brain\" stays in\n> git. So the daemon can read and validate stuff, but that's all it's\n> allowed to do. It does not add/create new information. It doesn't even\n> accept direct updates from git.\n\nI like this design. For now. Later, I really think we should add the\nability to update the index via the index-helper. I am thinking about a\nmethod similar to watchman where a separate process (that may use the\ninotify syscall on Linux, or tap into the NTFS journal on Windows) tells\nthe index-helper specifically which paths to look at, so that nobody ever\nhas to look at any files that were not modified at all.\n\n> +\tif (*new_mmap == MAP_FAILED) {\n> +\t\t*new_mmap = NULL;\n> +\t\tgoto done;\n\nShouldn't we provide some sort of error message here?\n\n> +\t}\n> +\tmadvise(new_mmap, size, MADV_WILLNEED);\n\nI guess I'll need to add support for that to compat/win32mmap.c (most\nlikely using PrefetchVirtualMemory() when available, i.e. Windows >= 8, see\nhttps://msdn.microsoft.com/en-us/library/windows/desktop/hh780543.aspx :-))\n\nLikewise, I think it will be easy to use bidirectional named pipes on\nWindows to accommodate for the lack of Unix domain sockets...\n\nCiao,\nDscho\n"},{"id":"283929","messageId":"alpine.DEB.2.20.1604201427440.2826@virtualbox","threadId":"42080","inReplyTo":"1461146394-5135-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 16/15] Add tracing to measure where most of the time is spent","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-20T12:28:24Z","receivedAt":"2016-04-20T12:28:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Wed, 20 Apr 2016, Nguyễn Thái Ngọc Duy wrote:\n\n> All the known heavy code blocks are measured (except object database\n> access). This should help identify if an optimization is effective or\n> not. An unoptimized git-status would give something like below (92% of\n> time is accounted). To sum up the effort of making git scale better:\n> \n> - read cache line is being addressed by index-helper\n> - preload/refresh index by watchman\n> - read packed refs by lmdb backend\n> - diff-index by rebuilding cache-tree\n> - read directory by untracked cache and watchman\n> - write index by split index\n> - name hash potientially by index-helper\n> \n> read-cache.c:2075 performance: 0.004058570 s: read cache .../index\n> preload-index.c:104 performance: 0.004419864 s: preload index\n> read-cache.c:1265 performance: 0.000185224 s: refresh index\n> refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs\n> diff-lib.c:240 performance: 0.000144132 s: diff-files\n> diff-lib.c:506 performance: 0.013592000 s: diff-index\n> name-hash.c:128 performance: 0.000614177 s: initialize name hash\n> dir.c:2030 performance: 0.015814103 s: read directory\n> read-cache.c:2565 performance: 0.004052343 s: write index, changed mask = 2\n> trace.c:420 performance: 0.048365509 s: git command: './git' 'status'\n\nThank you for doing this! It will be *highly* valuable to get the\nperformance on Windows where I want it to be, too.\n\nCiao,\nDscho"},{"id":"283930","messageId":"CACsJy8D7_MhzG0yYC-fWjBeqeceVj5aPo2-vYdBMdX6xaSVZrg@mail.gmail.com","threadId":"42080","inReplyTo":"alpine.DEB.2.20.1604201349590.2826@virtualbox","subject":"Re: [PATCH v5 03/15] index-helper: new daemon for caching index and related stuff","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T12:31:55Z","receivedAt":"2016-04-20T12:31:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 7:17 PM, Johannes Schindelin\n wrote:\n>> We keep this daemon's logic as thin as possible. The \"brain\" stays in\n>> git. So the daemon can read and validate stuff, but that's all it's\n>> allowed to do. It does not add/create new information. It doesn't even\n>> accept direct updates from git.\n>\n> I like this design. For now. Later, I really think we should add the\n> ability to update the index via the index-helper.\n\nLater we do. For watchman, index-helper creates and shares a piece of\ninformation retrieved from watchman, which has to be combined back to\nthe index by git process. But it's still not a _direct_ index update.\n\n> I am thinking about a\n> method similar to watchman where a separate process (that may use the\n> inotify syscall on Linux, or tap into the NTFS journal on Windows) tells\n> the index-helper specifically which paths to look at, so that nobody ever\n> has to look at any files that were not modified at all.\n\nAm I missing something here? I thought watchman could already run on\nWindows. We benefit from watchman, which was originally written for\nMercurial. If there's a better way to do inotify equivalent for\nWindows, is it possible to do it in watchman so Mercurial can benefit\nfrom it too?\n\nOn Wed, Apr 20, 2016 at 7:19 PM, Johannes Schindelin\n wrote:\n>> On Wed, Apr 20, 2016 at 6:27 AM, David Turner wrote:\n>> > Shared memory is done by storing files in a per-repository temporary\n>> > directory. This is more portable than shm (which requires\n>> > posix-realtime and has various quirks on OS X). It might even work on\n>> > Windows, although this has not been tested.\n>>\n>> There's another option, but I'm not sure if it's too clever/tricky to\n>> do. Anyway, on *nix we can send file descriptors over unix socket [2],\n>> then mmap them back to access content.\n>\n> This sounds too clever to me ;-)\n\nWell. At least we have considered everything (that I'm aware of).\n-- \nDuy\n"},{"id":"283931","messageId":"CACsJy8A_M7pjVFea3oK82dz5h7SHJ=-eCEwUmv6St6q1YOuwEw@mail.gmail.com","threadId":"42080","inReplyTo":"alpine.DEB.2.20.1604201427440.2826@virtualbox","subject":"Re: [PATCH 16/15] Add tracing to measure where most of the time is spent","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-04-20T12:36:02Z","receivedAt":"2016-04-20T12:36:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Apr 20, 2016 at 7:28 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Wed, 20 Apr 2016, Nguyễn Thái Ngọc Duy wrote:\n>\n>> All the known heavy code blocks are measured (except object database\n>> access). This should help identify if an optimization is effective or\n>> not. An unoptimized git-status would give something like below (92% of\n>> time is accounted). To sum up the effort of making git scale better:\n>>\n>> - read cache line is being addressed by index-helper\n>> - preload/refresh index by watchman\n>> - read packed refs by lmdb backend\n>> - diff-index by rebuilding cache-tree\n>> - read directory by untracked cache and watchman\n>> - write index by split index\n>> - name hash potientially by index-helper\n>>\n>> read-cache.c:2075 performance: 0.004058570 s: read cache .../index\n>> preload-index.c:104 performance: 0.004419864 s: preload index\n>> read-cache.c:1265 performance: 0.000185224 s: refresh index\n>> refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs\n>> diff-lib.c:240 performance: 0.000144132 s: diff-files\n>> diff-lib.c:506 performance: 0.013592000 s: diff-index\n>> name-hash.c:128 performance: 0.000614177 s: initialize name hash\n>> dir.c:2030 performance: 0.015814103 s: read directory\n>> read-cache.c:2565 performance: 0.004052343 s: write index, changed mask = 2\n>> trace.c:420 performance: 0.048365509 s: git command: './git' 'status'\n>\n> Thank you for doing this! It will be *highly* valuable to get the\n> performance on Windows where I want it to be, too.\n\nJust to be clear, these are unoptimized numbers, no untracked cache,\nno split index, no index helper, and on a tiny repository that is\ngit.git. It's mostly to show the percentage of time spent in each\nphase. We probably can do similar measurement and record it in the\nwatchman patch (can't wait for lmdb to be here :).\n-- \nDuy\n"},{"id":"283984","messageId":"1461181137.5540.167.camel@twopensource.com","threadId":"42080","inReplyTo":"alpine.DEB.2.20.1604201349590.2826@virtualbox","subject":"Re: [PATCH v5 03/15] index-helper: new daemon for caching index and related stuff","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T19:38:57Z","receivedAt":"2016-04-20T19:38:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 14:17 +0200, Johannes Schindelin wrote:\n> Hi Dave,\n> \n> (apologies in advance if I may bring up anything that has been\n> discussed\n> in earlier iterations; I simply was too busy with the rebase--helper\n> project to even look.)\n> \n> On Tue, 19 Apr 2016, David Turner wrote:\n> \n> > Shared memory is done by storing files in a per-repository\n> > temporary\n> > directory. This is more portable than shm (which requires\n> > posix-realtime and has various quirks on OS X). It might even work\n> > on\n> > Windows, although this has not been tested. The shared memory\n> > file's\n> > name folows the template \"shm--\" where is the\n> \n> s/folows/follows/\n\nWill fix, thanks.\n\n> And: now that it is no longer shared memory, should we not do away\n> with\n> the \"shm-\" prefix?\n\nHm. It's intended to be shared via MAP_SHARED. So it is just a \"disk\n-backed\" shared memory object instead of a POSIX shm object.\n\n> > +\tif (*new_mmap == MAP_FAILED) {\n> > +\t\t*new_mmap = NULL;\n> > +\t\tgoto done;\n> \n> Shouldn't we provide some sort of error message here?\n\nWe generally try to fail silently on index-helper failures -- the user\nisn't necessarily expecting output at that point, and we can always\nfall back to directly reading the index.\n\nOn the other hand, this error should pretty much never happen, so maybe\nit's worth calling out?\n"},{"id":"283985","messageId":"1461181284.5540.169.camel@twopensource.com","threadId":"42080","inReplyTo":"alpine.DEB.2.20.1604201059530.2826@virtualbox","subject":"Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T19:41:24Z","receivedAt":"2016-04-20T19:41:24Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 11:01 +0200, Johannes Schindelin wrote:\n> Hi Dave,\n> \n> On Tue, 19 Apr 2016, David Turner wrote:\n> \n> > unmap:\n> > +\tistate->mmap = NULL;\n> > \tmunmap(mmap, mmap_size);\n> > \tdie(\"index file corrupt\");\n> > }\n> > [...]\n> > @@ -1698,6 +1705,10 @@ int discard_index(struct index_state\n> > *istate)\n> > \tfree(istate->cache);\n> > \tistate->cache = NULL;\n> > \tistate->cache_alloc = 0;\n> > +\tif (istate->keep_mmap && istate->mmap) {\n> > +\t\tmunmap(istate->mmap, istate->mmap_size);\n> > +\t\tistate->mmap = NULL;\n> > +\t}\n> > \tdiscard_split_index(istate);\n> \n> Just curious: any reason why the first hunk munmap()s after resetting\n> the\n> field to NULL and the second hunk does it in the opposite order?\n\nNo idea. Will change the first hunk.\n"},{"id":"283986","messageId":"1461181395.5540.170.camel@twopensource.com","threadId":"42080","inReplyTo":"CACsJy8D229ARqEjH29Gqd=u8KijGA_Fzk0cbUNRXB9GA0ittMg@mail.gmail.com","subject":"Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T19:43:15Z","receivedAt":"2016-04-20T19:43:15Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 16:26 +0700, Duy Nguyen wrote:\n> On Wed, Apr 20, 2016 at 6:27 AM, David Turner <\n> dturner@twopensource.com> wrote:\n> > From: Nguyễn Thái Ngọc Duy \n> > \n> > Later, we will introduce git index-helper to share this memory with\n> > other git processes.\n> > \n> > Since the memory will be shared, it will never be unmapped\n> > (although\n> > the kernel may of course choose to page it out).\n> \n> This is not entirely true. We do need to keep the mmap'd memory for\n> longer, even after read_index_from() finishes. But we do not share\n> the\n> exact same address space to other processes (memcpy is used in\n> index-helper.c:share_index()). We could even discard_index() at the\n> end of share_index(), but I think we keep it anyway just in case\n> another process asks, or when index is not updated.\n\nWill reword.\n"},{"id":"283989","messageId":"1461181840.5540.171.camel@twopensource.com","threadId":"42080","inReplyTo":"CACsJy8D34SKcA0dsG0ZrsOZiOFLwgEuG-TJ914t4qFV1xZGnew@mail.gmail.com","subject":"Re: [PATCH v5 10/15] update-index: enable/disable watchman support","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-20T19:50:40Z","receivedAt":"2016-04-20T19:50:40Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 06:45 +0700, Duy Nguyen wrote:\n> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n> dturner@twopensource.com> wrote:\n> > + if (use_watchman > 0) {\n> > + the_index.last_update = xstrdup(\"\");\n> > + the_index.cache_changed |= WATCHMAN_CHANGED;\n> > + } else if (!use_watchman) {\n> > + the_index.last_update = NULL;\n> > + the_index.cache_changed |= WATCHMAN_CHANGED;\n> > + }\n> > +\n> \n> We probably should warn people if index-helper is not built with\n> watchman support, which makes this knob completely useless. If\n> watchman fails to start, that's a separate problem..\n\nWill warn.\n"},{"id":"284477","messageId":"1461617586.24444.4.camel@twopensource.com","threadId":"42080","inReplyTo":"CACsJy8Bcb=-V0wc8En2SCSz8jPefEL4qibxJsLsTB-c94x9y3Q@mail.gmail.com","subject":"Re: [PATCH v5 06/15] index-helper: add --detach","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-25T20:53:06Z","receivedAt":"2016-04-25T20:53:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:\n> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <\n> dturner@twopensource.com> wrote:\n> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)\n> > if (fd < 0)\n> > die_errno(_(\"could not set up index-helper\n> > socket\"));\n> > \n> > + if (detach && daemonize(&daemonized))\n> > + die_errno(_(\"unable to detach\"));\n> \n> At the least, I think we need to redirect both stdout and stderr to a\n> file, so we can catch errors. The watchman patch uses warning() to\n> report errors, for example. And there is always a chance of die().\n> \n> Then we need to report the errors back. I faced the same problem with\n> daemonizing git-gc, but I'm not sure if we can do exactly the same\n> here like in commit 329e6e8 (gc: save log from daemonized gc --auto\n> and print it next time - 2015-09-19)\n\nOn reflection, I decided not to do a complicated system for replaying\nwarnings. Here are my reasons why:\n\n1. A user will not be expecting to see warnings from previous git\ncommands. \n\n2. It's not super-important that users see most of these warnings. GC\nis different because it's critical to the health of a repository so it\nreally matters that users learn about issues. \n\n3. It involves many complications to the (presently very simple)\nprotocol. We would need to distinguish between messages, warnings, and\nprevious-session warnings.\n\n4. There are only a few cases where errors will happen, and none seem\nto be exciting to clients.\n\nInstead, I'll just log errors.\n"}]}