Re: [PATCH v5 0/7] Fast git status via a file system watcher
On 7/10/2017 9:36 AM, Ben Peart wrote:
Show 56 quoted lines
>
>
> On 6/28/2017 1:11 AM, Christian Couder wrote:
>> On Sat, Jun 10, 2017 at 3:40 PM, Ben Peart <peartben@gmail.com> wrote:
>>> Changes from V4 include:
>> ...
>>
>> I took a look at this patch series except the last patch ([PATCH v5
>> 7/7] fsmonitor: add a performance test) as Junio reviewed it already,
>> and had only a few comments on patches 3/7 and 4/7.
>>
>> I am still not convinced by the discussions following v2
>> (http://public-inbox.org/git/20170518201333.13088-1-benpeart@microsoft.com/)
>>
>> about using a hook instead of for example a "core.fsmonitorcommand".
>>
>> I think using a hook is not necessary and might not be a good match
>> for later optimizations. For example people might want to use a
>> library or some OS specific system calls to do what the hook does.
>>
>> AEvar previously reported some not so great performance numbers on
>> some big Booking.com boxes with a big monorepo and it seems that using
>> taskset for example to make sure that the hook is run on the same CPU
>> improves these numbers significantly. So avoiding to run a separate
>> process can be important in some cases.
>>
>
> Using a hook is the only pattern I've seen in git that provides a way to
> enable OS specific calls. I used a hook so that different file
> monitoring services could be plugged in depending on the OS or tools
> available. The Watchman integration script was mostly intended as a
> sample that could be used where Watchman is available and works well.
>
> I had not heard about the taskset issues you mention above. If there is
> something else that can be done to make this work better, please let me
> know the details.
>
> If I'm understanding you correctly, you are suggesting that someone
> should be able to configure a setting (core.fsmonitorcommand) that gives
> a custom command line that would be run instead of running the
> query-fsmonitor hook.
>
> I'm not entirely sure how that should work. There are command line
> options that need to be passed (currently the interface version as well
> as the current clock in nanoseconds). How would those passed when using
> the custom command?
>
> Is it OK to just append them to the given command line? Does there need
> to be some substitution token to indicate where they should be inserted
> (ie "mycustomcommand --custom --options %version% --more-options
> %timestamp%"). Are there any other tokens that should be supported (ie
> PID or processor mask?).
>
> Is there a design pattern already used somewhere in git that I can
> follow or is this all blazing a new trail?
>
My co-worker reminded me about git difftool - is this what you had in mind?
> Thanks for continuing to look into this. Feedback is good!
>
>