Re: Watchman support for git

From: Duy Nguyen <pclouds@gmail.com>
Date: May 11, 2014, 00:21 UTC
Message-ID: <CACsJy8Cazm+6ixw3r8WYfdFYeD01Lmf0PSF0sdsh7PGy_6WDTQ@mail.gmail.com>
In-Reply-To: <1399747109.11843.137.camel@stross>

On Sun, May 11, 2014 at 1:38 AM, David Turner <dturner@twopensource.com> wrote:

Show 7 quoted lines

>> I got "warning: Watchman watch error: Got bad JSON from watchman
>> get-sockname: '[' or '{' expected near end of file". Any ideas what I
>> did wrong? I'm using watchman.git and libwatchman.git. check-0.9.11
>> and jansson-2.4 were installed by system (gentoo).
>
> What do you get from watchman get-sockname on the command-line?  Do the
> watchman tests pass?

Found the problem. "watchman" binary is not in $PATH but popen() did not complain (or it did but your "2>/dev/null" in watchman_connect suppressed it). BTW you need to update the array size of "expressions" in test_watchman_misc().

So without watchman I got

   299.871ms read_index_from:1538 if (verify_hdr(hdr, mmap_size) < 0) go
   498.205ms cmd_status:1300 refresh_index(&the_index, REFRESH_QUIE
   796.050ms wt_status_collect:622 wt_status_collect_untracked(s)

and with watchman ("git status" ran several times to make sure it's cached)

   301.950ms read_index_from:1538 if (verify_hdr(hdr, mmap_size) < 0) go
    34.918ms  read_fs_cache:347 if (verify_hdr(hdr, mmap_size) < 0) go
  1564.096ms  watchman_load_fs_cache:628 update_fs_cache(istate, result);
   161.930ms cmd_status:1300 refresh_index(&the_index, REFRESH_QUIE
   251.614ms wt_status_collect:622 wt_status_collect_untracked(s)

Given the total time of "git status" without watchman is 1.9s,, update_fs_cache() nearly matches that number alone. All that is spent in the exclude update code in the function, but if you do last_excluding_matching() anyway, why cache it?

I think we're paying lookup cost in refresh_index(). I forced CE_VALID bit on as an experiment and refresh_index() went down to 33ms.

A bit surprised about wt_status_collect_untracked number. I verified that last_excluding_matching() still runs (on the same number of entries like in no-watchman case). Replacing fs_cache_open() in add_excludes_from_file_to_list() to plain open() does not change the number, so we probably won't need that (unless your worktree is filled with .gitignore, which I doubt it's a norm). So about 500ms saved is in opendir/readdir.. A simple/separate opendir/readdir/closedir program confirms that (ugh!).

-- 
Duy