* [PATCH v9 01/19] read-cache.c: fix constness of verify_hdr()
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 02/19] read-cache: allow to keep mmap'd memory after reading David Turner
` (18 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
read-cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index d9fb78b..16cc487 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1345,7 +1345,7 @@ struct ondisk_cache_entry_extended {
ondisk_cache_entry_extended_size(ce_namelen(ce)) : \
ondisk_cache_entry_size(ce_namelen(ce)))
-static int verify_hdr(struct cache_header *hdr, unsigned long size)
+static int verify_hdr(const struct cache_header *hdr, unsigned long size)
{
git_SHA_CTX c;
unsigned char sha1[20];
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 02/19] read-cache: allow to keep mmap'd memory after reading
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
2016-05-09 20:48 ` [PATCH v9 01/19] read-cache.c: fix constness of verify_hdr() David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff David Turner
` (17 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Later, we will introduce git index-helper to share this memory with
other git processes.
We only unmap it when we discard the index (although the kernel may of
course choose to page it out).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
cache.h | 3 +++
read-cache.c | 13 ++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/cache.h b/cache.h
index b829410..4180e2b 100644
--- a/cache.h
+++ b/cache.h
@@ -333,11 +333,14 @@ struct index_state {
struct split_index *split_index;
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
+ keep_mmap : 1,
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
unsigned char sha1[20];
struct untracked_cache *untracked;
+ void *mmap;
+ size_t mmap_size;
};
extern struct index_state the_index;
diff --git a/read-cache.c b/read-cache.c
index 16cc487..3cb0ec3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1574,6 +1574,10 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
if (mmap == MAP_FAILED)
die_errno("unable to map index file");
+ if (istate->keep_mmap) {
+ istate->mmap = mmap;
+ istate->mmap_size = mmap_size;
+ }
close(fd);
hdr = mmap;
@@ -1626,11 +1630,13 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
src_offset += 8;
src_offset += extsize;
}
- munmap(mmap, mmap_size);
+ if (!istate->keep_mmap)
+ munmap(mmap, mmap_size);
return istate->cache_nr;
unmap:
munmap(mmap, mmap_size);
+ istate->mmap = NULL;
die("index file corrupt");
}
@@ -1655,6 +1661,7 @@ int read_index_from(struct index_state *istate, const char *path)
discard_index(split_index->base);
else
split_index->base = xcalloc(1, sizeof(*split_index->base));
+ split_index->base->keep_mmap = istate->keep_mmap;
ret = do_read_index(split_index->base,
git_path("sharedindex.%s",
sha1_to_hex(split_index->base_sha1)), 1);
@@ -1698,6 +1705,10 @@ int discard_index(struct index_state *istate)
free(istate->cache);
istate->cache = NULL;
istate->cache_alloc = 0;
+ if (istate->keep_mmap && istate->mmap) {
+ munmap(istate->mmap, istate->mmap_size);
+ istate->mmap = NULL;
+ }
discard_split_index(istate);
free_untracked_cache(istate->untracked);
istate->untracked = NULL;
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
2016-05-09 20:48 ` [PATCH v9 01/19] read-cache.c: fix constness of verify_hdr() David Turner
2016-05-09 20:48 ` [PATCH v9 02/19] read-cache: allow to keep mmap'd memory after reading David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-10 10:13 ` SZEDER Gábor
2016-05-09 20:48 ` [PATCH v9 04/19] index-helper: add --strict David Turner
` (16 subsequent siblings)
19 siblings, 1 reply; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner, Ramsay Jones
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Instead of reading the index from disk and worrying about disk
corruption, the index is cached in memory (memory bit-flips happen
too, but hopefully less often). The result is faster read. Read time
is reduced by 70%.
The biggest gain is not having to verify the trailing SHA-1, which
takes lots of time especially on large index files. But this also
opens doors for further optimizations:
- we could create an in-memory format that's essentially the memory
dump of the index to eliminate most of parsing/allocation
overhead. The mmap'd memory can be used straight away. Experiment
[1] shows we could reduce read time by 88%.
- we could cache non-index info such as name hash
Shared memory is done by storing files in a per-repository temporary
directory. This is more portable than shm (which requires
posix-realtime and has various quirks on OS X). It might even work on
Windows, although this has not been tested. The shared memory file's
name follows the template "shm-<object>-<SHA1>" where <SHA1> is the
trailing SHA-1 of the index file. <object> is "index" for cached index
files (and might later be "name-hash" for name-hash cache). If such
shared memory exists, it contains the same index content as on
disk. The content is already validated by the daemon and git won't
validate it again (except comparing the trailing SHA-1s).
We also add some bits to the index (to_shm and from_shm) to track
when an index came from shared memory or is going to shared memory.
We keep this daemon's logic as thin as possible. The "brain" stays in
git. So the daemon can read and validate stuff, but that's all it's
allowed to do. It does not add/create new information. It doesn't even
accept direct updates from git.
Git can poke the daemon via unix domain sockets to tell it to refresh
the index cache, or to keep it alive some more minutes. It can't give
any real index data directly to the daemon. Real data goes to disk
first, then the daemon reads and verifies it from there. The daemon
only handles $GIT_DIR/index, not temporary index files; it only gets
poked for the former.
$GIT_DIR/index-helper.sock is the socket for the daemon process. The
daemon reads from the socket and executes commands.
Named pipes were considered for portability reasons, but then commands
that need replies from the daemon would have to open their own pipes,
since a named pipe should only have one reader. Unix domain sockets
don't have this problem.
On webkit.git with index format v2, duplicating 8 times to 1.5m
entries and 236MB in size:
(vanilla) 0.50 s: read_index_from .git/index
(index-helper) 0.18 s: read_index_from .git/index
Interestingly with index v4, we get less out of index-helper. It makes
sense as v4 requires more processing after loading the index:
(vanilla) 0.37 s: read_index_from .git/index
(index-helper) 0.22 s: read_index_from .git/index
[1] http://thread.gmane.org/gmane.comp.version-control.git/247268/focus=248771
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
.gitignore | 1 +
Documentation/git-index-helper.txt | 50 +++++++
Makefile | 5 +
cache.h | 11 ++
git-compat-util.h | 1 +
index-helper.c | 277 +++++++++++++++++++++++++++++++++++++
read-cache.c | 125 +++++++++++++++--
t/t7900-index-helper.sh | 23 +++
8 files changed, 484 insertions(+), 9 deletions(-)
create mode 100644 Documentation/git-index-helper.txt
create mode 100644 index-helper.c
create mode 100755 t/t7900-index-helper.sh
diff --git a/.gitignore b/.gitignore
index 5087ce1..b92f122 100644
--- a/.gitignore
+++ b/.gitignore
@@ -71,6 +71,7 @@
/git-http-fetch
/git-http-push
/git-imap-send
+/git-index-helper
/git-index-pack
/git-init
/git-init-db
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
new file mode 100644
index 0000000..f892184
--- /dev/null
+++ b/Documentation/git-index-helper.txt
@@ -0,0 +1,50 @@
+git-index-helper(1)
+===================
+
+NAME
+----
+git-index-helper - A simple cache daemon for speeding up index file access
+
+SYNOPSIS
+--------
+[verse]
+'git index-helper' [options]
+
+DESCRIPTION
+-----------
+Keep the index file in memory for faster access. This daemon is per
+repository and per working tree. So if you have two working trees
+each with a submodule, you might need four index-helpers. (In practice,
+this is only worthwhile for large indexes, so only use it if you notice
+that git status is slow).
+
+OPTIONS
+-------
+
+--exit-after=<n>::
+ Exit if the cached index is not accessed for `<n>`
+ seconds. Specify 0 to wait forever. Default is 600.
+
+NOTES
+-----
+
+$GIT_DIR/index-helper.sock a Unix domain socket that the daemon reads
+commands from. The directory will also contain files named
+"shm-index-<SHA1>". These are used as backing stores for shared
+memory. Normally the daemon will clean up these files when it exits
+or when they are no longer relevant. But if it crashes, some objects
+could remain there and they can be safely deleted with "rm"
+command. The following commands are used to control the daemon:
+
+"refresh"::
+ Reread the index.
+
+"poke":
+ Let the daemon know the index is to be read. It keeps the
+ daemon alive longer, unless `--exit-after=0` is used.
+
+All commands and replies are terminated by a NUL byte.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 2742a69..c8be0e7 100644
--- a/Makefile
+++ b/Makefile
@@ -1433,6 +1433,10 @@ ifdef HAVE_DEV_TTY
BASIC_CFLAGS += -DHAVE_DEV_TTY
endif
+ifndef NO_MMAP
+ PROGRAM_OBJS += index-helper.o
+endif
+
ifdef DIR_HAS_BSD_GROUP_SEMANTICS
COMPAT_CFLAGS += -DDIR_HAS_BSD_GROUP_SEMANTICS
endif
@@ -2159,6 +2163,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
+ @echo NO_MMAP=\''$(subst ','\'',$(subst ','\'',$(NO_MMAP)))'\' >>$@+
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
endif
diff --git a/cache.h b/cache.h
index 4180e2b..2d7af6f 100644
--- a/cache.h
+++ b/cache.h
@@ -334,6 +334,17 @@ struct index_state {
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
keep_mmap : 1,
+
+ /*
+ * This index came from index-helper originally.
+ */
+ from_shm : 1,
+ /*
+ * We're moving this index to shared memory, so we
+ * don't need to poke the daemon to request updates
+ * on it.
+ */
+ to_shm : 1,
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
diff --git a/git-compat-util.h b/git-compat-util.h
index c07e0c1..56945a7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -513,6 +513,7 @@ static inline int ends_with(const char *str, const char *suffix)
#define PROT_READ 1
#define PROT_WRITE 2
#define MAP_PRIVATE 1
+#define MAP_SHARED 2
#endif
#define mmap git_mmap
diff --git a/index-helper.c b/index-helper.c
new file mode 100644
index 0000000..b9443f4
--- /dev/null
+++ b/index-helper.c
@@ -0,0 +1,277 @@
+#include "cache.h"
+#include "parse-options.h"
+#include "sigchain.h"
+#include "strbuf.h"
+#include "exec_cmd.h"
+#include "split-index.h"
+#include "lockfile.h"
+#include "cache.h"
+#include "unix-socket.h"
+#include "pkt-line.h"
+
+struct shm {
+ unsigned char sha1[20];
+ void *shm;
+ size_t size;
+};
+
+static struct shm shm_index;
+static struct shm shm_base_index;
+
+static void release_index_shm(struct shm *is)
+{
+ if (!is->shm)
+ return;
+ munmap(is->shm, is->size);
+ unlink(git_path("shm-index-%s", sha1_to_hex(is->sha1)));
+ is->shm = NULL;
+}
+
+static void cleanup_shm(void)
+{
+ release_index_shm(&shm_index);
+ release_index_shm(&shm_base_index);
+}
+
+static void cleanup(void)
+{
+ unlink(git_path("index-helper.sock"));
+ cleanup_shm();
+}
+
+static void cleanup_on_signal(int sig)
+{
+ /*
+ * We ignore sigpipes -- that's just a client suddenly dying.
+ */
+ if (sig == SIGPIPE)
+ return;
+ cleanup();
+ sigchain_pop(sig);
+ raise(sig);
+}
+
+static int shared_mmap_create(size_t size, void **new_mmap, const char *path)
+{
+ int fd = -1;
+ int ret = -1;
+
+ fd = open(path, O_CREAT | O_EXCL | O_RDWR, 0600);
+
+ if (fd < 0)
+ goto done;
+
+ if (ftruncate(fd, size))
+ goto done;
+
+ *new_mmap = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+ fd, 0);
+
+ if (*new_mmap == MAP_FAILED) {
+ *new_mmap = NULL;
+ goto done;
+ }
+ madvise(new_mmap, size, MADV_WILLNEED);
+
+ ret = 0;
+done:
+ if (fd > 0)
+ close(fd);
+ return ret;
+}
+
+static void share_index(struct index_state *istate, struct shm *is)
+{
+ void *new_mmap;
+ if (istate->mmap_size <= 20 ||
+ hashcmp(istate->sha1,
+ (unsigned char *)istate->mmap + istate->mmap_size - 20) ||
+ !hashcmp(istate->sha1, is->sha1))
+ /* Nothing to do */
+ return;
+ if (shared_mmap_create(istate->mmap_size, &new_mmap,
+ git_path("shm-index-%s",
+ sha1_to_hex(istate->sha1))) < 0) {
+ die("Failed to create shm-index file");
+ }
+
+
+ release_index_shm(is);
+ is->size = istate->mmap_size;
+ is->shm = new_mmap;
+ hashcpy(is->sha1, istate->sha1);
+
+ memcpy(new_mmap, istate->mmap, istate->mmap_size - 20);
+
+ /*
+ * The trailing hash must be written last after everything is
+ * written. It's the indication that the shared memory is now
+ * ready.
+ * The memory barrier here matches read-cache.c:try_shm.
+ */
+ __sync_synchronize();
+
+ hashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);
+}
+
+static void share_the_index(void)
+{
+ if (the_index.split_index && the_index.split_index->base)
+ share_index(the_index.split_index->base, &shm_base_index);
+ share_index(&the_index, &shm_index);
+ discard_index(&the_index);
+}
+
+static void set_socket_blocking_flag(int fd, int make_nonblocking)
+{
+ int flags;
+
+ flags = fcntl(fd, F_GETFL, NULL);
+
+ if (flags < 0)
+ die(_("fcntl failed"));
+
+ if (make_nonblocking)
+ flags |= O_NONBLOCK;
+ else
+ flags &= ~O_NONBLOCK;
+
+ if (fcntl(fd, F_SETFL, flags) < 0)
+ die(_("fcntl failed"));
+}
+
+static void refresh(void)
+{
+ discard_index(&the_index);
+ the_index.keep_mmap = 1;
+ the_index.to_shm = 1;
+ if (read_cache() < 0)
+ die(_("could not read index"));
+ share_the_index();
+}
+
+#ifndef NO_MMAP
+
+static void loop(int fd, int idle_in_seconds)
+{
+ assert(idle_in_seconds < INT_MAX / 1000);
+
+ while (1) {
+ struct pollfd pollfd;
+ int result, client_fd;
+ int flags;
+ char buf[4096];
+ int bytes_read;
+
+ /* Wait for a request */
+ pollfd.fd = fd;
+ pollfd.events = POLLIN;
+ result = poll(&pollfd, 1, idle_in_seconds * 1000);
+ if (result < 0) {
+ if (errno == EINTR)
+ /*
+ * This can lead to an overlong keepalive,
+ * but that is better than a premature exit.
+ */
+ continue;
+ die_errno(_("poll() failed"));
+ } else if (result == 0)
+ /* timeout */
+ break;
+
+ client_fd = accept(fd, NULL, NULL);
+ if (client_fd < 0)
+ /*
+ * An error here is unlikely -- it probably
+ * indicates that the connecting process has
+ * already dropped the connection.
+ */
+ continue;
+
+ /*
+ * Our connection to the client is blocking since a client
+ * can always be killed by SIGINT or similar.
+ */
+ set_socket_blocking_flag(client_fd, 0);
+
+ flags = PACKET_READ_GENTLE_ON_EOF | PACKET_READ_CHOMP_NEWLINE;
+ bytes_read = packet_read(client_fd, NULL, NULL, buf,
+ sizeof(buf), flags);
+
+ if (bytes_read > 0) {
+ /* ensure string termination */
+ buf[bytes_read] = 0;
+ if (!strcmp(buf, "refresh")) {
+ refresh();
+ } else if (!strcmp(buf, "poke")) {
+ /*
+ * Just a poke to keep us
+ * alive, nothing to do.
+ */
+ } else {
+ warning("BUG: Bogus command %s", buf);
+ }
+ } else {
+ /*
+ * No command from client. Probably it's just
+ * a liveness check or client error. Just
+ * close up.
+ */
+ }
+ close(client_fd);
+ }
+
+ close(fd);
+}
+
+#else
+
+static void loop(int fd, int idle_in_seconds)
+{
+ die(_("index-helper is not supported on this platform"));
+}
+
+#endif
+
+static const char * const usage_text[] = {
+ N_("git index-helper [options]"),
+ NULL
+};
+
+int main(int argc, char **argv)
+{
+ const char *prefix;
+ int idle_in_seconds = 600;
+ int fd;
+ struct strbuf socket_path = STRBUF_INIT;
+ struct option options[] = {
+ OPT_INTEGER(0, "exit-after", &idle_in_seconds,
+ N_("exit if not used after some seconds")),
+ OPT_END()
+ };
+
+ git_extract_argv0_path(argv[0]);
+ git_setup_gettext();
+
+ if (argc == 2 && !strcmp(argv[1], "-h"))
+ usage_with_options(usage_text, options);
+
+ prefix = setup_git_directory();
+ if (parse_options(argc, (const char **)argv, prefix,
+ options, usage_text, 0))
+ die(_("too many arguments"));
+
+ atexit(cleanup);
+ sigchain_push_common(cleanup_on_signal);
+
+ strbuf_git_path(&socket_path, "index-helper.sock");
+
+ fd = unix_stream_listen(socket_path.buf);
+ if (fd < 0)
+ die_errno(_("could not set up index-helper socket"));
+
+ loop(fd, idle_in_seconds);
+
+ close(fd);
+ return 0;
+}
diff --git a/read-cache.c b/read-cache.c
index 3cb0ec3..1a5a03d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -18,6 +18,9 @@
#include "varint.h"
#include "split-index.h"
#include "utf8.h"
+#include "unix-socket.h"
+#include "pkt-line.h"
+#include "sigchain.h"
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
unsigned int options);
@@ -1541,6 +1544,98 @@ static void post_read_index_from(struct index_state *istate)
tweak_untracked_cache(istate);
}
+static int poke_daemon(struct index_state *istate,
+ const struct stat *st, int refresh_cache)
+{
+ int fd;
+ int ret = 0;
+ const char *socket_path;
+
+ /* if this is from index-helper, do not poke itself (recursively) */
+ if (istate->to_shm)
+ return 0;
+
+ socket_path = git_path("index-helper.sock");
+ if (!socket_path)
+ return -1;
+
+ fd = unix_stream_connect(socket_path);
+ sigchain_push(SIGPIPE, SIG_IGN);
+ if (refresh_cache) {
+ packet_write(fd, "refresh");
+ } else {
+ packet_write(fd, "poke");
+ }
+ packet_flush(fd);
+
+ close(fd);
+ sigchain_pop(SIGPIPE);
+ return ret;
+}
+
+static int is_main_index(struct index_state *istate)
+{
+ return istate == &the_index ||
+ (the_index.split_index &&
+ istate == the_index.split_index->base);
+}
+
+/*
+ * Try to open and verify a cached shm index if available. Return 0 if
+ * succeeds (istate->mmap and istate->mmap_size are updated). Return
+ * negative otherwise.
+ */
+static int try_shm(struct index_state *istate)
+{
+ void *new_mmap = NULL;
+ size_t old_size = istate->mmap_size;
+ ssize_t new_size;
+ const unsigned char *sha1;
+ struct stat st;
+ int fd = -1;
+
+ if (!is_main_index(istate) ||
+ old_size <= 20 ||
+ stat(git_path("index-helper.sock"), &st))
+ return -1;
+ if (poke_daemon(istate, &st, 0))
+ return -1;
+ sha1 = (unsigned char *)istate->mmap + old_size - 20;
+
+ fd = open(git_path("shm-index-%s", sha1_to_hex(sha1)), O_RDONLY);
+ if (fd < 0)
+ goto fail;
+
+ if (fstat(fd, &st))
+ goto fail;
+
+ new_size = st.st_size;
+ new_mmap = mmap(NULL, new_size, PROT_READ, MAP_SHARED, fd, 0);
+ if (new_size <= 20 ||
+ hashcmp((unsigned char *)istate->mmap + old_size - 20,
+ (unsigned char *)new_mmap + new_size - 20)) {
+ if (new_mmap)
+ munmap(new_mmap, new_size);
+ goto fail;
+ }
+
+ /* The memory barrier here matches index-helper.c:share_index. */
+ __sync_synchronize();
+
+ munmap(istate->mmap, istate->mmap_size);
+ istate->mmap = new_mmap;
+ istate->mmap_size = new_size;
+ istate->from_shm = 1;
+ close(fd);
+ return 0;
+
+fail:
+ if (fd >= 0)
+ close(fd);
+ poke_daemon(istate, &st, 1);
+ return -1;
+}
+
/* remember to discard_cache() before reading a different cache! */
int do_read_index(struct index_state *istate, const char *path, int must_exist)
{
@@ -1555,6 +1650,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
if (istate->initialized)
return istate->cache_nr;
+ istate->from_shm = 0;
istate->timestamp.sec = 0;
istate->timestamp.nsec = 0;
fd = open(path, O_RDONLY);
@@ -1574,15 +1670,17 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
if (mmap == MAP_FAILED)
die_errno("unable to map index file");
- if (istate->keep_mmap) {
- istate->mmap = mmap;
- istate->mmap_size = mmap_size;
- }
close(fd);
- hdr = mmap;
- if (verify_hdr(hdr, mmap_size) < 0)
+ istate->mmap = mmap;
+ istate->mmap_size = mmap_size;
+ if (try_shm(istate) &&
+ verify_hdr(istate->mmap, istate->mmap_size) < 0)
goto unmap;
+ hdr = mmap = istate->mmap;
+ mmap_size = istate->mmap_size;
+ if (!istate->keep_mmap)
+ istate->mmap = NULL;
hashcpy(istate->sha1, (const unsigned char *)hdr + mmap_size - 20);
istate->version = ntohl(hdr->hdr_version);
@@ -1662,6 +1760,8 @@ int read_index_from(struct index_state *istate, const char *path)
else
split_index->base = xcalloc(1, sizeof(*split_index->base));
split_index->base->keep_mmap = istate->keep_mmap;
+ split_index->base->to_shm = istate->to_shm;
+ split_index->base->from_shm = istate->from_shm;
ret = do_read_index(split_index->base,
git_path("sharedindex.%s",
sha1_to_hex(split_index->base_sha1)), 1);
@@ -1712,6 +1812,8 @@ int discard_index(struct index_state *istate)
discard_split_index(istate);
free_untracked_cache(istate->untracked);
istate->untracked = NULL;
+ istate->from_shm = 0;
+ istate->to_shm = 0;
return 0;
}
@@ -2138,9 +2240,14 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
return ret;
assert((flags & (COMMIT_LOCK | CLOSE_LOCK)) !=
(COMMIT_LOCK | CLOSE_LOCK));
- if (flags & COMMIT_LOCK)
- return commit_locked_index(lock);
- else if (flags & CLOSE_LOCK)
+ if (flags & COMMIT_LOCK) {
+ struct stat st;
+ ret = commit_locked_index(lock);
+ if (!ret && is_main_index(istate) &&
+ !stat(git_path("index-helper.sock"), &st))
+ poke_daemon(istate, &st, 1);
+ return ret;
+ } else if (flags & CLOSE_LOCK)
return close_lock_file(lock);
else
return ret;
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
new file mode 100755
index 0000000..114c112
--- /dev/null
+++ b/t/t7900-index-helper.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+#
+# Copyright (c) 2016, Twitter, Inc
+#
+
+test_description='git-index-helper
+
+Testing git index-helper
+'
+
+. ./test-lib.sh
+
+test -n "$NO_MMAP" && {
+ skip_all='skipping index-helper tests: no mmap'
+ test_done
+}
+
+test_expect_success 'index-helper smoke test' '
+ git index-helper --exit-after 1 &&
+ test_path_is_missing .git/index-helper.sock
+'
+
+test_done
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-09 20:48 ` [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff David Turner
@ 2016-05-10 10:13 ` SZEDER Gábor
2016-05-10 10:31 ` Duy Nguyen
2016-05-10 17:19 ` David Turner
0 siblings, 2 replies; 39+ messages in thread
From: SZEDER Gábor @ 2016-05-10 10:13 UTC (permalink / raw)
To: David Turner; +Cc: SZEDER Gábor, Ramsay Jones, git, pclouds
This patch adds a new plumbing command, which then will show up in
completion after 'git <TAB>'. Could you please squash in this
oneliner to exclude index-helper from porcelain commands in the
completion script?
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 34024754d929..9264ab919a6a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -684,6 +684,7 @@ __git_list_porcelain_commands ()
for-each-ref) : plumbing;;
hash-object) : plumbing;;
http-*) : transport;;
+ index-helper) : plumbing;;
index-pack) : plumbing;;
init-db) : deprecated;;
local-fetch) : plumbing;;
--
2.8.2.356.ge684b1d
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-10 10:13 ` SZEDER Gábor
@ 2016-05-10 10:31 ` Duy Nguyen
2016-05-10 11:12 ` SZEDER Gábor
2016-05-10 17:19 ` David Turner
1 sibling, 1 reply; 39+ messages in thread
From: Duy Nguyen @ 2016-05-10 10:31 UTC (permalink / raw)
To: SZEDER Gábor, David Turner; +Cc: Ramsay Jones, Git Mailing List
This reminds me, we need to update command-list.txt as well. I guess
it belongs to purehelpers? Back to the patch..
On Tue, May 10, 2016 at 5:13 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> This patch adds a new plumbing command, which then will show up in
> completion after 'git <TAB>'. Could you please squash in this
> oneliner to exclude index-helper from porcelain commands in the
> completion script?
I don't think the user should run this command explicitly a lot.
Usually it should be run automatically (with config key
indexhelper.autorun) and die when idle for some time. You just don't
care about it (until performance degrades and you have to
investigate). Occasionally you may have to "git index-helper --kill".
Is there any other reason it should be run often enough to justify
this change?
>
>
> ---
> contrib/completion/git-completion.bash | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 34024754d929..9264ab919a6a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -684,6 +684,7 @@ __git_list_porcelain_commands ()
> for-each-ref) : plumbing;;
> hash-object) : plumbing;;
> http-*) : transport;;
> + index-helper) : plumbing;;
> index-pack) : plumbing;;
> init-db) : deprecated;;
> local-fetch) : plumbing;;
> --
> 2.8.2.356.ge684b1d
>
--
Duy
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-10 10:31 ` Duy Nguyen
@ 2016-05-10 11:12 ` SZEDER Gábor
2016-05-10 11:31 ` Duy Nguyen
0 siblings, 1 reply; 39+ messages in thread
From: SZEDER Gábor @ 2016-05-10 11:12 UTC (permalink / raw)
To: Duy Nguyen; +Cc: David Turner, Ramsay Jones, Git Mailing List
Quoting Duy Nguyen <pclouds@gmail.com>:
> This reminds me, we need to update command-list.txt as well. I guess
> it belongs to purehelpers? Back to the patch..
>
> On Tue, May 10, 2016 at 5:13 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>> This patch adds a new plumbing command, which then will show up in
>> completion after 'git <TAB>'. Could you please squash in this
>> oneliner to exclude index-helper from porcelain commands in the
>> completion script?
>
> I don't think the user should run this command explicitly a lot.
> Usually it should be run automatically (with config key
> indexhelper.autorun) and die when idle for some time. You just don't
> care about it (until performance degrades and you have to
> investigate). Occasionally you may have to "git index-helper --kill".
> Is there any other reason it should be run often enough to justify
> this change?
I think you misunderstood this change, it _excludes_
'index-helper' from the list of porcelain commands.
>> ---
>> contrib/completion/git-completion.bash | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 34024754d929..9264ab919a6a 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -684,6 +684,7 @@ __git_list_porcelain_commands ()
>> for-each-ref) : plumbing;;
>> hash-object) : plumbing;;
>> http-*) : transport;;
>> + index-helper) : plumbing;;
>> index-pack) : plumbing;;
>> init-db) : deprecated;;
>> local-fetch) : plumbing;;
>> --
>> 2.8.2.356.ge684b1d
>>
> --
> Duy
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-10 11:12 ` SZEDER Gábor
@ 2016-05-10 11:31 ` Duy Nguyen
0 siblings, 0 replies; 39+ messages in thread
From: Duy Nguyen @ 2016-05-10 11:31 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: David Turner, Ramsay Jones, Git Mailing List
On Tue, May 10, 2016 at 6:12 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>
> Quoting Duy Nguyen <pclouds@gmail.com>:
>
>> This reminds me, we need to update command-list.txt as well. I guess
>> it belongs to purehelpers? Back to the patch..
>>
>> On Tue, May 10, 2016 at 5:13 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
>>>
>>> This patch adds a new plumbing command, which then will show up in
>>> completion after 'git <TAB>'. Could you please squash in this
>>> oneliner to exclude index-helper from porcelain commands in the
>>> completion script?
>>
>>
>> I don't think the user should run this command explicitly a lot.
>> Usually it should be run automatically (with config key
>> indexhelper.autorun) and die when idle for some time. You just don't
>> care about it (until performance degrades and you have to
>> investigate). Occasionally you may have to "git index-helper --kill".
>> Is there any other reason it should be run often enough to justify
>> this change?
>
>
> I think you misunderstood this change, it _excludes_
> 'index-helper' from the list of porcelain commands.
Oops, right it excludes it.
--
Duy
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff
2016-05-10 10:13 ` SZEDER Gábor
2016-05-10 10:31 ` Duy Nguyen
@ 2016-05-10 17:19 ` David Turner
1 sibling, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-10 17:19 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Ramsay Jones, git, pclouds
Will do, thanks.
On Tue, 2016-05-10 at 12:13 +0200, SZEDER Gábor wrote:
> This patch adds a new plumbing command, which then will show up in
> completion after 'git <TAB>'. Could you please squash in this
> oneliner to exclude index-helper from porcelain commands in the
> completion script?
>
>
> ---
> contrib/completion/git-completion.bash | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 34024754d929..9264ab919a6a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -684,6 +684,7 @@ __git_list_porcelain_commands ()
> for-each-ref) : plumbing;;
> hash-object) : plumbing;;
> http-*) : transport;;
> + index-helper) : plumbing;;
> index-pack) : plumbing;;
> init-db) : deprecated;;
> local-fetch) : plumbing;;
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v9 04/19] index-helper: add --strict
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (2 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 03/19] index-helper: new daemon for caching index and related stuff David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 05/19] index-helper: log warnings David Turner
` (15 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
There are "holes" in the index-helper approach because the shared
memory is not verified again by git. If $USER is compromised, shared
memory could be modified. But anyone who could do this could already
modify $GIT_DIR/index. A more realistic risk is some bugs in
index-helper that produce corrupt shared memory. --strict is added to
avoid that.
Strictly speaking there's still a very small gap where corrupt shared
memory could still be read by git: after we write the trailing SHA-1 in
the shared memory (thus signaling "this shm is ready") and before
verify_shm() detects an error.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 9 +++++++
cache.h | 1 +
index-helper.c | 48 ++++++++++++++++++++++++++++++++++++++
read-cache.c | 9 ++++---
4 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index f892184..1f92c89 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -25,6 +25,15 @@ OPTIONS
Exit if the cached index is not accessed for `<n>`
seconds. Specify 0 to wait forever. Default is 600.
+--strict::
+--no-strict::
+ Strict mode makes index-helper verify the shared memory after
+ it's created. If the result does not match what's read from
+ $GIT_DIR/index, the shared memory is destroyed. This makes
+ index-helper take more than double the amount of time required
+ for reading an index, but because it will happen in the
+ background, it's not noticable. `--strict` is enabled by default.
+
NOTES
-----
diff --git a/cache.h b/cache.h
index 2d7af6f..6cb0d02 100644
--- a/cache.h
+++ b/cache.h
@@ -345,6 +345,7 @@ struct index_state {
* on it.
*/
to_shm : 1,
+ always_verify_trailing_sha1 : 1,
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
diff --git a/index-helper.c b/index-helper.c
index b9443f4..bc7e110 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -17,6 +17,7 @@ struct shm {
static struct shm shm_index;
static struct shm shm_base_index;
+static int to_verify = 1;
static void release_index_shm(struct shm *is)
{
@@ -114,11 +115,56 @@ static void share_index(struct index_state *istate, struct shm *is)
hashcpy((unsigned char *)new_mmap + istate->mmap_size - 20, is->sha1);
}
+static int verify_shm(void)
+{
+ int i;
+ struct index_state istate;
+ memset(&istate, 0, sizeof(istate));
+ istate.always_verify_trailing_sha1 = 1;
+ istate.to_shm = 1;
+ i = read_index(&istate);
+ if (i != the_index.cache_nr)
+ goto done;
+ for (; i < the_index.cache_nr; i++) {
+ struct cache_entry *base, *ce;
+ /* namelen is checked separately */
+ const unsigned int ondisk_flags =
+ CE_STAGEMASK | CE_VALID | CE_EXTENDED_FLAGS;
+ unsigned int ce_flags, base_flags, ret;
+ base = the_index.cache[i];
+ ce = istate.cache[i];
+ if (ce->ce_namelen != base->ce_namelen ||
+ strcmp(ce->name, base->name)) {
+ warning("mismatch at entry %d", i);
+ break;
+ }
+ ce_flags = ce->ce_flags;
+ base_flags = base->ce_flags;
+ /* only on-disk flags matter */
+ ce->ce_flags &= ondisk_flags;
+ base->ce_flags &= ondisk_flags;
+ ret = memcmp(&ce->ce_stat_data, &base->ce_stat_data,
+ offsetof(struct cache_entry, name) -
+ offsetof(struct cache_entry, ce_stat_data));
+ ce->ce_flags = ce_flags;
+ base->ce_flags = base_flags;
+ if (ret) {
+ warning("mismatch at entry %d", i);
+ break;
+ }
+ }
+done:
+ discard_index(&istate);
+ return i == the_index.cache_nr;
+}
+
static void share_the_index(void)
{
if (the_index.split_index && the_index.split_index->base)
share_index(the_index.split_index->base, &shm_base_index);
share_index(&the_index, &shm_index);
+ if (to_verify && !verify_shm())
+ cleanup_shm();
discard_index(&the_index);
}
@@ -247,6 +293,8 @@ int main(int argc, char **argv)
struct option options[] = {
OPT_INTEGER(0, "exit-after", &idle_in_seconds,
N_("exit if not used after some seconds")),
+ OPT_BOOL(0, "strict", &to_verify,
+ "verify shared memory after creating"),
OPT_END()
};
diff --git a/read-cache.c b/read-cache.c
index 1a5a03d..d7849d6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1674,9 +1674,12 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
istate->mmap = mmap;
istate->mmap_size = mmap_size;
- if (try_shm(istate) &&
- verify_hdr(istate->mmap, istate->mmap_size) < 0)
- goto unmap;
+ if (try_shm(istate)) {
+ if (verify_hdr(istate->mmap, istate->mmap_size) < 0)
+ goto unmap;
+ } else if (istate->always_verify_trailing_sha1 &&
+ verify_hdr(istate->mmap, istate->mmap_size) < 0)
+ goto unmap;
hdr = mmap = istate->mmap;
mmap_size = istate->mmap_size;
if (!istate->keep_mmap)
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 05/19] index-helper: log warnings
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (3 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 04/19] index-helper: add --strict David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 06/19] daemonize(): set a flag before exiting the main process David Turner
` (14 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Instead of writing warnings to stderr, write them to a log. Later, we'll
probably be daemonized, so writing to stderr will be pointless.
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 3 +++
index-helper.c | 32 ++++++++++++++++++++++++++++----
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index 1f92c89..9c2f5eb 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -54,6 +54,9 @@ command. The following commands are used to control the daemon:
All commands and replies are terminated by a NUL byte.
+In the event of an error, messages may be written to
+$GIT_DIR/index-helper.log.
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/index-helper.c b/index-helper.c
index bc7e110..21fb431 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -19,6 +19,29 @@ static struct shm shm_index;
static struct shm shm_base_index;
static int to_verify = 1;
+static void log_warning(const char *warning, ...)
+{
+ va_list ap;
+ FILE *fp;
+
+ fp = fopen(git_path("index-helper.log"), "a");
+ if (!fp)
+ /*
+ * Probably nobody will see this, but it's the best
+ * we can do.
+ */
+ die("Failed to open log for warnings");
+
+ fprintf(fp, "%"PRIuMAX"\t", (uintmax_t)time(NULL));
+
+ va_start(ap, warning);
+ vfprintf(fp, warning, ap);
+ va_end(ap);
+
+ fputc('\n', fp);
+ fclose(fp);
+}
+
static void release_index_shm(struct shm *is)
{
if (!is->shm)
@@ -93,7 +116,8 @@ static void share_index(struct index_state *istate, struct shm *is)
if (shared_mmap_create(istate->mmap_size, &new_mmap,
git_path("shm-index-%s",
sha1_to_hex(istate->sha1))) < 0) {
- die("Failed to create shm-index file");
+ log_warning("Failed to create shm-index file");
+ exit(1);
}
@@ -135,7 +159,7 @@ static int verify_shm(void)
ce = istate.cache[i];
if (ce->ce_namelen != base->ce_namelen ||
strcmp(ce->name, base->name)) {
- warning("mismatch at entry %d", i);
+ log_warning("mismatch at entry %d", i);
break;
}
ce_flags = ce->ce_flags;
@@ -149,7 +173,7 @@ static int verify_shm(void)
ce->ce_flags = ce_flags;
base->ce_flags = base_flags;
if (ret) {
- warning("mismatch at entry %d", i);
+ log_warning("mismatch at entry %d", i);
break;
}
}
@@ -255,7 +279,7 @@ static void loop(int fd, int idle_in_seconds)
* alive, nothing to do.
*/
} else {
- warning("BUG: Bogus command %s", buf);
+ log_warning("BUG: Bogus command %s", buf);
}
} else {
/*
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 06/19] daemonize(): set a flag before exiting the main process
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (4 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 05/19] index-helper: log warnings David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 07/19] index-helper: add --detach David Turner
` (13 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
This allows signal handlers and atexit functions to realize this
situation and not clean up.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
builtin/gc.c | 2 +-
cache.h | 2 +-
daemon.c | 2 +-
setup.c | 4 +++-
4 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index c583aad..37180de 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -385,7 +385,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
* failure to daemonize is ok, we'll continue
* in foreground
*/
- daemonized = !daemonize();
+ daemonized = !daemonize(NULL);
}
} else
add_repack_all_option();
diff --git a/cache.h b/cache.h
index 6cb0d02..4c1529a 100644
--- a/cache.h
+++ b/cache.h
@@ -539,7 +539,7 @@ extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);
extern int init_db(const char *template_dir, unsigned int flags);
extern void sanitize_stdfds(void);
-extern int daemonize(void);
+extern int daemonize(int *);
#define alloc_nr(x) (((x)+16)*3/2)
diff --git a/daemon.c b/daemon.c
index 8d45c33..a5cf954 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1365,7 +1365,7 @@ int main(int argc, char **argv)
return execute();
if (detach) {
- if (daemonize())
+ if (daemonize(NULL))
die("--detach not supported on this platform");
} else
sanitize_stdfds();
diff --git a/setup.c b/setup.c
index de1a2a7..9adf13f 100644
--- a/setup.c
+++ b/setup.c
@@ -1017,7 +1017,7 @@ void sanitize_stdfds(void)
close(fd);
}
-int daemonize(void)
+int daemonize(int *daemonized)
{
#ifdef NO_POSIX_GOODIES
errno = ENOSYS;
@@ -1029,6 +1029,8 @@ int daemonize(void)
case -1:
die_errno("fork failed");
default:
+ if (daemonized)
+ *daemonized = 1;
exit(0);
}
if (setsid() == -1)
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 07/19] index-helper: add --detach
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (5 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 06/19] daemonize(): set a flag before exiting the main process David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 08/19] read-cache: add watchman 'WAMA' extension David Turner
` (12 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
We detach after creating and opening the socket, because otherwise
we might return control to the shell before index-helper is ready to
accept commands. This might lead to flaky tests.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 3 +++
index-helper.c | 15 +++++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index 9c2f5eb..e144752 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -34,6 +34,9 @@ OPTIONS
for reading an index, but because it will happen in the
background, it's not noticable. `--strict` is enabled by default.
+--detach::
+ Detach from the shell.
+
NOTES
-----
diff --git a/index-helper.c b/index-helper.c
index 21fb431..7df7a97 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -17,7 +17,7 @@ struct shm {
static struct shm shm_index;
static struct shm shm_base_index;
-static int to_verify = 1;
+static int daemonized, to_verify = 1;
static void log_warning(const char *warning, ...)
{
@@ -59,6 +59,8 @@ static void cleanup_shm(void)
static void cleanup(void)
{
+ if (daemonized)
+ return;
unlink(git_path("index-helper.sock"));
cleanup_shm();
}
@@ -311,7 +313,7 @@ static const char * const usage_text[] = {
int main(int argc, char **argv)
{
const char *prefix;
- int idle_in_seconds = 600;
+ int idle_in_seconds = 600, detach = 0;
int fd;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
@@ -319,6 +321,7 @@ int main(int argc, char **argv)
N_("exit if not used after some seconds")),
OPT_BOOL(0, "strict", &to_verify,
"verify shared memory after creating"),
+ OPT_BOOL(0, "detach", &detach, "detach the process"),
OPT_END()
};
@@ -342,6 +345,14 @@ int main(int argc, char **argv)
if (fd < 0)
die_errno(_("could not set up index-helper socket"));
+ if (!freopen("/dev/null", "w", stderr))
+ die_errno("failed to redirect stderr to /dev/null");
+
+ if (!freopen("/dev/null", "w", stdout))
+ die_errno("failed to redirect stdout to /dev/null");
+
+ if (detach && daemonize(&daemonized))
+ die_errno(_("unable to detach"));
loop(fd, idle_in_seconds);
close(fd);
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 08/19] read-cache: add watchman 'WAMA' extension
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (6 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 07/19] index-helper: add --detach David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 09/19] watchman: support watchman to reduce index refresh cost David Turner
` (11 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
The extension contains a bitmap, one bit for each entry in the
index. If the n-th bit is zero, the n-th entry is considered
unchanged, we can ce_mark_uptodate() it without refreshing. If the bit
is non-zero and we found out the corresponding file is clean after
refresh, we can clear the bit.
In addition, there's a list of directories in the untracked-cache
to invalidate (because they have new or modified entries).
The 'skipping refresh' bit is not in this patch yet as we would need
watchman. More details in later patches.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/technical/index-format.txt | 22 ++++++
cache.h | 4 ++
dir.h | 3 +
read-cache.c | 117 ++++++++++++++++++++++++++++++-
4 files changed, 144 insertions(+), 2 deletions(-)
diff --git a/Documentation/technical/index-format.txt b/Documentation/technical/index-format.txt
index ade0b0c..86ed3a6 100644
--- a/Documentation/technical/index-format.txt
+++ b/Documentation/technical/index-format.txt
@@ -295,3 +295,25 @@ The remaining data of each directory block is grouped by type:
in the previous ewah bitmap.
- One NUL.
+
+== Watchman cache
+
+ The watchman cache tracks files for which watchman has told us about
+ changes. The signature for this extension is { 'W', 'A', 'M', 'A' }.
+
+ The extension starts with
+
+ - A NUL-terminated string: the watchman vector clock at the last
+ time we heard from watchman.
+
+ - 32-bit bitmap size: the size of the CE_WATCHMAN_DIRTY bitmap
+
+ - 32-bit untracked cache entry count: the number of dirty untracked
+ cache entries
+
+ - An ewah bitmap, the n-th bit indicates whether the n-th index entry
+ is CE_WATCHMAN_DIRTY.
+
+ - a list of N NUL-terminated strings. Each is a directory that should
+ be marked dirty in the untracked cache because watchman has told us
+ about an update to a file in it.
diff --git a/cache.h b/cache.h
index 4c1529a..f10992d 100644
--- a/cache.h
+++ b/cache.h
@@ -182,6 +182,8 @@ struct cache_entry {
#define CE_VALID (0x8000)
#define CE_STAGESHIFT 12
+#define CE_WATCHMAN_DIRTY (0x0001)
+
/*
* Range 0xFFFF0FFF in ce_flags is divided into
* two parts: in-memory flags and on-disk ones.
@@ -320,6 +322,7 @@ static inline unsigned int canon_mode(unsigned int mode)
#define CACHE_TREE_CHANGED (1 << 5)
#define SPLIT_INDEX_ORDERED (1 << 6)
#define UNTRACKED_CHANGED (1 << 7)
+#define WATCHMAN_CHANGED (1 << 8)
struct split_index;
struct untracked_cache;
@@ -353,6 +356,7 @@ struct index_state {
struct untracked_cache *untracked;
void *mmap;
size_t mmap_size;
+ char *last_update;
};
extern struct index_state the_index;
diff --git a/dir.h b/dir.h
index 3ec3fb0..3d540de 100644
--- a/dir.h
+++ b/dir.h
@@ -142,6 +142,9 @@ struct untracked_cache {
int gitignore_invalidated;
int dir_invalidated;
int dir_opened;
+ /* watchman invalidation data */
+ unsigned int use_watchman : 1;
+ struct string_list invalid_untracked;
};
struct dir_struct {
diff --git a/read-cache.c b/read-cache.c
index d7849d6..9399a81 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -21,6 +21,7 @@
#include "unix-socket.h"
#include "pkt-line.h"
#include "sigchain.h"
+#include "ewah/ewok.h"
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
unsigned int options);
@@ -43,11 +44,13 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
#define CACHE_EXT_RESOLVE_UNDO 0x52455543 /* "REUC" */
#define CACHE_EXT_LINK 0x6c696e6b /* "link" */
#define CACHE_EXT_UNTRACKED 0x554E5452 /* "UNTR" */
+#define CACHE_EXT_WATCHMAN 0x57414D41 /* "WAMA" */
/* changes that can be kept in $GIT_DIR/index (basically all extensions) */
#define EXTMASK (RESOLVE_UNDO_CHANGED | CACHE_TREE_CHANGED | \
CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \
- SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED)
+ SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | \
+ WATCHMAN_CHANGED)
struct index_state the_index;
static const char *alternate_index_output;
@@ -1222,8 +1225,13 @@ int refresh_index(struct index_state *istate, unsigned int flags,
continue;
new = refresh_cache_ent(istate, ce, options, &cache_errno, &changed);
- if (new == ce)
+ if (new == ce) {
+ if (ce->ce_flags & CE_WATCHMAN_DIRTY) {
+ ce->ce_flags &= ~CE_WATCHMAN_DIRTY;
+ istate->cache_changed |= WATCHMAN_CHANGED;
+ }
continue;
+ }
if (!new) {
const char *fmt;
@@ -1367,6 +1375,94 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
return 0;
}
+static void mark_no_watchman(size_t pos, void *data)
+{
+ struct index_state *istate = data;
+ assert(pos < istate->cache_nr);
+ istate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;
+}
+
+static int read_watchman_ext(struct index_state *istate, const void *data,
+ unsigned long sz)
+{
+ struct ewah_bitmap *bitmap;
+ int ret, len;
+ uint32_t bitmap_size;
+ uint32_t untracked_nr;
+
+ if (memchr(data, 0, sz) == NULL)
+ return error("invalid extension");
+
+ len = strlen(data) + 1;
+ memcpy(&bitmap_size, (const char *)data + len, 4);
+ memcpy(&untracked_nr, (const char *)data + len + 4, 4);
+ untracked_nr = ntohl(untracked_nr);
+ bitmap_size = ntohl(bitmap_size);
+
+ bitmap = ewah_new();
+ ret = ewah_read_mmap(bitmap, (const char *)data + len + 8, bitmap_size);
+ if (ret != bitmap_size) {
+ ewah_free(bitmap);
+ return error("failed to parse ewah bitmap reading watchman index extension");
+ }
+ istate->last_update = xstrdup(data);
+ ewah_each_bit(bitmap, mark_no_watchman, istate);
+ ewah_free(bitmap);
+
+ /*
+ * TODO: update the untracked cache from the untracked data in this
+ * extension.
+ */
+ return 0;
+}
+
+static int untracked_entry_append(struct string_list_item *item, void *sbvoid)
+{
+ struct strbuf *sb = sbvoid;
+
+ strbuf_addstr(sb, item->string);
+ strbuf_addch(sb, 0);
+ return 0;
+}
+
+void write_watchman_ext(struct strbuf *sb, struct index_state *istate)
+{
+ struct ewah_bitmap *bitmap;
+ int i;
+ int ewah_start;
+ int ewah_size = 0;
+ int fixup = 0;
+
+ strbuf_add(sb, istate->last_update, strlen(istate->last_update) + 1);
+ fixup = sb->len;
+ strbuf_add(sb, &ewah_size, 4); /* we'll fix this up later */
+ if (istate->untracked) {
+ uint32_t nr = istate->untracked->invalid_untracked.nr;
+ nr = htonl(nr);
+ strbuf_add(sb, &nr, 4);
+ } else {
+ /* zero */
+ strbuf_add(sb, &ewah_size, 4);
+ }
+
+ ewah_start = sb->len;
+ bitmap = ewah_new();
+ for (i = 0; i < istate->cache_nr; i++)
+ if (istate->cache[i]->ce_flags & CE_WATCHMAN_DIRTY)
+ ewah_set(bitmap, i);
+ ewah_serialize_strbuf(bitmap, sb);
+ ewah_free(bitmap);
+
+ /* fix up size field */
+ ewah_size = sb->len - ewah_start;
+ ewah_size = htonl(ewah_size);
+ memcpy(sb->buf + fixup, &ewah_size, 4);
+
+ if (istate->untracked)
+ for_each_string_list(&istate->untracked->invalid_untracked,
+ untracked_entry_append, sb);
+}
+
static int read_index_extension(struct index_state *istate,
const char *ext, void *data, unsigned long sz)
{
@@ -1384,6 +1480,11 @@ static int read_index_extension(struct index_state *istate,
case CACHE_EXT_UNTRACKED:
istate->untracked = read_untracked_extension(data, sz);
break;
+
+ case CACHE_EXT_WATCHMAN:
+ read_watchman_ext(istate, data, sz);
+ break;
+
default:
if (*ext < 'A' || 'Z' < *ext)
return error("index uses %.4s extension, which we do not understand",
@@ -1817,6 +1918,8 @@ int discard_index(struct index_state *istate)
istate->untracked = NULL;
istate->from_shm = 0;
istate->to_shm = 0;
+ free(istate->last_update);
+ istate->last_update = NULL;
return 0;
}
@@ -2214,6 +2317,16 @@ static int do_write_index(struct index_state *istate, int newfd,
if (err)
return -1;
}
+ if (!strip_extensions && istate->last_update) {
+ struct strbuf sb = STRBUF_INIT;
+
+ write_watchman_ext(&sb, istate);
+ err = write_index_ext_header(&c, newfd, CACHE_EXT_WATCHMAN, sb.len) < 0
+ || ce_write(&c, newfd, sb.buf, sb.len) < 0;
+ strbuf_release(&sb);
+ if (err)
+ return -1;
+ }
if (ce_flush(&c, newfd, istate->sha1) || fstat(newfd, &st))
return -1;
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 09/19] watchman: support watchman to reduce index refresh cost
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (7 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 08/19] read-cache: add watchman 'WAMA' extension David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 10/19] index-helper: use watchman to avoid refreshing index with lstat() David Turner
` (10 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
The previous patch has the logic to clear bits in 'WAMA' bitmap. This
patch has logic to set bits as told by watchman. The missing bit,
_using_ these bits, are not here yet.
A lot of this code is written by David Turner originally, mostly from
[1]. I'm just copying and polishing it a bit.
[1] http://article.gmane.org/gmane.comp.version-control.git/248006
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Makefile | 12 +++++
cache.h | 1 +
config.c | 5 ++
configure.ac | 8 ++++
environment.c | 3 ++
watchman-support.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++
watchman-support.h | 7 +++
7 files changed, 171 insertions(+)
create mode 100644 watchman-support.c
create mode 100644 watchman-support.h
diff --git a/Makefile b/Makefile
index c8be0e7..65ab0f4 100644
--- a/Makefile
+++ b/Makefile
@@ -451,6 +451,7 @@ MSGFMT = msgfmt
CURL_CONFIG = curl-config
PTHREAD_LIBS = -lpthread
PTHREAD_CFLAGS =
+WATCHMAN_LIBS =
GCOV = gcov
export TCL_PATH TCLTK_PATH
@@ -1416,6 +1417,13 @@ else
LIB_OBJS += thread-utils.o
endif
+ifdef USE_WATCHMAN
+ LIB_H += watchman-support.h
+ LIB_OBJS += watchman-support.o
+ WATCHMAN_LIBS = -lwatchman
+ BASIC_CFLAGS += -DUSE_WATCHMAN
+endif
+
ifdef HAVE_PATHS_H
BASIC_CFLAGS += -DHAVE_PATHS_H
endif
@@ -2025,6 +2033,9 @@ git-remote-testsvn$X: remote-testsvn.o GIT-LDFLAGS $(GITLIBS) $(VCSSVN_LIB)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) \
$(VCSSVN_LIB)
+git-index-helper$X: index-helper.o GIT-LDFLAGS $(GITLIBS)
+ $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) $(WATCHMAN_LIBS)
+
$(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
$(QUIET_LNCP)$(RM) $@ && \
ln $< $@ 2>/dev/null || \
@@ -2164,6 +2175,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
@echo NO_MMAP=\''$(subst ','\'',$(subst ','\'',$(NO_MMAP)))'\' >>$@+
+ @echo USE_WATCHMAN=\''$(subst ','\'',$(subst ','\'',$(USE_WATCHMAN)))'\' >>$@+
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+
endif
diff --git a/cache.h b/cache.h
index f10992d..452aea2 100644
--- a/cache.h
+++ b/cache.h
@@ -696,6 +696,7 @@ extern char *git_replace_ref_base;
extern int fsync_object_files;
extern int core_preload_index;
+extern int core_watchman_sync_timeout;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
extern int protect_hfs;
diff --git a/config.c b/config.c
index 9ba40bc..e6dc141 100644
--- a/config.c
+++ b/config.c
@@ -882,6 +882,11 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.watchmansynctimeout")) {
+ core_watchman_sync_timeout = git_config_int(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "core.createobject")) {
if (!strcmp(value, "rename"))
object_creation_mode = OBJECT_CREATION_USES_RENAMES;
diff --git a/configure.ac b/configure.ac
index 0cd9f46..334d63b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1099,6 +1099,14 @@ AC_COMPILE_IFELSE([BSD_SYSCTL_SRC],
HAVE_BSD_SYSCTL=])
GIT_CONF_SUBST([HAVE_BSD_SYSCTL])
+#
+# Check for watchman client library
+
+AC_CHECK_LIB([watchman], [watchman_connect],
+ [USE_WATCHMAN=YesPlease],
+ [USE_WATCHMAN=])
+GIT_CONF_SUBST([USE_WATCHMAN])
+
## Other checks.
# Define USE_PIC if you need the main git objects to be built with -fPIC
# in order to build and link perl/Git.so. x86-64 seems to need this.
diff --git a/environment.c b/environment.c
index 6dec9d0..35e03c7 100644
--- a/environment.c
+++ b/environment.c
@@ -94,6 +94,9 @@ int core_preload_index = 1;
*/
int ignore_untracked_cache_config;
+int core_watchman_sync_timeout = 300;
+
+
/* This is set by setup_git_dir_gently() and/or git_default_config() */
char *git_work_tree_cfg;
static char *work_tree;
diff --git a/watchman-support.c b/watchman-support.c
new file mode 100644
index 0000000..dc8cd51
--- /dev/null
+++ b/watchman-support.c
@@ -0,0 +1,135 @@
+#include "cache.h"
+#include "watchman-support.h"
+#include "strbuf.h"
+#include "dir.h"
+#include <watchman.h>
+
+static struct watchman_query *make_query(const char *last_update)
+{
+ struct watchman_query *query = watchman_query();
+ watchman_query_set_fields(query, WATCHMAN_FIELD_NAME |
+ WATCHMAN_FIELD_EXISTS |
+ WATCHMAN_FIELD_NEWER);
+ watchman_query_set_empty_on_fresh(query, 1);
+ query->sync_timeout = core_watchman_sync_timeout;
+ if (*last_update)
+ watchman_query_set_since_oclock(query, last_update);
+ return query;
+}
+
+static struct watchman_query_result *query_watchman(
+ struct index_state *istate, struct watchman_connection *connection,
+ const char *fs_path, const char *last_update)
+{
+ struct watchman_error wm_error;
+ struct watchman_query *query;
+ struct watchman_expression *expr;
+ struct watchman_query_result *result;
+
+ query = make_query(last_update);
+ expr = watchman_true_expression();
+ result = watchman_do_query(connection, fs_path, query, expr, &wm_error);
+ watchman_free_query(query);
+ watchman_free_expression(expr);
+
+ if (!result)
+ warning("Watchman query error: %s (at %s)",
+ wm_error.message,
+ *last_update ? last_update : "the beginning");
+
+ return result;
+}
+
+static void update_index(struct index_state *istate,
+ struct watchman_query_result *result)
+{
+ int i;
+
+ if (result->is_fresh_instance) {
+ /* let refresh clear them later */
+ for (i = 0; i < istate->cache_nr; i++)
+ istate->cache[i]->ce_flags |= CE_WATCHMAN_DIRTY;
+ goto done;
+ }
+
+ for (i = 0; i < result->nr; i++) {
+ struct watchman_stat *wm = result->stats + i;
+ int pos;
+
+ if (S_ISDIR(wm->mode) ||
+ !strncmp(wm->name, ".git/", 5) ||
+ strstr(wm->name, "/.git/"))
+ continue;
+
+ pos = index_name_pos(istate, wm->name, strlen(wm->name));
+ if (pos < 0) {
+ if (istate->untracked) {
+ char *name = xstrdup(wm->name);
+ char *dname = dirname(name);
+
+ /*
+ * dirname() returns '.' for the root,
+ * but we call it ''.
+ */
+ if (dname[0] == '.' && dname[1] == 0)
+ string_list_append(&istate->untracked->invalid_untracked, "");
+ else
+ string_list_append(&istate->untracked->invalid_untracked,
+ dname);
+ free(name);
+ }
+ continue;
+ }
+ /* FIXME: ignore staged entries and gitlinks too? */
+
+ istate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;
+ }
+
+done:
+ free(istate->last_update);
+ istate->last_update = xstrdup(result->clock);
+ istate->cache_changed |= WATCHMAN_CHANGED;
+ if (istate->untracked)
+ string_list_remove_duplicates(&istate->untracked->invalid_untracked, 0);
+}
+
+int check_watchman(struct index_state *istate)
+{
+ struct watchman_error wm_error;
+ struct watchman_connection *connection;
+ struct watchman_query_result *result;
+ const char *fs_path;
+ struct timeval timeout;
+ /*
+ * Convert core_watchman_sync_timeout, in milliseconds, to
+ * struct timeval, in seconds and microseconds.
+ */
+
+ fs_path = get_git_work_tree();
+ if (!fs_path)
+ return -1;
+
+ timeout.tv_sec = core_watchman_sync_timeout / 1000;
+ timeout.tv_usec = (core_watchman_sync_timeout % 1000) * 1000;
+ connection = watchman_connect(timeout, &wm_error);
+
+ if (!connection) {
+ warning("Watchman watch error: %s", wm_error.message);
+ return -1;
+ }
+
+ if (watchman_watch(connection, fs_path, &wm_error)) {
+ warning("Watchman watch error: %s", wm_error.message);
+ watchman_connection_close(connection);
+ return -1;
+ }
+
+
+ result = query_watchman(istate, connection, fs_path, istate->last_update);
+ watchman_connection_close(connection);
+ if (!result)
+ return -1;
+ update_index(istate, result);
+ watchman_free_query_result(result);
+ return 0;
+}
diff --git a/watchman-support.h b/watchman-support.h
new file mode 100644
index 0000000..ee1ef2c
--- /dev/null
+++ b/watchman-support.h
@@ -0,0 +1,7 @@
+#ifndef WATCHMAN_SUPPORT_H
+#define WATCHMAN_SUPPORT_H
+
+struct index_state;
+int check_watchman(struct index_state *index);
+
+#endif /* WATCHMAN_SUPPORT_H */
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 10/19] index-helper: use watchman to avoid refreshing index with lstat()
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (8 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 09/19] watchman: support watchman to reduce index refresh cost David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 11/19] update-index: enable/disable watchman support David Turner
` (9 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Watchman is hidden behind index-helper. Before git tries to read the
index from shm, it notifies index-helper through the socket and waits
for index-helper to prepare a file for sharing memory (with
MAP_SHARED). index-helper then contacts watchman, updates 'WAMA'
extension and put it in a separate file and wakes git up with a reply
to git's socket.
Git uses this extension to not lstat unchanged entries. Git only
trusts the 'WAMA' extension when it's received from the separate file,
not from disk. Unmarked entries are "clean". Marked entries are dirty
from watchman point of view. If it finds out some entries are
'watchman-dirty', but are really unchanged (e.g. the file was changed,
then reverted back), then Git will clear the marking in 'WAMA' before
writing it down.
Hiding watchman behind index-helper means you need both daemons. You
can't run watchman alone. Not so good. But on the other hand, 'git'
binary is not linked to watchman/json libraries, which is good for
packaging. Core git package will run fine without watchman-related
packages. If they need watchman, they can install git-index-helper and
dependencies.
This also lets us trust anything in the untracked cache that we haven't
marked invalid, saving those stat() calls.
Another reason for tying watchman to index-helper is, when used with
untracked cache, we need to keep track of $GIT_WORK_TREE file
listing. That kind of list can be kept in index-helper.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 6 +
cache.h | 2 +
dir.c | 23 +++-
dir.h | 3 +
index-helper.c | 107 ++++++++++++++--
read-cache.c | 243 ++++++++++++++++++++++++++++++++++---
6 files changed, 357 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index e144752..cce00cb 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -55,6 +55,12 @@ command. The following commands are used to control the daemon:
Let the daemon know the index is to be read. It keeps the
daemon alive longer, unless `--exit-after=0` is used.
+"poke <path>":
+ Like "poke", but replies with "OK". If the index has the
+ watchman extension, index-helper queries watchman, then
+ prepares a shared memory object with the watchman index
+ extension before replying.
+
All commands and replies are terminated by a NUL byte.
In the event of an error, messages may be written to
diff --git a/cache.h b/cache.h
index 452aea2..633e1dd 100644
--- a/cache.h
+++ b/cache.h
@@ -567,6 +567,7 @@ extern int daemonize(int *);
/* Initialize and use the cache information */
struct lock_file;
+extern int verify_index(const struct index_state *);
extern int read_index(struct index_state *);
extern int read_index_preload(struct index_state *, const struct pathspec *pathspec);
extern int do_read_index(struct index_state *istate, const char *path,
@@ -574,6 +575,7 @@ extern int do_read_index(struct index_state *istate, const char *path,
extern int read_index_from(struct index_state *, const char *path);
extern int is_index_unborn(struct index_state *);
extern int read_index_unmerged(struct index_state *);
+extern void write_watchman_ext(struct strbuf *sb, struct index_state *istate);
#define COMMIT_LOCK (1 << 0)
#define CLOSE_LOCK (1 << 1)
extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
diff --git a/dir.c b/dir.c
index 69e0be6..5058b29 100644
--- a/dir.c
+++ b/dir.c
@@ -597,9 +597,9 @@ static void trim_trailing_spaces(char *buf)
*
* If "name" has the trailing slash, it'll be excluded in the search.
*/
-static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
- struct untracked_cache_dir *dir,
- const char *name, int len)
+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
+ struct untracked_cache_dir *dir,
+ const char *name, int len)
{
int first, last;
struct untracked_cache_dir *d;
@@ -1726,6 +1726,17 @@ static int valid_cached_dir(struct dir_struct *dir,
if (!untracked)
return 0;
+ if (dir->untracked->use_watchman) {
+ /*
+ * With watchman, we can trust the untracked cache's
+ * valid field.
+ */
+ if (untracked->valid)
+ goto skip_stat;
+ else
+ invalidate_directory(dir->untracked, untracked);
+ }
+
if (stat(path->len ? path->buf : ".", &st)) {
invalidate_directory(dir->untracked, untracked);
memset(&untracked->stat_data, 0, sizeof(untracked->stat_data));
@@ -1739,6 +1750,7 @@ static int valid_cached_dir(struct dir_struct *dir,
return 0;
}
+skip_stat:
if (untracked->check_only != !!check_only) {
invalidate_directory(dir->untracked, untracked);
return 0;
@@ -2625,8 +2637,10 @@ static void free_untracked(struct untracked_cache_dir *ucd)
void free_untracked_cache(struct untracked_cache *uc)
{
- if (uc)
+ if (uc) {
free_untracked(uc->root);
+ string_list_clear(&uc->invalid_untracked, 0);
+ }
free(uc);
}
@@ -2775,6 +2789,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
return NULL;
uc = xcalloc(1, sizeof(*uc));
+ string_list_init(&uc->invalid_untracked, 1);
strbuf_init(&uc->ident, ident_len);
strbuf_add(&uc->ident, ident, ident_len);
load_sha1_stat(&uc->ss_info_exclude, &ouc->info_exclude_stat,
diff --git a/dir.h b/dir.h
index 3d540de..8fd3f9e 100644
--- a/dir.h
+++ b/dir.h
@@ -315,4 +315,7 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
void add_untracked_cache(struct index_state *istate);
void remove_untracked_cache(struct index_state *istate);
+struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
+ struct untracked_cache_dir *dir,
+ const char *name, int len);
#endif
diff --git a/index-helper.c b/index-helper.c
index 7df7a97..71c4f48 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -8,15 +8,18 @@
#include "cache.h"
#include "unix-socket.h"
#include "pkt-line.h"
+#include "watchman-support.h"
struct shm {
unsigned char sha1[20];
void *shm;
size_t size;
+ pid_t pid;
};
static struct shm shm_index;
static struct shm shm_base_index;
+static struct shm shm_watchman;
static int daemonized, to_verify = 1;
static void log_warning(const char *warning, ...)
@@ -51,10 +54,21 @@ static void release_index_shm(struct shm *is)
is->shm = NULL;
}
+static void release_watchman_shm(struct shm *is)
+{
+ if (!is->shm)
+ return;
+ munmap(is->shm, is->size);
+ unlink(git_path("shm-watchman-%s-%" PRIuMAX,
+ sha1_to_hex(is->sha1), (uintmax_t)is->pid));
+ is->shm = NULL;
+}
+
static void cleanup_shm(void)
{
release_index_shm(&shm_index);
release_index_shm(&shm_base_index);
+ release_watchman_shm(&shm_watchman);
}
static void cleanup(void)
@@ -189,9 +203,10 @@ static void share_the_index(void)
if (the_index.split_index && the_index.split_index->base)
share_index(the_index.split_index->base, &shm_base_index);
share_index(&the_index, &shm_index);
- if (to_verify && !verify_shm())
+ if (to_verify && !verify_shm()) {
cleanup_shm();
- discard_index(&the_index);
+ discard_index(&the_index);
+ }
}
static void set_socket_blocking_flag(int fd, int make_nonblocking)
@@ -224,6 +239,80 @@ static void refresh(void)
#ifndef NO_MMAP
+#ifdef USE_WATCHMAN
+static void share_watchman(struct index_state *istate,
+ struct shm *is, pid_t pid)
+{
+ struct strbuf sb = STRBUF_INIT;
+ void *shm;
+
+ write_watchman_ext(&sb, istate);
+ if (!shared_mmap_create(sb.len + 20, &shm,
+ git_path("shm-watchman-%s-%" PRIuMAX,
+ sha1_to_hex(istate->sha1),
+ (uintmax_t)pid))) {
+ is->size = sb.len + 20;
+ is->shm = shm;
+ is->pid = pid;
+ hashcpy(is->sha1, istate->sha1);
+
+ memcpy(shm, sb.buf, sb.len);
+ hashcpy((unsigned char *)shm + is->size - 20, is->sha1);
+ }
+ strbuf_release(&sb);
+}
+
+
+static void prepare_with_watchman(pid_t pid)
+{
+ /*
+ * TODO: with the help of watchman, maybe we could detect if
+ * $GIT_DIR/index is updated.
+ */
+ if (!verify_index(&the_index))
+ refresh();
+
+ if (check_watchman(&the_index))
+ return;
+
+ share_watchman(&the_index, &shm_watchman, pid);
+}
+
+static void prepare_index(pid_t pid)
+{
+ if (shm_index.shm == NULL)
+ refresh();
+ release_watchman_shm(&shm_watchman);
+ if (the_index.last_update)
+ prepare_with_watchman(pid);
+}
+
+#endif
+
+static void reply_to_poke(int client_fd, const char *pid_buf)
+{
+ char *capabilities;
+ struct strbuf sb = STRBUF_INIT;
+
+#ifdef USE_WATCHMAN
+ pid_t client_pid = strtoull(pid_buf, NULL, 10);
+
+ prepare_index(client_pid);
+#endif
+ capabilities = strchr(pid_buf, ' ');
+
+ if (!strcmp(capabilities, " watchman"))
+#ifdef USE_WATCHMAN
+ packet_buf_write(&sb, "OK watchman");
+#else
+ packet_buf_write(&sb, "NAK watchman");
+#endif
+ else
+ packet_buf_write(&sb, "OK");
+ if (write_in_full(client_fd, sb.buf, sb.len) != sb.len)
+ warning(_("client write failed"));
+}
+
static void loop(int fd, int idle_in_seconds)
{
assert(idle_in_seconds < INT_MAX / 1000);
@@ -275,11 +364,15 @@ static void loop(int fd, int idle_in_seconds)
buf[bytes_read] = 0;
if (!strcmp(buf, "refresh")) {
refresh();
- } else if (!strcmp(buf, "poke")) {
- /*
- * Just a poke to keep us
- * alive, nothing to do.
- */
+ } else if (starts_with(buf, "poke")) {
+ if (buf[4] == ' ') {
+ reply_to_poke(client_fd, buf + 5);
+ } else {
+ /*
+ * Just a poke to keep us
+ * alive, nothing to do.
+ */
+ }
} else {
log_warning("BUG: Bogus command %s", buf);
}
diff --git a/read-cache.c b/read-cache.c
index 9399a81..b4ed18e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1235,7 +1235,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,
if (!new) {
const char *fmt;
- if (really && cache_errno == EINVAL) {
+ if (really || cache_errno == EINVAL) {
/* If we are doing --really-refresh that
* means the index is not valid anymore.
*/
@@ -1375,11 +1375,75 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
return 0;
}
+static struct untracked_cache_dir *find_untracked_cache_dir(
+ struct untracked_cache *uc, struct untracked_cache_dir *ucd,
+ const char *name)
+{
+ int component_len;
+ const char *end;
+ struct untracked_cache_dir *dir;
+
+ if (!*name)
+ return ucd;
+
+ end = strchr(name, '/');
+ if (end)
+ component_len = end - name;
+ else
+ component_len = strlen(name);
+
+ dir = lookup_untracked(uc, ucd, name, component_len);
+ if (dir)
+ return find_untracked_cache_dir(uc, dir, name + component_len + 1);
+
+ return NULL;
+}
+
static void mark_no_watchman(size_t pos, void *data)
{
struct index_state *istate = data;
+ struct cache_entry *ce = istate->cache[pos];
+ struct strbuf sb = STRBUF_INIT;
+ char *c;
+ struct untracked_cache_dir *dir;
+
assert(pos < istate->cache_nr);
- istate->cache[pos]->ce_flags |= CE_WATCHMAN_DIRTY;
+ ce->ce_flags |= CE_WATCHMAN_DIRTY;
+
+ if (!istate->untracked || !istate->untracked->root)
+ return;
+
+ strbuf_add(&sb, ce->name, ce_namelen(ce));
+
+ for (c = sb.buf + sb.len - 1; c > sb.buf; c--) {
+ if (*c == '/') {
+ strbuf_setlen(&sb, c - sb.buf);
+ break;
+ }
+ }
+
+ if (c == sb.buf)
+ strbuf_setlen(&sb, 0);
+
+ dir = find_untracked_cache_dir(istate->untracked,
+ istate->untracked->root, sb.buf);
+ if (dir)
+ dir->valid = 0;
+
+ strbuf_release(&sb);
+}
+
+static int mark_untracked_invalid(struct string_list_item *item, void *uc)
+{
+ struct untracked_cache *untracked = uc;
+ struct untracked_cache_dir *dir;
+
+ dir = find_untracked_cache_dir(untracked, untracked->root,
+ item->string);
+ if (dir)
+ dir->valid = 0;
+
+ return 0;
}
static int read_watchman_ext(struct index_state *istate, const void *data,
@@ -1409,10 +1473,24 @@ static int read_watchman_ext(struct index_state *istate, const void *data,
ewah_each_bit(bitmap, mark_no_watchman, istate);
ewah_free(bitmap);
- /*
- * TODO: update the untracked cache from the untracked data in this
- * extension.
- */
+ if (istate->untracked && istate->untracked->root) {
+ int i;
+ const char *untracked;
+
+ untracked = (const char *)data + len + 8 + bitmap_size;
+ for (i = 0; i < untracked_nr; ++i) {
+ int len = strlen(untracked);
+ string_list_append(&istate->untracked->invalid_untracked,
+ untracked);
+ untracked += len + 1;
+ }
+
+ for_each_string_list(&istate->untracked->invalid_untracked,
+ mark_untracked_invalid, istate->untracked);
+
+ if (untracked_nr)
+ istate->cache_changed |= WATCHMAN_CHANGED;
+ }
return 0;
}
@@ -1645,29 +1723,90 @@ static void post_read_index_from(struct index_state *istate)
tweak_untracked_cache(istate);
}
+/* in ms */
+#define WATCHMAN_TIMEOUT 1000
+
+static int poke_and_wait_for_reply(int fd)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int ret = -1;
+ struct pollfd pollfd;
+ int bytes_read;
+ char reply_buf[4096];
+ const char *requested_capabilities = "";
+
+#ifdef USE_WATCHMAN
+ requested_capabilities = "watchman";
+#endif
+
+ if (fd < 0)
+ return -1;
+
+ strbuf_addf(&buf, "poke %d %s", getpid(), requested_capabilities);
+ packet_write(fd, buf.buf, buf.len);
+ packet_flush(fd);
+
+ /* Now wait for a reply */
+ pollfd.fd = fd;
+ pollfd.events = POLLIN;
+ if (poll(&pollfd, 1, WATCHMAN_TIMEOUT) <= 0)
+ /* No reply or error, giving up */
+ goto done_poke;
+
+ bytes_read = packet_read(fd, NULL, NULL, reply_buf, sizeof(reply_buf),
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_CHOMP_NEWLINE);
+
+ if (bytes_read < 0)
+ goto done_poke;
+
+ if (!strcmp(reply_buf, "NAK watchman")
+#ifdef USE_WATCHMAN
+ || !ends_with(reply_buf, "watchman")
+#endif
+ ) {
+ warning("We requested watchman support from index-helper, but "
+ "it doesn't support it. Please use a version of git "
+ "index-helper with watchman support.");
+ goto done_poke;
+ }
+
+ if (!starts_with(reply_buf, "OK"))
+ goto done_poke;
+
+ ret = 0;
+done_poke:
+ close(fd);
+ strbuf_release(&buf);
+
+ return ret;
+}
+
static int poke_daemon(struct index_state *istate,
const struct stat *st, int refresh_cache)
{
int fd;
- int ret = 0;
- const char *socket_path;
+ int ret = -1;
/* if this is from index-helper, do not poke itself (recursively) */
if (istate->to_shm)
return 0;
- socket_path = git_path("index-helper.sock");
- if (!socket_path)
+ fd = unix_stream_connect(git_path("index-helper.sock"));
+ if (fd < 0) {
+ warning("Failed to connect to index-helper socket");
+ unlink(git_path("index-helper.sock"));
return -1;
-
- fd = unix_stream_connect(socket_path);
+ }
sigchain_push(SIGPIPE, SIG_IGN);
+
if (refresh_cache) {
packet_write(fd, "refresh");
+ packet_flush(fd);
+ ret = 0;
} else {
- packet_write(fd, "poke");
+ ret = poke_and_wait_for_reply(fd);
}
- packet_flush(fd);
close(fd);
sigchain_pop(SIGPIPE);
@@ -1737,6 +1876,74 @@ fail:
return -1;
}
+static void refresh_by_watchman(struct index_state *istate)
+{
+ void *shm = NULL;
+ int length;
+ int i;
+ struct stat st;
+ int fd = -1;
+ const char *path = git_path("shm-watchman-%s-%"PRIuMAX,
+ sha1_to_hex(istate->sha1),
+ (uintmax_t)getpid());
+
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ return;
+
+ /*
+ * This watchman data is just for us -- no need to keep it
+ * around once we've got it open.
+ */
+ unlink(path);
+
+ if (fstat(fd, &st) < 0)
+ goto done;
+
+ length = st.st_size;
+ shm = mmap(NULL, length, PROT_READ, MAP_SHARED, fd, 0);
+
+ if (shm == MAP_FAILED)
+ goto done;
+
+ close(fd);
+ fd = -1;
+
+ if (length <= 20 ||
+ hashcmp(istate->sha1, (unsigned char *)shm + length - 20) ||
+ /*
+ * No need to clear CE_WATCHMAN_DIRTY set by 'WAMA' on
+ * disk. Watchman can only set more, not clear any, so
+ * this is OR mask.
+ */
+ read_watchman_ext(istate, shm, length - 20))
+ goto done;
+
+ /*
+ * Now that we've marked the invalid entries in the
+ * untracked-cache itself, we can erase them from the list of
+ * entries to be processed and mark the untracked cache for
+ * watchman usage.
+ */
+ if (istate->untracked) {
+ string_list_clear(&istate->untracked->invalid_untracked, 0);
+ istate->untracked->use_watchman = 1;
+ }
+
+ for (i = 0; i < istate->cache_nr; i++) {
+ struct cache_entry *ce = istate->cache[i];
+ if (ce_stage(ce) || (ce->ce_flags & CE_WATCHMAN_DIRTY))
+ continue;
+ ce_mark_uptodate(ce);
+ }
+done:
+ if (shm)
+ munmap(shm, length);
+
+ if (fd >= 0)
+ close(fd);
+}
+
/* remember to discard_cache() before reading a different cache! */
int do_read_index(struct index_state *istate, const char *path, int must_exist)
{
@@ -1856,7 +2063,7 @@ int read_index_from(struct index_state *istate, const char *path)
split_index = istate->split_index;
if (!split_index || is_null_sha1(split_index->base_sha1)) {
post_read_index_from(istate);
- return ret;
+ goto done;
}
if (split_index->base)
@@ -1877,6 +2084,10 @@ int read_index_from(struct index_state *istate, const char *path)
sha1_to_hex(split_index->base->sha1));
merge_base_index(istate);
post_read_index_from(istate);
+
+done:
+ if (ret > 0 && istate->from_shm && istate->last_update)
+ refresh_by_watchman(istate);
return ret;
}
@@ -2178,7 +2389,7 @@ out:
return 0;
}
-static int verify_index(const struct index_state *istate)
+int verify_index(const struct index_state *istate)
{
return verify_index_from(istate, get_index_file());
}
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 11/19] update-index: enable/disable watchman support
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (9 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 10/19] index-helper: use watchman to avoid refreshing index with lstat() David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 12/19] unpack-trees: preserve index extensions David Turner
` (8 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 3 +++
Documentation/git-update-index.txt | 6 ++++++
builtin/update-index.c | 16 ++++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index cce00cb..55a8a5a 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -18,6 +18,9 @@ each with a submodule, you might need four index-helpers. (In practice,
this is only worthwhile for large indexes, so only use it if you notice
that git status is slow).
+If you want the index-helper to accelerate untracked file checking,
+run git update-index --watchman before using it.
+
OPTIONS
-------
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index c6cbed1..6736487 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -19,6 +19,7 @@ SYNOPSIS
[--ignore-submodules]
[--[no-]split-index]
[--[no-|test-|force-]untracked-cache]
+ [--[no-]watchman]
[--really-refresh] [--unresolve] [--again | -g]
[--info-only] [--index-info]
[-z] [--stdin] [--index-version <n>]
@@ -176,6 +177,11 @@ may not support it yet.
--no-untracked-cache::
Enable or disable untracked cache feature. Please use
`--test-untracked-cache` before enabling it.
+
+--watchman::
+--no-watchman::
+ Enable or disable watchman support. This is, at present,
+ only useful with git index-helper.
+
These options take effect whatever the value of the `core.untrackedCache`
configuration variable (see linkgit:git-config[1]). But a warning is
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 1c94ca5..a3b4b5d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -914,6 +914,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
{
int newfd, entries, has_errors = 0, nul_term_line = 0;
enum uc_mode untracked_cache = UC_UNSPECIFIED;
+ int use_watchman = -1;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -1012,6 +1013,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
N_("test if the filesystem supports untracked cache"), UC_TEST),
OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
N_("enable untracked cache without testing the filesystem"), UC_FORCE),
+ OPT_BOOL(0, "watchman", &use_watchman,
+ N_("use or not use watchman to reduce refresh cost")),
OPT_END()
};
@@ -1149,6 +1152,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
die("Bug: bad untracked_cache value: %d", untracked_cache);
}
+ if (use_watchman > 0) {
+ the_index.last_update = xstrdup("");
+ the_index.cache_changed |= WATCHMAN_CHANGED;
+#ifndef USE_WATCHMAN
+ warning("git was built without watchman support -- I'm "
+ "adding the extension here, but it probably won't "
+ "do you any good.");
+#endif
+ } else if (!use_watchman) {
+ the_index.last_update = NULL;
+ the_index.cache_changed |= WATCHMAN_CHANGED;
+ }
+
if (active_cache_changed) {
if (newfd < 0) {
if (refresh_args.flags & REFRESH_QUIET)
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 12/19] unpack-trees: preserve index extensions
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (10 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 11/19] update-index: enable/disable watchman support David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 13/19] watchman: add a config option to enable the extension David Turner
` (7 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Make git checkout (and other unpack_tree operations) preserve the
untracked cache and watchman status. This is valuable for two reasons:
1. Often, an unpack_tree operation will not touch large parts of the
working tree, and thus most of the untracked cache will continue to be
valid.
2. Even if the untracked cache were entirely invalidated by such an
operation, the user has signaled their intention to have such a cache,
and we don't want to throw it away.
The same logic applies to the watchman state.
Signed-off-by: David Turner <dturner@twopensource.com>
---
cache.h | 1 +
read-cache.c | 8 ++++++++
t/t7063-status-untracked-cache.sh | 22 ++++++++++++++++++++++
t/test-lib-functions.sh | 4 ++++
unpack-trees.c | 1 +
5 files changed, 36 insertions(+)
diff --git a/cache.h b/cache.h
index 633e1dd..1b372ed 100644
--- a/cache.h
+++ b/cache.h
@@ -580,6 +580,7 @@ extern void write_watchman_ext(struct strbuf *sb, struct index_state *istate);
#define CLOSE_LOCK (1 << 1)
extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
extern int discard_index(struct index_state *);
+extern void move_index_extensions(struct index_state *dst, struct index_state *src);
extern int unmerged_index(const struct index_state *);
extern int verify_path(const char *path);
extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
diff --git a/read-cache.c b/read-cache.c
index b4ed18e..76b65c2 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2769,3 +2769,11 @@ void stat_validity_update(struct stat_validity *sv, int fd)
fill_stat_data(sv->sd, &st);
}
}
+
+void move_index_extensions(struct index_state *dst, struct index_state *src)
+{
+ dst->untracked = src->untracked;
+ src->untracked = NULL;
+ dst->last_update = src->last_update;
+ src->last_update = NULL;
+}
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index a971884..083516d 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -646,4 +646,26 @@ test_expect_success 'test ident field is working' '
test_cmp ../expect ../err
'
+test_expect_success 'untracked cache survives a checkout' '
+ git commit --allow-empty -m empty &&
+ test-dump-untracked-cache >../before &&
+ test_when_finished "git checkout master" &&
+ git checkout -b other_branch &&
+ test-dump-untracked-cache >../after &&
+ test_cmp ../before ../after &&
+ test_commit test &&
+ test-dump-untracked-cache >../before &&
+ git checkout master &&
+ test-dump-untracked-cache >../after &&
+ test_cmp ../before ../after
+'
+
+test_expect_success 'untracked cache survives a commit' '
+ test-dump-untracked-cache >../before &&
+ git add done/two &&
+ git commit -m commit &&
+ test-dump-untracked-cache >../after &&
+ test_cmp ../before ../after
+'
+
test_done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d99eb3..e974b5b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -186,6 +186,10 @@ test_commit () {
test_tick
fi &&
git commit $signoff -m "$1" &&
+ if [ "$(git config core.bare)" = false ]
+ then
+ git update-index --force-untracked-cache
+ fi
git tag "${4:-$1}"
}
diff --git a/unpack-trees.c b/unpack-trees.c
index 9f55cc2..fc90eb3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1215,6 +1215,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
WRITE_TREE_SILENT |
WRITE_TREE_REPAIR);
}
+ move_index_extensions(&o->result, o->dst_index);
discard_index(o->dst_index);
*o->dst_index = o->result;
} else {
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 13/19] watchman: add a config option to enable the extension
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (11 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 12/19] unpack-trees: preserve index extensions David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 14/19] index-helper: kill mode David Turner
` (6 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
For installations that have centrally-managed configuration, it's
easier to set a config once than to run update-index on every
repository.
Signed-off-by: David Turner <dturner@twopensource.com>
---
.gitignore | 1 +
Documentation/config.txt | 4 ++++
Makefile | 1 +
read-cache.c | 6 ++++++
t/t1701-watchman-extension.sh | 37 +++++++++++++++++++++++++++++++++++++
test-dump-watchman.c | 16 ++++++++++++++++
6 files changed, 65 insertions(+)
create mode 100755 t/t1701-watchman-extension.sh
create mode 100644 test-dump-watchman.c
diff --git a/.gitignore b/.gitignore
index b92f122..e6a5b2c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -188,6 +188,7 @@
/test-dump-cache-tree
/test-dump-split-index
/test-dump-untracked-cache
+/test-dump-watchman
/test-fake-ssh
/test-scrap-cache-tree
/test-genrandom
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..15001ce 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1848,6 +1848,10 @@ imap::
The configuration variables in the 'imap' section are described
in linkgit:git-imap-send[1].
+index.addwatchmanextension::
+ Automatically add the watchman extension to the index whenever
+ it is written.
+
index.version::
Specify the version with which new index files should be
initialized. This does not affect existing repositories.
diff --git a/Makefile b/Makefile
index 65ab0f4..5f0a954 100644
--- a/Makefile
+++ b/Makefile
@@ -599,6 +599,7 @@ TEST_PROGRAMS_NEED_X += test-delta
TEST_PROGRAMS_NEED_X += test-dump-cache-tree
TEST_PROGRAMS_NEED_X += test-dump-split-index
TEST_PROGRAMS_NEED_X += test-dump-untracked-cache
+TEST_PROGRAMS_NEED_X += test-dump-watchman
TEST_PROGRAMS_NEED_X += test-fake-ssh
TEST_PROGRAMS_NEED_X += test-genrandom
TEST_PROGRAMS_NEED_X += test-hashmap
diff --git a/read-cache.c b/read-cache.c
index 76b65c2..7b7cb39 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2428,6 +2428,7 @@ static int do_write_index(struct index_state *istate, int newfd,
int entries = istate->cache_nr;
struct stat st;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
+ int watchman = 0;
for (i = removed = extended = 0; i < entries; i++) {
if (cache[i]->ce_flags & CE_REMOVE)
@@ -2451,6 +2452,11 @@ static int do_write_index(struct index_state *istate, int newfd,
if (istate->version == 3 || istate->version == 2)
istate->version = extended ? 3 : 2;
+ if (!git_config_get_bool("index.addwatchmanextension", &watchman) &&
+ watchman &&
+ !the_index.last_update)
+ the_index.last_update = xstrdup("");
+
hdr_version = istate->version;
hdr.hdr_signature = htonl(CACHE_SIGNATURE);
diff --git a/t/t1701-watchman-extension.sh b/t/t1701-watchman-extension.sh
new file mode 100755
index 0000000..71f1d46
--- /dev/null
+++ b/t/t1701-watchman-extension.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='watchman extension smoke tests'
+
+# These don't actually test watchman interaction -- just the
+# index extension
+
+. ./test-lib.sh
+
+test_expect_success 'enable watchman' '
+ test_commit a &&
+ test-dump-watchman .git/index >actual &&
+ echo "last_update: (null)" >expect &&
+ test_cmp expect actual &&
+ git update-index --watchman &&
+ test-dump-watchman .git/index >actual &&
+ echo "last_update: " >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'disable watchman' '
+ git update-index --no-watchman &&
+ test-dump-watchman .git/index >actual &&
+ echo "last_update: (null)" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'auto-enable watchman' '
+ test_config index.addwatchmanextension true &&
+ test_commit c &&
+ test-dump-watchman .git/index >actual &&
+ echo "last_update: " >expect &&
+ test_cmp expect actual
+'
+
+
+test_done
diff --git a/test-dump-watchman.c b/test-dump-watchman.c
new file mode 100644
index 0000000..0314fa5
--- /dev/null
+++ b/test-dump-watchman.c
@@ -0,0 +1,16 @@
+#include "cache.h"
+#include "ewah/ewok.h"
+
+int main(int argc, char **argv)
+{
+ do_read_index(&the_index, argv[1], 1);
+ printf("last_update: %s\n", the_index.last_update ?
+ the_index.last_update : "(null)");
+
+ /*
+ * For now, we just dump last_update, since it is not reasonable
+ * to populate the extension itself in tests.
+ */
+
+ return 0;
+}
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 14/19] index-helper: kill mode
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (12 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 13/19] watchman: add a config option to enable the extension David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 15/19] index-helper: don't run if already running David Turner
` (5 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Add a new command (and command-line arg) to allow index-helpers to
exit cleanly.
This is mainly useful for tests.
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 3 +++
index-helper.c | 31 ++++++++++++++++++++++++++++++-
t/t7900-index-helper.sh | 9 +++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index 55a8a5a..addf694 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -40,6 +40,9 @@ OPTIONS
--detach::
Detach from the shell.
+--kill::
+ Kill any running index-helper and clean up the socket
+
NOTES
-----
diff --git a/index-helper.c b/index-helper.c
index 71c4f48..4ed1610 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -373,6 +373,8 @@ static void loop(int fd, int idle_in_seconds)
* alive, nothing to do.
*/
}
+ } else if (!strcmp(buf, "die")) {
+ break;
} else {
log_warning("BUG: Bogus command %s", buf);
}
@@ -403,10 +405,29 @@ static const char * const usage_text[] = {
NULL
};
+static void request_kill(void)
+{
+ int fd = unix_stream_connect(git_path("index-helper.sock"));
+
+ if (fd >= 0) {
+ write_in_full(fd, "die", 4);
+ close(fd);
+ }
+
+ /*
+ * The child will try to do this anyway, but we want to be
+ * ready to launch a new daemon immediately after this command
+ * returns.
+ */
+
+ unlink(git_path("index-helper.sock"));
+ return;
+}
+
int main(int argc, char **argv)
{
const char *prefix;
- int idle_in_seconds = 600, detach = 0;
+ int idle_in_seconds = 600, detach = 0, kill = 0;
int fd;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
@@ -415,6 +436,7 @@ int main(int argc, char **argv)
OPT_BOOL(0, "strict", &to_verify,
"verify shared memory after creating"),
OPT_BOOL(0, "detach", &detach, "detach the process"),
+ OPT_BOOL(0, "kill", &kill, "request that existing index helper processes exit"),
OPT_END()
};
@@ -429,6 +451,13 @@ int main(int argc, char **argv)
options, usage_text, 0))
die(_("too many arguments"));
+ if (kill) {
+ if (detach)
+ die(_("--kill doesn't want any other options"));
+ request_kill();
+ return 0;
+ }
+
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index 114c112..e71b5af 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -20,4 +20,13 @@ test_expect_success 'index-helper smoke test' '
test_path_is_missing .git/index-helper.sock
'
+test_expect_success 'index-helper creates usable path file and can be killed' '
+ test_when_finished "git index-helper --kill" &&
+ test_path_is_missing .git/index-helper.sock &&
+ git index-helper --detach &&
+ test -S .git/index-helper.sock &&
+ git index-helper --kill &&
+ test_path_is_missing .git/index-helper.sock
+'
+
test_done
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 15/19] index-helper: don't run if already running
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (13 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 14/19] index-helper: kill mode David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 16/19] index-helper: autorun mode David Turner
` (4 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Signed-off-by: David Turner <dturner@twopensource.com>
---
index-helper.c | 7 +++++++
t/t7900-index-helper.sh | 9 +++++++++
2 files changed, 16 insertions(+)
diff --git a/index-helper.c b/index-helper.c
index 4ed1610..b275f6e 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -458,6 +458,13 @@ int main(int argc, char **argv)
return 0;
}
+ /* check that no other copy is running */
+ fd = unix_stream_connect(git_path("index-helper.sock"));
+ if (fd > 0)
+ die(_("Already running"));
+ if (errno != ECONNREFUSED && errno != ENOENT)
+ die_errno(_("Unexpected error checking socket"));
+
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index e71b5af..7159971 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -29,4 +29,13 @@ test_expect_success 'index-helper creates usable path file and can be killed' '
test_path_is_missing .git/index-helper.sock
'
+test_expect_success 'index-helper will not start if already running' '
+ test_when_finished "git index-helper --kill" &&
+ git index-helper --detach &&
+ test -S .git/index-helper.sock &&
+ test_must_fail git index-helper 2>err &&
+ test -S .git/index-helper.sock &&
+ grep "Already running" err
+'
+
test_done
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 16/19] index-helper: autorun mode
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (14 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 15/19] index-helper: don't run if already running David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 17/19] index-helper: optionally automatically run David Turner
` (3 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Soon, we'll want to automatically start index-helper, so we need
a mode that silently exits if it can't start up (either because
it's not in a git dir, or because another one is already running).
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/git-index-helper.txt | 4 ++++
index-helper.c | 29 +++++++++++++++++++++++------
t/t7900-index-helper.sh | 8 ++++++++
3 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-index-helper.txt b/Documentation/git-index-helper.txt
index addf694..0466296 100644
--- a/Documentation/git-index-helper.txt
+++ b/Documentation/git-index-helper.txt
@@ -43,6 +43,10 @@ OPTIONS
--kill::
Kill any running index-helper and clean up the socket
+--autorun::
+ If index-helper is not already running, start it. Else, do
+ nothing.
+
NOTES
-----
diff --git a/index-helper.c b/index-helper.c
index b275f6e..9743481 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -427,8 +427,9 @@ static void request_kill(void)
int main(int argc, char **argv)
{
const char *prefix;
- int idle_in_seconds = 600, detach = 0, kill = 0;
+ int idle_in_seconds = 600, detach = 0, kill = 0, autorun = 0;
int fd;
+ int nongit;
struct strbuf socket_path = STRBUF_INIT;
struct option options[] = {
OPT_INTEGER(0, "exit-after", &idle_in_seconds,
@@ -437,6 +438,7 @@ int main(int argc, char **argv)
"verify shared memory after creating"),
OPT_BOOL(0, "detach", &detach, "detach the process"),
OPT_BOOL(0, "kill", &kill, "request that existing index helper processes exit"),
+ OPT_BOOL(0, "autorun", &autorun, "this is an automatic run of git index-helper, so certain errors can be solved by silently exiting"),
OPT_END()
};
@@ -446,7 +448,14 @@ int main(int argc, char **argv)
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(usage_text, options);
- prefix = setup_git_directory();
+ prefix = setup_git_directory_gently(&nongit);
+ if (nongit) {
+ if (autorun)
+ exit(0);
+ else
+ die(_("not a git repository"));
+ }
+
if (parse_options(argc, (const char **)argv, prefix,
options, usage_text, 0))
die(_("too many arguments"));
@@ -460,10 +469,18 @@ int main(int argc, char **argv)
/* check that no other copy is running */
fd = unix_stream_connect(git_path("index-helper.sock"));
- if (fd > 0)
- die(_("Already running"));
- if (errno != ECONNREFUSED && errno != ENOENT)
- die_errno(_("Unexpected error checking socket"));
+ if (fd > 0) {
+ if (autorun)
+ exit(0);
+ else
+ die(_("Already running"));
+ }
+ if (errno != ECONNREFUSED && errno != ENOENT) {
+ if (autorun)
+ return 0;
+ else
+ die_errno(_("Unexpected error checking socket"));
+ }
atexit(cleanup);
sigchain_push_common(cleanup_on_signal);
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index 7159971..aa6e5fc 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -38,4 +38,12 @@ test_expect_success 'index-helper will not start if already running' '
grep "Already running" err
'
+test_expect_success 'index-helper is quiet with --autorun' '
+ test_when_finished "git index-helper --kill" &&
+ git index-helper --kill &&
+ git index-helper --detach &&
+ test -S .git/index-helper.sock &&
+ git index-helper --autorun
+'
+
test_done
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 17/19] index-helper: optionally automatically run
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (15 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 16/19] index-helper: autorun mode David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 18/19] trace: measure where the time is spent in the index-heavy operations David Turner
` (2 subsequent siblings)
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Introduce a new config option, indexhelper.autorun, to automatically
run git index-helper before starting up a builtin git command. This
enables users to keep index-helper running without manual
intervention.
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/config.txt | 4 ++++
read-cache.c | 37 +++++++++++++++++++++++++++++++++++--
t/t7900-index-helper.sh | 20 ++++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 15001ce..385ea66 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1856,6 +1856,10 @@ index.version::
Specify the version with which new index files should be
initialized. This does not affect existing repositories.
+indexhelper.autorun::
+ Automatically run git index-helper when any builtin git
+ command is run inside a repository.
+
init.templateDir::
Specify the directory from which templates will be copied.
(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
diff --git a/read-cache.c b/read-cache.c
index 7b7cb39..ebbcb7f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -22,6 +22,7 @@
#include "pkt-line.h"
#include "sigchain.h"
#include "ewah/ewok.h"
+#include "run-command.h"
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
unsigned int options);
@@ -1723,6 +1724,33 @@ static void post_read_index_from(struct index_state *istate)
tweak_untracked_cache(istate);
}
+static int want_auto_index_helper(void)
+{
+ int want = -1;
+ const char *value = NULL;
+ const char *key = "indexhelper.autorun";
+
+ if (git_config_key_is_valid(key) &&
+ !git_config_get_value(key, &value)) {
+ int b = git_config_maybe_bool(key, value);
+ want = b >= 0 ? b : 0;
+ }
+ return want;
+}
+
+static void autorun_index_helper(void)
+{
+ const char *argv[] = {"git-index-helper", "--detach", "--autorun", NULL};
+ if (want_auto_index_helper() <= 0)
+ return;
+
+ trace_argv_printf(argv, "trace: auto index-helper:");
+
+ if (run_command_v_opt(argv,
+ RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT))
+ warning(_("You specified indexhelper.autorun, but running git-index-helper failed."));
+}
+
/* in ms */
#define WATCHMAN_TIMEOUT 1000
@@ -1796,6 +1824,7 @@ static int poke_daemon(struct index_state *istate,
if (fd < 0) {
warning("Failed to connect to index-helper socket");
unlink(git_path("index-helper.sock"));
+ autorun_index_helper();
return -1;
}
sigchain_push(SIGPIPE, SIG_IGN);
@@ -1835,9 +1864,13 @@ static int try_shm(struct index_state *istate)
int fd = -1;
if (!is_main_index(istate) ||
- old_size <= 20 ||
- stat(git_path("index-helper.sock"), &st))
+ old_size <= 20)
return -1;
+
+ if (stat(git_path("index-helper.sock"), &st)) {
+ autorun_index_helper();
+ return -1;
+ }
if (poke_daemon(istate, &st, 0))
return -1;
sha1 = (unsigned char *)istate->mmap + old_size - 20;
diff --git a/t/t7900-index-helper.sh b/t/t7900-index-helper.sh
index aa6e5fc..3cfdf63 100755
--- a/t/t7900-index-helper.sh
+++ b/t/t7900-index-helper.sh
@@ -16,6 +16,9 @@ test -n "$NO_MMAP" && {
}
test_expect_success 'index-helper smoke test' '
+ # We need an existing commit so that the index exists (otherwise,
+ # the index-helper will not be autostarted)
+ test_commit x &&
git index-helper --exit-after 1 &&
test_path_is_missing .git/index-helper.sock
'
@@ -46,4 +49,21 @@ test_expect_success 'index-helper is quiet with --autorun' '
git index-helper --autorun
'
+test_expect_success 'index-helper autorun works' '
+ test_when_finished "git index-helper --kill" &&
+ rm -f .git/index-helper.sock &&
+ git status &&
+ test_path_is_missing .git/index-helper.sock &&
+ test_config indexhelper.autorun true &&
+ git status &&
+ test -S .git/index-helper.sock &&
+ git status 2>err &&
+ test -S .git/index-helper.sock &&
+ test_must_be_empty err &&
+ git index-helper --kill &&
+ test_config indexhelper.autorun false &&
+ git status &&
+ test_path_is_missing .git/index-helper.sock
+'
+
test_done
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 18/19] trace: measure where the time is spent in the index-heavy operations
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (16 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 17/19] index-helper: optionally automatically run David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 20:48 ` [PATCH v9 19/19] untracked-cache: config option David Turner
2016-05-09 21:40 ` [PATCH v9 00/19] index-helper/watchman Junio C Hamano
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
All the known heavy code blocks are measured (except object database
access). This should help identify if an optimization is effective or
not. An unoptimized git-status would give something like below (92% of
time is accounted). To sum up the effort of making git scale better:
- read cache line is being addressed by index-helper
- preload/refresh index by watchman
- read packed refs by lmdb backend
- diff-index by rebuilding cache-tree
- read directory by untracked cache and watchman
- write index by split index
- name hash potientially by index-helper
read-cache.c:2075 performance: 0.004058570 s: read cache .../index
preload-index.c:104 performance: 0.004419864 s: preload index
read-cache.c:1265 performance: 0.000185224 s: refresh index
refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs
diff-lib.c:240 performance: 0.000144132 s: diff-files
diff-lib.c:506 performance: 0.013592000 s: diff-index
name-hash.c:128 performance: 0.000614177 s: initialize name hash
dir.c:2030 performance: 0.015814103 s: read directory
read-cache.c:2565 performance: 0.004052343 s: write index, changed mask = 2
trace.c:420 performance: 0.048365509 s: git command: './git' 'status'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
---
diff-lib.c | 4 ++++
dir.c | 2 ++
name-hash.c | 2 ++
preload-index.c | 2 ++
read-cache.c | 11 +++++++++++
refs/files-backend.c | 2 ++
6 files changed, 23 insertions(+)
diff --git a/diff-lib.c b/diff-lib.c
index bc49c70..7af7f9a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -90,6 +90,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
int diff_unmerged_stage = revs->max_count;
unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
? CE_MATCH_RACY_IS_DIRTY : 0);
+ uint64_t start = getnanotime();
diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
@@ -236,6 +237,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
}
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
+ trace_performance_since(start, "diff-files");
return 0;
}
@@ -491,6 +493,7 @@ static int diff_cache(struct rev_info *revs,
int run_diff_index(struct rev_info *revs, int cached)
{
struct object_array_entry *ent;
+ uint64_t start = getnanotime();
ent = revs->pending.objects;
if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
@@ -500,6 +503,7 @@ int run_diff_index(struct rev_info *revs, int cached)
diffcore_fix_diff_index(&revs->diffopt);
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
+ trace_performance_since(start, "diff-index");
return 0;
}
diff --git a/dir.c b/dir.c
index 5058b29..c56a8b9 100644
--- a/dir.c
+++ b/dir.c
@@ -2183,6 +2183,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru
{
struct path_simplify *simplify;
struct untracked_cache_dir *untracked;
+ uint64_t start = getnanotime();
/*
* Check out create_simplify()
@@ -2224,6 +2225,7 @@ int read_directory(struct dir_struct *dir, const char *path, int len, const stru
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), cmp_name);
+ trace_performance_since(start, "read directory %.*s", len, path);
if (dir->untracked) {
static struct trace_key trace_untracked_stats = TRACE_KEY_INIT(UNTRACKED_STATS);
trace_printf_key(&trace_untracked_stats,
diff --git a/name-hash.c b/name-hash.c
index 6d9f23e..b3966d8 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -115,6 +115,7 @@ static int cache_entry_cmp(const struct cache_entry *ce1,
static void lazy_init_name_hash(struct index_state *istate)
{
int nr;
+ uint64_t start = getnanotime();
if (istate->name_hash_initialized)
return;
@@ -124,6 +125,7 @@ static void lazy_init_name_hash(struct index_state *istate)
for (nr = 0; nr < istate->cache_nr; nr++)
hash_index_entry(istate, istate->cache[nr]);
istate->name_hash_initialized = 1;
+ trace_performance_since(start, "initialize name hash");
}
void add_name_hash(struct index_state *istate, struct cache_entry *ce)
diff --git a/preload-index.c b/preload-index.c
index c1fe3a3..7bb4809 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -72,6 +72,7 @@ static void preload_index(struct index_state *index,
{
int threads, i, work, offset;
struct thread_data data[MAX_PARALLEL];
+ uint64_t start = getnanotime();
if (!core_preload_index)
return;
@@ -100,6 +101,7 @@ static void preload_index(struct index_state *index,
if (pthread_join(p->pthread, NULL))
die("unable to join threaded lstat");
}
+ trace_performance_since(start, "preload index");
}
#endif
diff --git a/read-cache.c b/read-cache.c
index ebbcb7f..c547b0b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1189,6 +1189,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,
const char *typechange_fmt;
const char *added_fmt;
const char *unmerged_fmt;
+ uint64_t start = getnanotime();
modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
@@ -1263,6 +1264,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,
replace_index_entry(istate, i, new);
}
+ trace_performance_since(start, "refresh index");
return has_errors;
}
@@ -2086,12 +2088,15 @@ int read_index_from(struct index_state *istate, const char *path)
{
struct split_index *split_index;
int ret;
+ uint64_t start;
/* istate->initialized covers both .git/index and .git/sharedindex.xxx */
if (istate->initialized)
return istate->cache_nr;
+ start = getnanotime();
ret = do_read_index(istate, path, 0);
+ trace_performance_since(start, "read cache %s", path);
split_index = istate->split_index;
if (!split_index || is_null_sha1(split_index->base_sha1)) {
@@ -2106,6 +2111,7 @@ int read_index_from(struct index_state *istate, const char *path)
split_index->base->keep_mmap = istate->keep_mmap;
split_index->base->to_shm = istate->to_shm;
split_index->base->from_shm = istate->from_shm;
+ start = getnanotime();
ret = do_read_index(split_index->base,
git_path("sharedindex.%s",
sha1_to_hex(split_index->base_sha1)), 1);
@@ -2117,6 +2123,9 @@ int read_index_from(struct index_state *istate, const char *path)
sha1_to_hex(split_index->base->sha1));
merge_base_index(istate);
post_read_index_from(istate);
+ trace_performance_since(start, "read cache %s",
+ git_path("sharedindex.%s",
+ sha1_to_hex(split_index->base_sha1)));
done:
if (ret > 0 && istate->from_shm && istate->last_update)
@@ -2462,6 +2471,7 @@ static int do_write_index(struct index_state *istate, int newfd,
struct stat st;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
int watchman = 0;
+ uint64_t start = getnanotime();
for (i = removed = extended = 0; i < entries; i++) {
if (cache[i]->ce_flags & CE_REMOVE)
@@ -2582,6 +2592,7 @@ static int do_write_index(struct index_state *istate, int newfd,
return -1;
istate->timestamp.sec = (unsigned int)st.st_mtime;
istate->timestamp.nsec = ST_MTIME_NSEC(st);
+ trace_performance_since(start, "write index, changed mask = %x", istate->cache_changed);
return 0;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 81f68f8..57571ce 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1048,6 +1048,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
struct ref_entry *last = NULL;
struct strbuf line = STRBUF_INIT;
enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
+ uint64_t start = getnanotime();
while (strbuf_getwholeline(&line, f, '\n') != EOF) {
unsigned char sha1[20];
@@ -1096,6 +1097,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
}
strbuf_release(&line);
+ trace_performance_since(start, "read packed refs");
}
/*
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v9 19/19] untracked-cache: config option
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (17 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 18/19] trace: measure where the time is spent in the index-heavy operations David Turner
@ 2016-05-09 20:48 ` David Turner
2016-05-09 21:40 ` [PATCH v9 00/19] index-helper/watchman Junio C Hamano
19 siblings, 0 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 20:48 UTC (permalink / raw)
To: git, pclouds; +Cc: David Turner
Add a config option to populate the untracked cache.
For installations that have centrally-managed configuration, it's
easier to set a config once than to run update-index on every
repository.
Signed-off-by: David Turner <dturner@twopensource.com>
---
Documentation/config.txt | 4 ++++
read-cache.c | 7 ++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 385ea66..c7b76ef 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1848,6 +1848,10 @@ imap::
The configuration variables in the 'imap' section are described
in linkgit:git-imap-send[1].
+index.adduntrackedcache::
+ Automatically populate the untracked cache whenever the index
+ is written.
+
index.addwatchmanextension::
Automatically add the watchman extension to the index whenever
it is written.
diff --git a/read-cache.c b/read-cache.c
index c547b0b..7e735e2 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2470,7 +2470,7 @@ static int do_write_index(struct index_state *istate, int newfd,
int entries = istate->cache_nr;
struct stat st;
struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;
- int watchman = 0;
+ int watchman = 0, untracked = 0;
uint64_t start = getnanotime();
for (i = removed = extended = 0; i < entries; i++) {
@@ -2500,6 +2500,11 @@ static int do_write_index(struct index_state *istate, int newfd,
!the_index.last_update)
the_index.last_update = xstrdup("");
+ if (!git_config_get_bool("index.adduntrackedcache", &untracked) &&
+ untracked &&
+ !istate->untracked)
+ add_untracked_cache(&the_index);
+
hdr_version = istate->version;
hdr.hdr_signature = htonl(CACHE_SIGNATURE);
--
2.4.2.767.g62658d5-twtrsrc
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 20:48 [PATCH v9 00/19] index-helper/watchman David Turner
` (18 preceding siblings ...)
2016-05-09 20:48 ` [PATCH v9 19/19] untracked-cache: config option David Turner
@ 2016-05-09 21:40 ` Junio C Hamano
2016-05-09 22:15 ` David Turner
19 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2016-05-09 21:40 UTC (permalink / raw)
To: David Turner; +Cc: git, pclouds
Hmmm, I seem to be getting
$ cat t/trash*7900*/err
fatal: Already running
after running t7900 and it fails at #5, after applying
"index-helper: optionally automatically run".
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 21:40 ` [PATCH v9 00/19] index-helper/watchman Junio C Hamano
@ 2016-05-09 22:15 ` David Turner
2016-05-09 22:22 ` Junio C Hamano
0 siblings, 1 reply; 39+ messages in thread
From: David Turner @ 2016-05-09 22:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, pclouds
On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> Hmmm, I seem to be getting
>
> $ cat t/trash*7900*/err
> fatal: Already running
>
> after running t7900 and it fails at #5, after applying
> "index-helper: optionally automatically run"
It still passes for me (with or without USE_WATCHMAN set). I'm testing
on Ubuntu 15.10 with "Linux ubuntu 4.2.0-35-generic", on an ext4
filesystem.
Does it fail before or after git status 2>err ? (If before, then those
"err" contents are from the previous test).
Is index-helper in fact running after the test fails?
Can you tell me a little about your system? I'm not even sure what
would be useful here, but if I could set up a similarly-configured vm,
I could try to repro.
Thanks, and sorry for the failures.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 22:15 ` David Turner
@ 2016-05-09 22:22 ` Junio C Hamano
2016-05-09 22:32 ` Junio C Hamano
2016-05-10 11:52 ` Dennis Kaarsemaker
0 siblings, 2 replies; 39+ messages in thread
From: Junio C Hamano @ 2016-05-09 22:22 UTC (permalink / raw)
To: David Turner; +Cc: git, pclouds
David Turner <dturner@twopensource.com> writes:
> On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
>> Hmmm, I seem to be getting
>>
>> $ cat t/trash*7900*/err
>> fatal: Already running
>>
>> after running t7900 and it fails at #5, after applying
>> "index-helper: optionally automatically run"
>
> It still passes for me (with or without USE_WATCHMAN set). I'm testing
> on Ubuntu 15.10 with "Linux ubuntu 4.2.0-35-generic", on an ext4
> filesystem.
It passes on one box and fails on another. They both run the same
Ubuntu 14.04 derivative, with same ext3 filesystem. The failing one
is on a VM.
> Does it fail before or after git status 2>err ? (If before, then those
> "err" contents are from the previous test).
I didn't dig and I do not know (yet).
> Is index-helper in fact running after the test fails?
"ps ax | grep index-helper" showed a process, which I had to "kill"
manually.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 22:22 ` Junio C Hamano
@ 2016-05-09 22:32 ` Junio C Hamano
2016-05-09 23:12 ` David Turner
2016-05-10 17:28 ` Dennis Kaarsemaker
2016-05-10 11:52 ` Dennis Kaarsemaker
1 sibling, 2 replies; 39+ messages in thread
From: Junio C Hamano @ 2016-05-09 22:32 UTC (permalink / raw)
To: David Turner; +Cc: git, pclouds
Junio C Hamano <gitster@pobox.com> writes:
> David Turner <dturner@twopensource.com> writes:
>
>> On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
>>> Hmmm, I seem to be getting
>>>
>>> $ cat t/trash*7900*/err
>>> fatal: Already running
>>>
>>> after running t7900 and it fails at #5, after applying
>>> "index-helper: optionally automatically run"
The symptom looks pretty similar to $gmane/293461 reported earlier.
Here is how "t7900-index-helper.sh -i -v -x -d" ends.
expecting success:
test_when_finished "git index-helper --kill" &&
rm -f .git/index-helper.sock &&
git status &&
test_path_is_missing .git/index-helper.sock &&
test_config indexhelper.autorun true &&
git status &&
test -S .git/index-helper.sock &&
git status 2>err &&
test -S .git/index-helper.sock &&
test_must_be_empty err &&
git index-helper --kill &&
test_config indexhelper.autorun false &&
git status &&
test_path_is_missing .git/index-helper.sock
+ test_when_finished git index-helper --kill
+ test 0 = 0
+ test_cleanup={ git index-helper --kill
} && (exit "$eval_ret"); eval_ret=$?; :
+ rm -f .git/index-helper.sock
+ git status
On branch master
Untracked files:
(use "git add <file>..." to include in what will be committed)
err
nothing added to commit but untracked files present (use "git add" to track)
+ test_path_is_missing .git/index-helper.sock
+ test -e .git/index-helper.sock
+ test_config indexhelper.autorun true
+ config_dir=
+ test indexhelper.autorun = -C
+ test_when_finished test_unconfig 'indexhelper.autorun'
+ test 0 = 0
+ test_cleanup={ test_unconfig 'indexhelper.autorun'
} && (exit "$eval_ret"); eval_ret=$?; { git index-helper --kill
} && (exit "$eval_ret"); eval_ret=$?; :
+ git config indexhelper.autorun true
+ git status
error: last command exited with $?=141
not ok 5 - index-helper autorun works
#
# test_when_finished "git index-helper --kill" &&
# rm -f .git/index-helper.sock &&
# git status &&
# test_path_is_missing .git/index-helper.sock &&
# test_config indexhelper.autorun true &&
# git status &&
# test -S .git/index-helper.sock &&
# git status 2>err &&
# test -S .git/index-helper.sock &&
# test_must_be_empty err &&
# git index-helper --kill &&
# test_config indexhelper.autorun false &&
# git status &&
# test_path_is_missing .git/index-helper.sock
#
:
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 22:32 ` Junio C Hamano
@ 2016-05-09 23:12 ` David Turner
2016-05-09 23:46 ` Ramsay Jones
2016-05-10 12:45 ` Duy Nguyen
2016-05-10 17:28 ` Dennis Kaarsemaker
1 sibling, 2 replies; 39+ messages in thread
From: David Turner @ 2016-05-09 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, pclouds
On Mon, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > David Turner <dturner@twopensource.com> writes:
> >
> > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > > Hmmm, I seem to be getting
> > > >
> > > > $ cat t/trash*7900*/err
> > > > fatal: Already running
> > > >
> > > > after running t7900 and it fails at #5, after applying
> > > > "index-helper: optionally automatically run"
>
> The symptom looks pretty similar to $gmane/293461 reported earlier.
> Here is how "t7900-index-helper.sh -i -v -x -d" ends.
>
>
> expecting success:
> test_when_finished "git index-helper --kill" &&
> rm -f .git/index-helper.sock &&
> git status &&
> test_path_is_missing .git/index-helper.sock &&
> test_config indexhelper.autorun true &&
> git status &&
> test -S .git/index-helper.sock &&
> git status 2>err &&
> test -S .git/index-helper.sock &&
> test_must_be_empty err &&
> git index-helper --kill &&
> test_config indexhelper.autorun false &&
> git status &&
> test_path_is_missing .git/index-helper.sock
>
> + test_when_finished git index-helper --kill
> + test 0 = 0
> + test_cleanup={ git index-helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + rm -f .git/index-helper.sock
> + git status
> On branch master
> Untracked files:
> (use "git add <file>..." to include in what will be committed)
>
> err
>
> nothing added to commit but untracked files present (use "git add" to
> track)
> + test_path_is_missing .git/index-helper.sock
> + test -e .git/index-helper.sock
> + test_config indexhelper.autorun true
> + config_dir=
> + test indexhelper.autorun = -C
> + test_when_finished test_unconfig 'indexhelper.autorun'
> + test 0 = 0
> + test_cleanup={ test_unconfig 'indexhelper.autorun'
> } && (exit "$eval_ret"); eval_ret=$?; { git index
> -helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + git config indexhelper.autorun true
> + git status
> error: last command exited with $?=141
I think that's a SIGPIPE on the first git status. Weird, since I just
added sigpipe-avoidance code (in v8). Does anyone have any idea why
the sigchain stuff isn't doing what I think it is?
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 23:12 ` David Turner
@ 2016-05-09 23:46 ` Ramsay Jones
2016-05-10 12:45 ` Duy Nguyen
1 sibling, 0 replies; 39+ messages in thread
From: Ramsay Jones @ 2016-05-09 23:46 UTC (permalink / raw)
To: David Turner, Junio C Hamano; +Cc: git, pclouds
On 10/05/16 00:12, David Turner wrote:
> On Mon, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> David Turner <dturner@twopensource.com> writes:
>>>
>>>> On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
>>>>> Hmmm, I seem to be getting
>>>>>
>>>>> $ cat t/trash*7900*/err
>>>>> fatal: Already running
>>>>>
>>>>> after running t7900 and it fails at #5, after applying
>>>>> "index-helper: optionally automatically run"
>>
>> The symptom looks pretty similar to $gmane/293461 reported earlier.
>> Here is how "t7900-index-helper.sh -i -v -x -d" ends.
>>
>>
>> expecting success:
>> test_when_finished "git index-helper --kill" &&
>> rm -f .git/index-helper.sock &&
>> git status &&
>> test_path_is_missing .git/index-helper.sock &&
>> test_config indexhelper.autorun true &&
>> git status &&
>> test -S .git/index-helper.sock &&
>> git status 2>err &&
>> test -S .git/index-helper.sock &&
>> test_must_be_empty err &&
>> git index-helper --kill &&
>> test_config indexhelper.autorun false &&
>> git status &&
>> test_path_is_missing .git/index-helper.sock
>>
>> + test_when_finished git index-helper --kill
>> + test 0 = 0
>> + test_cleanup={ git index-helper --kill
>> } && (exit "$eval_ret"); eval_ret=$?; :
>> + rm -f .git/index-helper.sock
>> + git status
>> On branch master
>> Untracked files:
>> (use "git add <file>..." to include in what will be committed)
>>
>> err
>>
>> nothing added to commit but untracked files present (use "git add" to
>> track)
>> + test_path_is_missing .git/index-helper.sock
>> + test -e .git/index-helper.sock
>> + test_config indexhelper.autorun true
>> + config_dir=
>> + test indexhelper.autorun = -C
>> + test_when_finished test_unconfig 'indexhelper.autorun'
>> + test 0 = 0
>> + test_cleanup={ test_unconfig 'indexhelper.autorun'
>> } && (exit "$eval_ret"); eval_ret=$?; { git index
>> -helper --kill
>> } && (exit "$eval_ret"); eval_ret=$?; :
>> + git config indexhelper.autorun true
>> + git status
>> error: last command exited with $?=141
>
> I think that's a SIGPIPE on the first git status. Weird, since I just
> added sigpipe-avoidance code (in v8). Does anyone have any idea why
> the sigchain stuff isn't doing what I think it is?
Sorry for a late report (I've been a bit busy last couple of days), but
I've been seeing exactly the same on v8 of this series.
Note that the above 'git status' is actually the second git-status in the
test.
I haven't been able to debug it too much, but I can tell you that it is
not failing at exactly the same place every time (so it may be time
sensitive). However, it often fails in poke_and_wait_for_reply() at the
first packet_flush() (which in turn calls write_or_die() which calls
check_pipe() with an EPIPE(32)). At other times it fails when issuing
a flush after a refresh packet. For example, on one run with packet
tracing enabled, I got this for the trace:
trace: built-in: git 'status'
packet: git> poke 3221
packet: git> 0000
packet: git< OK
packet: git> refresh
packet: git> 0000
So, its getting the EPIPE for the refresh in this case, even though
the index-helper is still running, the unix socket is in .git/
(and so is a shm-* file BTW).
I didn't get any further than that I'm afraid.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 23:12 ` David Turner
2016-05-09 23:46 ` Ramsay Jones
@ 2016-05-10 12:45 ` Duy Nguyen
2016-05-10 12:57 ` Duy Nguyen
1 sibling, 1 reply; 39+ messages in thread
From: Duy Nguyen @ 2016-05-10 12:45 UTC (permalink / raw)
To: David Turner; +Cc: Junio C Hamano, Git Mailing List
On Tue, May 10, 2016 at 6:12 AM, David Turner <dturner@twopensource.com> wrote:
> I think that's a SIGPIPE on the first git status. Weird, since I just
> added sigpipe-avoidance code (in v8). Does anyone have any idea why
> the sigchain stuff isn't doing what I think it is?
SIGPIPE code works. I tested with and without --detach (suspecting
fork causing problems). I even logged all sigchain_push() calls to see
if anybody else is interfering (none did, at least without watchman).
Problem is elsewhere.
There are two problems with logging code in index-helper though. The
redirecting std{out,err} to /dev/null should only be done when
--detach is given (and I' m quite sure daemonize() already takes care
of that). If --detach is used, log_warning() can't cover die(),
warning() or error(), most importantly die() for example because of
bugs. I think you can call set_die_routine() and similar, just be
careful not lead to recursive calls (die() is protected, but I dont
think error and warning are).
--
Duy
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-10 12:45 ` Duy Nguyen
@ 2016-05-10 12:57 ` Duy Nguyen
0 siblings, 0 replies; 39+ messages in thread
From: Duy Nguyen @ 2016-05-10 12:57 UTC (permalink / raw)
To: David Turner; +Cc: Junio C Hamano, Git Mailing List
On Tue, May 10, 2016 at 7:45 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> If --detach is used, log_warning() can't cover die(),
> warning() or error(), most importantly die() for example because of
> bugs.
A case for redirecting warning() is because watchman-support.c uses
it. But because this file is only used by index-helper, you have
another option, simply convert it to using log_warning().
--
Duy
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 22:32 ` Junio C Hamano
2016-05-09 23:12 ` David Turner
@ 2016-05-10 17:28 ` Dennis Kaarsemaker
2016-05-10 17:44 ` Dennis Kaarsemaker
1 sibling, 1 reply; 39+ messages in thread
From: Dennis Kaarsemaker @ 2016-05-10 17:28 UTC (permalink / raw)
To: Junio C Hamano, David Turner; +Cc: git, pclouds
On ma, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> >
> > David Turner <dturner@twopensource.com> writes:
> >
> > >
> > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > >
> > > > Hmmm, I seem to be getting
> > > >
> > > > $ cat t/trash*7900*/err
> > > > fatal: Already running
> > > >
> > > > after running t7900 and it fails at #5, after applying
> > > > "index-helper: optionally automatically run"
> The symptom looks pretty similar to $gmane/293461 reported earlier.
> Here is how "t7900-index-helper.sh -i -v -x -d" ends.
>
>
> expecting success:
> test_when_finished "git index-helper --kill" &&
> rm -f .git/index-helper.sock &&
> git status &&
> test_path_is_missing .git/index-helper.sock &&
> test_config indexhelper.autorun true &&
> git status &&
> test -S .git/index-helper.sock &&
> git status 2>err &&
> test -S .git/index-helper.sock &&
> test_must_be_empty err &&
> git index-helper --kill &&
> test_config indexhelper.autorun false &&
> git status &&
> test_path_is_missing .git/index-helper.sock
>
> + test_when_finished git index-helper --kill
> + test 0 = 0
> + test_cleanup={ git index-helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + rm -f .git/index-helper.sock
> + git status
> On branch master
> Untracked files:
> (use "git add <file>..." to include in what will be committed)
>
> err
>
> nothing added to commit but untracked files present (use "git add" to
> track)
> + test_path_is_missing .git/index-helper.sock
> + test -e .git/index-helper.sock
> + test_config indexhelper.autorun true
> + config_dir=
> + test indexhelper.autorun = -C
> + test_when_finished test_unconfig 'indexhelper.autorun'
> + test 0 = 0
> + test_cleanup={ test_unconfig 'indexhelper.autorun'
> } && (exit "$eval_ret"); eval_ret=$?; { git index-
> helper --kill
> } && (exit "$eval_ret"); eval_ret=$?; :
> + git config indexhelper.autorun true
> + git status
> error: last command exited with $?=141
> not ok 5 - index-helper autorun works
> #
> # test_when_finished "git index-helper --kill" &&
> # rm -f .git/index-helper.sock &&
> # git status &&
> # test_path_is_missing .git/index-helper.sock &&
> # test_config indexhelper.autorun true &&
> # git status &&
> # test -S .git/index-helper.sock &&
> # git status 2>err &&
> # test -S .git/index-helper.sock &&
> # test_must_be_empty err &&
> # git index-helper --kill &&
> # test_config indexhelper.autorun false &&
> # git status &&
> # test_path_is_missing .git/index-helper.sock
> #
Here are the relevant bits of a strace, pid 22200 is the second git
status, 222197 is the index helper. 22122 is the test script
22200 socket(PF_LOCAL, SOCK_STREAM, 0) = 7
22200 connect(7, {sa_family=AF_LOCAL, sun_path=".git/index-helper.sock"}, 110 <unfinished ...>
22197 <... poll resumed> ) = 1
22197 accept(7, 0, NULL) = 8
22197 fcntl(8, F_GETFL) = 0x2 (flags O_RDWR)
22197 fcntl(8, F_SETFL, O_RDWR) = 0
22197 read(8, <unfinished ...>
22200 <... connect resumed> ) = 0
22200 rt_sigaction(SIGPIPE, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, 8) = 0
22200 write(7, "000fpoke 22200 ", 15 <unfinished ...>
22197 <... read resumed> 0x7ffc4e4b9b20, 4) = 4
22197 read(8, 0x7ffc4e4b9c70, 11) = 11
22197 write(8, 0x18b08b0, 6) = 6
22197 close(8) = 0
22197 poll([?] 0x7ffc4e4b9b80, 1, 600000 <unfinished ...>
22200 <... write resumed> ) = 15
22200 write(7, "0000", 4) = -1 EPIPE (Broken pipe)
22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=22200, si_uid=1000} ---
22200 rt_sigaction(SIGPIPE, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART, 0x7fcc463fdd40}, 8) = 0
22200 tgkill(22200, 22200, SIGPIPE) = 0
22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=22200, si_uid=1000} ---
22200 +++ killed by SIGPIPE +++
22122 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) == SIGPIPE}], 0, NULL) = 22200
22122 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED, si_pid=22200, si_status=SIGPIPE, si_utime=0, si_stime=0} ---
Looks like the index helper closes the socket, but git status still
wants to write to it. The index-helper also doesn't seem to read all
data from the socket.
--
Dennis Kaarsemaker
www.kaarsemaker.net
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-10 17:28 ` Dennis Kaarsemaker
@ 2016-05-10 17:44 ` Dennis Kaarsemaker
0 siblings, 0 replies; 39+ messages in thread
From: Dennis Kaarsemaker @ 2016-05-10 17:44 UTC (permalink / raw)
To: Junio C Hamano, David Turner; +Cc: git, pclouds
On di, 2016-05-10 at 19:28 +0200, Dennis Kaarsemaker wrote:
> On ma, 2016-05-09 at 15:32 -0700, Junio C Hamano wrote:
> >
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> > >
> > >
> > > David Turner <dturner@twopensource.com> writes:
> > >
> > > >
> > > >
> > > > On Mon, 2016-05-09 at 14:40 -0700, Junio C Hamano wrote:
> > > > >
> > > > >
> > > > > Hmmm, I seem to be getting
> > > > >
> > > > > $ cat t/trash*7900*/err
> > > > > fatal: Already running
> > > > >
> > > > > after running t7900 and it fails at #5, after applying
> > > > > "index-helper: optionally automatically run"
> > The symptom looks pretty similar to $gmane/293461 reported earlier.
> > Here is how "t7900-index-helper.sh -i -v -x -d" ends.
> >
> >
> > expecting success:
> > test_when_finished "git index-helper --kill" &&
> > rm -f .git/index-helper.sock &&
> > git status &&
> > test_path_is_missing .git/index-helper.sock &&
> > test_config indexhelper.autorun true &&
> > git status &&
> > test -S .git/index-helper.sock &&
> > git status 2>err &&
> > test -S .git/index-helper.sock &&
> > test_must_be_empty err &&
> > git index-helper --kill &&
> > test_config indexhelper.autorun false &&
> > git status &&
> > test_path_is_missing .git/index-helper.sock
> >
> > + test_when_finished git index-helper --kill
> > + test 0 = 0
> > + test_cleanup={ git index-helper --kill
> > } && (exit "$eval_ret"); eval_ret=$?; :
> > + rm -f .git/index-helper.sock
> > + git status
> > On branch master
> > Untracked files:
> > (use "git add <file>..." to include in what will be committed)
> >
> > err
> >
> > nothing added to commit but untracked files present (use "git add"
> > to
> > track)
> > + test_path_is_missing .git/index-helper.sock
> > + test -e .git/index-helper.sock
> > + test_config indexhelper.autorun true
> > + config_dir=
> > + test indexhelper.autorun = -C
> > + test_when_finished test_unconfig 'indexhelper.autorun'
> > + test 0 = 0
> > + test_cleanup={ test_unconfig 'indexhelper.autorun'
> > } && (exit "$eval_ret"); eval_ret=$?; { git index-
> > helper --kill
> > } && (exit "$eval_ret"); eval_ret=$?; :
> > + git config indexhelper.autorun true
> > + git status
> > error: last command exited with $?=141
> > not ok 5 - index-helper autorun works
> > #
> > # test_when_finished "git index-helper --kill" &&
> > # rm -f .git/index-helper.sock &&
> > # git status &&
> > # test_path_is_missing .git/index-helper.sock &&
> > # test_config indexhelper.autorun true &&
> > # git status &&
> > # test -S .git/index-helper.sock &&
> > # git status 2>err &&
> > # test -S .git/index-helper.sock &&
> > # test_must_be_empty err &&
> > # git index-helper --kill &&
> > # test_config indexhelper.autorun false &&
> > # git status &&
> > # test_path_is_missing .git/index-helper.sock
> > #
> Here are the relevant bits of a strace, pid 22200 is the second git
> status, 222197 is the index helper. 22122 is the test script
>
> 22200 socket(PF_LOCAL, SOCK_STREAM, 0) = 7
> 22200 connect(7, {sa_family=AF_LOCAL, sun_path=".git/index-
> helper.sock"}, 110 <unfinished ...>
> 22197 <... poll resumed> ) = 1
> 22197 accept(7, 0, NULL) = 8
> 22197 fcntl(8, F_GETFL) = 0x2 (flags O_RDWR)
> 22197 fcntl(8, F_SETFL, O_RDWR) = 0
> 22197 read(8, <unfinished ...>
> 22200 <... connect resumed> ) = 0
> 22200 rt_sigaction(SIGPIPE, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 write(7, "000fpoke 22200 ", 15 <unfinished ...>
> 22197 <... read resumed> 0x7ffc4e4b9b20, 4) = 4
> 22197 read(8, 0x7ffc4e4b9c70, 11) = 11
> 22197 write(8, 0x18b08b0, 6) = 6
> 22197 close(8) = 0
> 22197 poll([?] 0x7ffc4e4b9b80, 1, 600000 <unfinished ...>
> 22200 <... write resumed> ) = 15
> 22200 write(7, "0000", 4) = -1 EPIPE (Broken pipe)
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=22200,
> si_uid=1000} ---
> 22200 rt_sigaction(SIGPIPE, {SIG_DFL, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, {SIG_IGN, [PIPE], SA_RESTORER|SA_RESTART,
> 0x7fcc463fdd40}, 8) = 0
> 22200 tgkill(22200, 22200, SIGPIPE) = 0
> 22200 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=22200,
> si_uid=1000} ---
> 22200 +++ killed by SIGPIPE +++
> 22122 <... wait4 resumed> [{WIFSIGNALED(s) && WTERMSIG(s) ==
> SIGPIPE}], 0, NULL) = 22200
> 22122 --- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_KILLED,
> si_pid=22200, si_status=SIGPIPE, si_utime=0, si_stime=0} ---
>
> Looks like the index helper closes the socket, but git status still
> wants to write to it. The index-helper also doesn't seem to read all
> data from the socket.
This dodgy hack to make the index helper read that empty pkt-line
before closing the socket seems to work around the issue enough to make
the test pass
diff --git a/index-helper.c b/index-helper.c
index 9743481..fa072de 100644
--- a/index-helper.c
+++ b/index-helper.c
@@ -385,6 +385,8 @@ static void loop(int fd, int idle_in_seconds)
* close up.
*/
}
+ bytes_read = packet_read(client_fd, NULL, NULL, buf,
+ sizeof(buf), flags);
close(client_fd);
}
And this even dodgier hack breaks the race on a physical machine, so
the test reliably fails everywher:
diff --git a/read-cache.c b/read-cache.c
index e8593bc..848a1fe 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1836,6 +1836,7 @@ static int poke_and_wait_for_reply(int fd)
strbuf_addf(&buf, "poke %d %s", getpid(), requested_capabilities);
packet_write(fd, buf.buf, buf.len);
+ sleep(5);
packet_flush(fd);
/* Now wait for a reply */
But at this point I'll have to claim ignorance on whether reading the
empty pkt-line is a fix or just hiding the problem.
--
Dennis Kaarsemaker
www.kaarsemaker.net
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-09 22:22 ` Junio C Hamano
2016-05-09 22:32 ` Junio C Hamano
@ 2016-05-10 11:52 ` Dennis Kaarsemaker
2016-05-10 15:35 ` Ramsay Jones
1 sibling, 1 reply; 39+ messages in thread
From: Dennis Kaarsemaker @ 2016-05-10 11:52 UTC (permalink / raw)
To: Junio C Hamano, David Turner; +Cc: git, pclouds
On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
> It passes on one box and fails on another. They both run the same
> Ubuntu 14.04 derivative, with same ext3 filesystem. The failing one
> is on a VM.
Same here, except ext4 instead of ext3. Failing on a virtual machine,
not failing on a physical one.
--
Dennis Kaarsemaker
http://www.kaarsemaker.net
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-10 11:52 ` Dennis Kaarsemaker
@ 2016-05-10 15:35 ` Ramsay Jones
2016-05-10 20:30 ` Junio C Hamano
0 siblings, 1 reply; 39+ messages in thread
From: Ramsay Jones @ 2016-05-10 15:35 UTC (permalink / raw)
To: Dennis Kaarsemaker, Junio C Hamano, David Turner; +Cc: git, pclouds
On 10/05/16 12:52, Dennis Kaarsemaker wrote:
> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
>> It passes on one box and fails on another. They both run the same
>> Ubuntu 14.04 derivative, with same ext3 filesystem. The failing one
>> is on a VM.
>
> Same here, except ext4 instead of ext3. Failing on a virtual machine,
> not failing on a physical one.
I can confirm the trend:
Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-10 15:35 ` Ramsay Jones
@ 2016-05-10 20:30 ` Junio C Hamano
2016-05-10 22:09 ` Ramsay Jones
0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2016-05-10 20:30 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Dennis Kaarsemaker, David Turner, git, pclouds
Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> On 10/05/16 12:52, Dennis Kaarsemaker wrote:
>> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
>>> It passes on one box and fails on another. They both run the same
>>> Ubuntu 14.04 derivative, with same ext3 filesystem. The failing one
>>> is on a VM.
>>
>> Same here, except ext4 instead of ext3. Failing on a virtual machine,
>> not failing on a physical one.
>
> I can confirm the trend:
>
> Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.
>
> ATB,
> Ramsay Jones
I do not think there is anything VM specific, though. If it is
SIGPIPE, it is very much understandable it is timing dependent, and
VMness may be a cause of timing differences, nothing more.
I seem to getting the failure on my physical box today, by the way.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v9 00/19] index-helper/watchman
2016-05-10 20:30 ` Junio C Hamano
@ 2016-05-10 22:09 ` Ramsay Jones
0 siblings, 0 replies; 39+ messages in thread
From: Ramsay Jones @ 2016-05-10 22:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dennis Kaarsemaker, David Turner, git, pclouds
On 10/05/16 21:30, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>
>> On 10/05/16 12:52, Dennis Kaarsemaker wrote:
>>> On ma, 2016-05-09 at 15:22 -0700, Junio C Hamano wrote:
>>>> It passes on one box and fails on another. They both run the same
>>>> Ubuntu 14.04 derivative, with same ext3 filesystem. The failing one
>>>> is on a VM.
>>>
>>> Same here, except ext4 instead of ext3. Failing on a virtual machine,
>>> not failing on a physical one.
>>
>> I can confirm the trend:
>>
>> Linux Mint 17.3, ext4 - bare-metal pass, (Virtual Box) VM fail.
>>
> I do not think there is anything VM specific, though. If it is
> SIGPIPE, it is very much understandable it is timing dependent, and
> VMness may be a cause of timing differences, nothing more.
Yeah, I had previously noted that this was probably a timing related
issue - this was only meant as confirmation that David could probably
reproduce the issue if he tested in a VM. ;-)
However, it appears Dennis has found a way to force a failure, even
on bare-metal (at least on v8, I think).
> I seem to getting the failure on my physical box today, by the way.
Ah, interesting.
ATB,
Ramsay Jones
^ permalink raw reply [flat|nested] 39+ messages in thread