Re: [PATCH 1/4] fsmonitor: use fsmonitor data in `git diff`

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> Hmm. I do agree that I'd like to stay out of the business of trying to
> figure out exactly what that trade-off is (although I'm sure that it
> exists), only because it seems likely to vary to a large extent from
> repository to repository. (That is, 20% may be a good number for some
> repository, but a terrible choice for another).

I think both of you misunderstood me.  

My question was a simple yes/no "does there a trade off exist?"
question and the sentences with 20% in it were mere example of
possible trade-off I had in mind that _could_ exist.  I wasn't even
suggesting to figure out what the optimum cut-off heuristics would
be (e.g. solving "when more than N% paths are subject to diff
fsmonitor is faster" for N).

I was hoping that we can show that even having to lstat just a
single path is expensive enough---IOW, "there is no trade-off worth
worrying about, because talking to fsmonitor is so cheap compared to
the cost of even a single lstst" would have been a valid and happy
answer.  With such a number, there is no risk of introducing an
unwarranted performance regression to use cases that we did not
anticipate by adding an unconditional call to refresh_fsmonitor().

But without any rationale, the performance implication of adding an
unconditional call to refresh_fsmonitor() would become much muddier.

> But, I think that we can invoke watchman better here; the
> fsmonitor-watchman hook has no notion of a "pathspec", so every query
> just asks for everything that isn't in '$GIT_DIR'. Is there anything
> preventing us from taking an optional pathspec and building up a more
> targeted query?

Yup, it is what I had in mind when I brought up the pathspec.  It
may be something worth pursuing longer term, but not within the
scope of this patch.

> There is some overhead to invoke the hook and talk to watchman, but
> I'd expect that to be dwarfed by not having to issue O(# files)
> syscalls.

"invoke the hook"---is that a pipe+fork+exec, or something else that
is far lighter-weight?

n