From: Ben Peart Date: Mon, 10 Jul 2017 14:40:15 GMT Subject: Re: [PATCH v5 0/7] Fast git status via a file system watcher Message-ID: <97342e22-c2ca-9c09-4db0-24cac014b689@gmail.com> In-Reply-To: <9b1af00c-2451-501c-5315-7ed2b81f3f18@gmail.com> On 7/10/2017 9:36 AM, Ben Peart wrote: > > > On 6/28/2017 1:11 AM, Christian Couder wrote: >> On Sat, Jun 10, 2017 at 3:40 PM, Ben Peart 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! > >