Thread (214 messages) flat view 214 messages, 10 authors, 2021-07-26

Re: [PATCH v3 00/34] Builtin FSMonitor Feature

From: Johannes Schindelin <hidden>
Date: 2021-07-05 21:35:58

Hi Ævar,

On Thu, 1 Jul 2021, Ævar Arnfjörð Bjarmason wrote:
On Thu, Jul 01 2021, Jeff Hostetler wrote:
quoted
On 7/1/21 1:40 PM, Ævar Arnfjörð Bjarmason wrote:
quoted
Any other testing of it is stalled by there being no linux backend
for it as part of this series. I see from spelunking repos that
Johannes had a WIP compat/fsmonitor/linux.c which looks like it
could/should mostly work, but the API names all changed since then,
and after a short try I gave up on trying to rebase it.
I am a bit surprised that you gave up so easily, it cannot be that hard if
you use `git rebase -i` smartly.

But I think there is an even bigger obstacle lurking than just the
challenge of rebasing that experimental backend.
quoted
The early Linux version was dropped because inotify does not give
recursive coverage -- only the requested directory.  Using inotify
requires adding a watch to each subdirectory (recursively) in the
worktree.  There's a system limit on the maximum number of watched
directories (defaults to 8K IIRC) and that limit is system-wide.

Since the whole point was to support large very large repos, using
inotify was a non-starter, so I removed the Linux version from our
patch series.  For example, the first repo I tried it on (outside of
the test suite) had 25K subdirectories.

I'm told there is a new fanotify API in recent Linux kernels that is a
better fit for what we need, but we haven't had time to investigate it
yet.
That default limit is a bit annoying, but I don't see how it's a blocker
in any way.
Let me help you to see it.

So let's assume that you start FSMonitor-enabled Git, with the default
values. What is going to happen if you have any decently-sized worktree?
You run out of handles. What then? Throw your hands in the air? Stop
working? Report incorrect results?

Those are real design challenges, and together with the race problems Jeff
mentioned, they pose a much bigger obstacle than the rebasing you
mentioned above.
You simply adjust the limit. E.g. I deployed and tested the hook version
of inotify (using watchman) in a sizable development environment, and
written my own code using the API. This was all before fanotify(7)
existed. IIRC I set most of the limits to 2^24 or 2^20. I've used it
with really large Git repos, including with David Turner's
2015-04-03-1M-git for testing (`git ls-files | wc -l` on that is around
a quarter-million).

If you have a humongous repository and don't have root on your own
machine you're out of luck. But I think most people who'd use this are
either using their own laptop, or e.g. in a corporate setting where
administrators would tweak the sysctl limits given the performance
advantages (as I did).
This conjecture that most people who'd use this are using their own laptop
or have a corporate setting where administrators would tweak the sysctl
limits according to engineers' wishes strikes me as totally made up from
thin air, nothing else.

In other words, I find it an incredibly unconvincing argument.

I prefer not to address the rest of your mail, as I found it not only a
lengthy tangent (basically trying to talk Jeff into adding Linux support
in what could have been a much shorter mail), but actively distracting
from the already long patch series Jeff presented. It is so long, in fact,
that we had to put in an exemption in GitGitGadget because it is already
longer than a usually-unreasonable 30 patches. Also, at this point,
insisting on Linux support (in so many words) is unhelpful.

Let me summarize why I think this is unhelpful: In Git, it is our
tradition to develop incrementally, for better or worse. Jeff's effort
brought us to a point where we already have Windows and macOS support,
i.e. support for the most prevalent development platforms (see e.g.
https://insights.stackoverflow.com/survey/2020#technology-developers-primary-operating-systems).
We already established multiple obstacles for Linux support, therefore
demanding Linux support to be included Right Now would increase the patch
series even further, making it even less reviewable, being even less
incremental, hold up the current known-to-work-well state, force Jeff to
work on something he probably cannot work on right now, and therefore
delaying the entire effort even further.

Ciao,
Johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help