git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] index-helper: use watchman to avoid refreshing index with lstat()

From
Ramsay Jones <ramsay@ramsayjones.plus.com>
Date
May 13, 2016, 20:17 UTC
Message-ID
<5736366D.6080706@ramsayjones.plus.com>
In-Reply-To
<1463164054-15342-1-git-send-email-dturner@twopensource.com>
On 13/05/16 19:27, David Turner wrote:
> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
[snip]
Show 12 quoted lines
> +
> +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, ' ');

So, if the pid is *not* followed by a space, the capabilities will be NULL here, and ...

> +
> +	if (!strcmp(capabilities, " watchman"))
... we segfault here.
Show 177 quoted lines
> +#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);
> @@ -252,11 +341,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 {
>  				warning("BUG: Bogus command %s", buf);
>  			}
> diff --git a/read-cache.c b/read-cache.c
> index 1719f5a..8ec4be3 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,88 @@ 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)
> +{
> +	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;
> +
> +	if (packet_write_gently(fd, "poke %d %s", getpid(), requested_capabilities))

So, adding the empty capabilities (and more importantly the separating space) is not so much 'doesn't hurt', rather than 'prevents a core-dump!' ;-)

> +		return -1;
> +	if (packet_flush_gently(fd))
> +		return -1;

And yes, I'd forgotten about the 'maybe sometime in the future, we could buffer the packets' ...

ATB, Ramsay Jones

Previous: David TurnerNext: David Turner
Message 17 of 33 in “index-helper/watchman”
  1. 00/20 index-helper/watchmanDavid Turner, May 12, 2016
  2. 01/20 read-cache.c: fix constness of verify_hdr()David Turner, May 12, 2016
  3. 02/20 read-cache: allow to keep mmap'd memory after readingDavid Turner, May 12, 2016
  4. 03/20 pkt-line: add gentle version of packet_writeDavid Turner, May 12, 2016
  5. Junio C HamanoMay 12, 2016
  6. 04/20 index-helper: new daemon for caching index and related stuffDavid Turner, May 12, 2016
  7. 05/20 index-helper: add --strictDavid Turner, May 12, 2016
  8. 06/20 daemonize(): set a flag before exiting the main processDavid Turner, May 12, 2016
  9. 07/20 index-helper: add --detachDavid Turner, May 12, 2016
  10. 08/20 index-helper: log warningsDavid Turner, May 12, 2016
  11. 09/20 read-cache: add watchman 'WAMA' extensionDavid Turner, May 12, 2016
  12. 10/20 watchman: support watchman to reduce index refresh costDavid Turner, May 12, 2016
  13. 11/20 index-helper: use watchman to avoid refreshing index with lstat()David Turner, May 12, 2016
  14. Ramsay JonesMay 12, 2016
  15. David TurnerMay 13, 2016
  16. index-helper: use watchman to avoid refreshing index with lstat()David Turner, May 13, 2016
  17. Ramsay JonesMay 13, 2016
  18. index-helper: use watchman to avoid refreshing index with lstat()David Turner, May 13, 2016
  19. 12/20 update-index: enable/disable watchman supportDavid Turner, May 12, 2016
  20. 13/20 unpack-trees: preserve index extensionsDavid Turner, May 12, 2016
  21. 14/20 watchman: add a config option to enable the extensionDavid Turner, May 12, 2016
  22. 15/20 index-helper: kill modeDavid Turner, May 12, 2016
  23. 16/20 index-helper: don't run if already runningDavid Turner, May 12, 2016
  24. 17/20 index-helper: autorun modeDavid Turner, May 12, 2016
  25. 18/20 index-helper: optionally automatically runDavid Turner, May 12, 2016
  26. 19/20 trace: measure where the time is spent in the index-heavy operationsDavid Turner, May 12, 2016
  27. 20/20 untracked-cache: config optionDavid Turner, May 12, 2016
  28. Duy NguyenMay 15, 2016
  29. David TurnerMay 16, 2016
  30. Dennis KaarsemakerMay 14, 2016
  31. David TurnerMay 19, 2016
  32. Junio C HamanoMay 19, 2016
  33. David TurnerMay 19, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.