Re: Watchman support for git

From: Duy Nguyen <pclouds@gmail.com>
Date: May 10, 2014, 08:16 UTC
Message-ID: <CACsJy8DVVpfYEmE8pSZNyXy1m5WRkdm08deW3EXgAy_0Gn72zw@mail.gmail.com>
In-Reply-To: <1399072451-15561-1-git-send-email-dturner@twopensource.com>

On Sat, May 3, 2014 at 6:14 AM,  <dturner@twopensource.com> wrote:

> The most sigificant patch uses Facebook's watchman daemon[1] to monitor
> the repository work tree for changes.  This makes allows git status
> to avoid traversing the entire work tree to find changes.

Some comments on this series. I still haven't been able to run it with watchman so there are many guesses from my side.

First of all, when I set USE_WATCHMAN=Yes in config.mak I expect it to work out of the box, provided that all dependencies are installed. I still need to set WATCHMAN_LIBS for it to build because you only set it with configure script. Would be nice to have a default value for non-configure people too.

I'm not so happy that git now needs to link to libjansson.so and libwatchman.so. I would load libwatchman.so at runtime only when core.usewatchman is on, but this is more of personal taste.

I still prefer my old tracking model, where the majority of lstat() is done by refresh operation and we only need to optimize those lstat calls, not every single lstat statement in the code base. With that in mind, I think you don't need to keep a fs cache on disk at all. All you need to store (in the index) is the clock value from watchman. After we parse the index, we perform a "since" query to get path names (and perhaps "exists" and "mode" for later). Then we set CE_VALID bit on entries that are _not_ in the query result. The remaining items will be lstat'd by git (see [1] and read-cache.c changes onthe next few patches). Assuming the number of updated files is reasonably small, we won't be punished by lookup time. To avoid write time cost due to possible mass CE_VALID change, assuming split-index is already used, we could store this bit separately as an extension and apply it back at read time.

Your changes in dir.c. If I didn't misread it, you still do last_exclude_matching when using fs-cache. That call is where all your CPU is spent and probably explains why you didn't see big perf gain with watchman. I think my untracked cache has dealt correctly with caching in this area. So when you do the watchman query earlier, you also check if any of the directories are updated and force using untracked cache for the rest of the directories.

Hope it helps, Duy

[1] http://thread.gmane.org/gmane.comp.version-control.git/240339