Re: [PATCH v2 0/6] Fast git status via a file system watcher

[Ben Peart <peartben@gmail.com>]

On 5/31/2017 3:59 AM, Christian Couder wrote:
> On Thu, May 25, 2017 at 3:55 PM, Ben Peart <peartben@gmail.com> wrote:
>>
>> On 5/24/2017 6:54 AM, Christian Couder wrote:
>>>>
>>>> A new git hook (query-fsmonitor) must exist and be enabled
>>>> (core.fsmonitor=true) that takes a time_t formatted as a string and
>>>> outputs to stdout all files that have been modified since the requested
>>>> time.
>>>
>>> Is there a reason why there is a new hook, instead of a
>>> "core.fsmonitorquery" config option to which you could pass whatever
>>> command line with options?
>>
>> A hook is a simple and well defined way to integrate git with another
>> process.  If there is some fixed set of arguments that need to be passed to
>> a file system monitor (beyond the timestamp stored in the index extension),
>> they can be encoded in the integration script like I've done in the Watchman
>> integration sample hook.
> 
> Yeah, but a hook must also be called everytime git wants to
> communicate with the file system monitor. And we could perhaps get a
> speed up if we could have only one long running process to communicate
> with the file system monitor.
> 

In this particular case a long running background processes isn't 
helpful because refresh_by_fsmonitor() already has logic to ensure the 
file system monitor is only called once per git process.

The overhead of that one call isn't significant as demonstrated by the 
performance numbers I sent out in the cover letter.  Even with the cost 
of the hook and all the associated post-processing, this patch series 
still results in a nice performance win (even on Windows where spawning 
processes is typically more expensive than on other platforms).

I appreciate the close look at this patch series!  Even when I'm pushing 
back, I'm glad someone is asking questions and making sure I've thought 
through things.