Subject: [PATCH/WIP v2 05/14] read-cache: put some limits on file watching

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Fri, 17 Jan 2014 16:47:31 +0700

watch_entries() is a lot of computation and could trigger a lot more
lookups in file-watcher. Normally after the first set of watches are
in place, we do not need to update often. Moreover if the number of
entries is small, the overhead of file watcher may actually slow git
down.

This patch only allows to update watches if the number of watchable
files is over a limit (and there are new files added if this is not
the first time). Measurements on Core i5-2520M and Linux 3.7.6, about
920 lstat() take 1ms. Somewhere between 2^16 and 2^17 lstat calls that
it starts to take longer than 100ms. 2^16 is chosen at the minimum
limit to start using file watcher.

Recently updated files are not considered watchable because they are
likely to be updated again soon, not worth the ping-pong game with
file watcher. The default limit 30min is just a random value.

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 Documentation/config.txt |  9 +++++++++
 cache.h                  |  1 +
 read-cache.c             | 44 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..e394399 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1038,6 +1038,15 @@ difftool.<tool>.cmd::
 difftool.prompt::
 	Prompt before each invocation of the diff tool.
 
+filewatcher.minfiles::
+	Start watching files if the number of watchable files are
+	above this limit. Default value is 65536.
+
+filewatcher.recentlimit::
+	Files that are last updated within filewatcher.recentlimit
+	seconds from now are not considered watchable. Default value
+	is 1800 (30 minutes).
+
 fetch.recurseSubmodules::
 	This option can be either set to a boolean value or to 'on-demand'.
 	Setting it to a boolean changes the behavior of fetch and pull to
diff --git a/cache.h b/cache.h
index 0d55551..bcec29b 100644
--- a/cache.h
+++ b/cache.h
@@ -278,6 +278,7 @@ struct index_state {
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
 	unsigned name_hash_initialized : 1,
+		 update_watches : 1,
 		 initialized : 1;
 	struct hashmap name_hash;
 	struct hashmap dir_hash;
diff --git a/read-cache.c b/read-cache.c
index 21c3207..406834a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -38,6 +38,8 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
 #define CACHE_EXT_WATCH 0x57415443	  /* "WATC" */
 
 struct index_state the_index;
+static int watch_lowerlimit = 65536;
+static int recent_limit = 1800;
 
 static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)
 {
@@ -1014,6 +1016,7 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti
 			(istate->cache_nr - pos - 1) * sizeof(ce));
 	set_index_entry(istate, pos, ce);
 	istate->cache_changed = 1;
+	istate->update_watches = 1;
 	return 0;
 }
 
@@ -1300,13 +1303,14 @@ static void read_watch_extension(struct index_state *istate, uint8_t *data,
 				 unsigned long sz)
 {
 	int i;
-	if ((istate->cache_nr + 7) / 8 != sz) {
+	if ((istate->cache_nr + 7) / 8 + 1 != sz) {
 		error("invalid 'WATC' extension");
 		return;
 	}
 	for (i = 0; i < istate->cache_nr; i++)
 		if (data[i / 8] & (1 << (i % 8)))
 			istate->cache[i]->ce_flags |= CE_WATCHED;
+	istate->update_watches = data[sz - 1];
 }
 
 static int read_index_extension(struct index_state *istate,
@@ -1449,6 +1453,19 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk,
 	return ce;
 }
 
+static int watcher_config(const char *var, const char *value, void *data)
+{
+	if (!strcmp(var, "filewatcher.minfiles")) {
+		watch_lowerlimit = git_config_int(var, value);
+		return 0;
+	}
+	if (!strcmp(var, "filewatcher.recentlimit")) {
+		recent_limit = git_config_int(var, value);
+		return 0;
+	}
+	return 0;
+}
+
 static void validate_watcher(struct index_state *istate, const char *path)
 {
 	int i;
@@ -1458,6 +1475,7 @@ static void validate_watcher(struct index_state *istate, const char *path)
 		return;
 	}
 
+	git_config(watcher_config, NULL);
 	istate->watcher = connect_watcher(path);
 	if (istate->watcher != -1) {
 		struct strbuf sb = STRBUF_INIT;
@@ -1478,6 +1496,7 @@ static void validate_watcher(struct index_state *istate, const char *path)
 			istate->cache[i]->ce_flags &= ~CE_WATCHED;
 			istate->cache_changed = 1;
 		}
+	istate->update_watches = 1;
 }
 
 static int sort_by_date(const void *a_, const void *b_)
@@ -1524,7 +1543,7 @@ static int do_watch_entries(struct index_state *istate,
 	return 0;
 }
 
-static inline int ce_watchable(struct cache_entry *ce)
+static inline int ce_watchable(struct cache_entry *ce, time_t now)
 {
 	return ce_uptodate(ce) && /* write_index will catch late ce_uptodate bits */
 		!(ce->ce_flags & CE_WATCHED) &&
@@ -1534,7 +1553,8 @@ static inline int ce_watchable(struct cache_entry *ce)
 		 * obviously. S_IFLNK could be problematic because
 		 * inotify may follow symlinks without IN_DONT_FOLLOW
 		 */
-		S_ISREG(ce->ce_mode);
+		S_ISREG(ce->ce_mode) &&
+		(ce->ce_stat_data.sd_mtime.sec + recent_limit < now);
 }
 
 static void watch_entries(struct index_state *istate)
@@ -1544,15 +1564,20 @@ static void watch_entries(struct index_state *istate)
 	struct strbuf sb = STRBUF_INIT;
 	int val;
 	socklen_t vallen = sizeof(val);
+	time_t now = time(NULL);
 
-	if (istate->watcher <= 0)
+	if (istate->watcher <= 0 || !istate->update_watches)
 		return;
+	istate->update_watches = 0;
+	istate->cache_changed = 1;
 	for (i = nr = 0; i < istate->cache_nr; i++)
-		if (ce_watchable(istate->cache[i]))
+		if (ce_watchable(istate->cache[i], now))
 			nr++;
+	if (nr < watch_lowerlimit)
+		return;
 	sorted = xmalloc(sizeof(*sorted) * nr);
 	for (i = nr = 0; i < istate->cache_nr; i++)
-		if (ce_watchable(istate->cache[i]))
+		if (ce_watchable(istate->cache[i], now))
 			sorted[nr++] = istate->cache[i];
 
 	getsockopt(istate->watcher, SOL_SOCKET, SO_SNDBUF, &val, &vallen);
@@ -1616,6 +1641,7 @@ int read_index_from(struct index_state *istate, const char *path)
 	istate->cache_alloc = alloc_nr(istate->cache_nr);
 	istate->cache = xcalloc(istate->cache_alloc, sizeof(*istate->cache));
 	istate->initialized = 1;
+	istate->update_watches = 1;
 
 	if (istate->version == 4)
 		previous_name = &previous_name_buf;
@@ -2024,8 +2050,9 @@ int write_index(struct index_state *istate, int newfd)
 		if (err)
 			return -1;
 	}
-	if (has_watches) {
-		int id, sz = (entries - removed + 7) / 8;
+	if (has_watches ||
+	    (istate->watcher != -1 && !istate->update_watches)) {
+		int id, sz = (entries - removed + 7) / 8 + 1;
 		uint8_t *data = xmalloc(sz);
 		memset(data, 0, sz);
 		for (i = 0, id = 0; i < entries && has_watches; i++) {
@@ -2038,6 +2065,7 @@ int write_index(struct index_state *istate, int newfd)
 			}
 			id++;
 		}
+		data[sz - 1] = istate->update_watches;
 		err = write_index_ext_header(&c, newfd, CACHE_EXT_WATCH, sz) < 0
 			|| ce_write(&c, newfd, data, sz) < 0;
 		free(data);
-- 
1.8.5.1.208.g05b12ea