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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, May 31, 2017 at 3:37 PM, Ben Peart <peartben@gmail.com> wrote:
> 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.

I think where this series is going for now makes perfect sense.
Looking forward to the next version, need to get watchman compiling
though.

I think what Christian may be getting at here (correct me if I'm
wrong) is that David had a series 1-2 years ago that went a bit beyond
what this is doing, i.e. it had a persistent index-helper daemon
talking to watchman, so "git status" would stream the index state over
from this daemon, and it would consult watchman.

That's an interesting *additional* optimization, but a huge change on
top of this, so it makes perfect sense to start with something simpler
as this series does, and my hunch from having tested/looked at both is
that just talking to watchman is going to be by far the biggest win.