Downgrade to Notify 4, use FSEvents, use minimal recursive watches

[kennethloeffler]

This PR makes a few changes to the file watching component of memofs' StdBackend to address performance problems on macOS. Closes #815 and closes #609. The amount of code churn here is unfortunate, but we haven't had a maintainer who works on macOS until now.

Downgrade to Notify 4

First, I downgraded back to Notify 4 by reverting #816. The reason for this is the fact that Notify 6's debouncer provides incorrect results when using the FSEvents backend. Specifically, FSEvents under some circumstances emits a Create event immediately followed by a Remove event for file deletions [???], which Notify 6's debouncer then collates into zero events, never reporting any changes. Notify 4's debouncer does not have this problem.

Because the raw events can be quite noisy, we do need some form of debouncing to avoid unnecessary patch re-computations. So, we're blocked on upgrading Notify unless memofs implements its own deboucing, or we maintain our own patch until it's fixed upstream.

Use FSEvents backend instead of PollWatcher

Next, I reverted #783. The use of PollWatcher worked around the massive latency described in #609, but incurred prohibitive cost as reported in #815.

Minimize number of watch calls by leveraging Notify's recursive watch mode

I determined that the performance problem in #609 is caused by excessive calls to FsEventWatcher::watch. This method is potentially very expensive, and rojo serve calls it for every file in the project during initialization. It seems to get slower and slower the more it is called, which starts to be problematic once the number of files in the project exceeds about 1000. I profiled rojo serve using a debug build on a project containing 2000 Lua files, and calls to FsEventWatcher::watch dominated the profile, accounting for over 90% of the execution time:

It's possible to avoid this by using Notify's recursive watch mode, and only calling watch on files that aren't already covered by an existing recursive watch. FSEvents is designed to watch an entire hierarchy - in other words, it will report events for any descendant of a watched directory. By using the recursive watch mode, we only have to call watch once per top-level directory, and once per "naked" file, i.e. a file that is directly referred to using $path that is not already a descendant of a watched directory. With this change, calls to FsEventWatcher::watch virtually disappear in the profile:

Takeaways and alternatives?

The main takeaways here are that the underlying file watching APIs are racy, unreliable, and possibly very costly. It's also important to note that changes to memofs are hazardous, and must be tested thoroughly on each of Rojo's main supported platforms.

I looked into using the kqueue backend on macOS (which is far better behaved compared FSEvents), but it's still rather slow, and presents an additional issue: the kqueue backend opens every file it watches and stores the file descriptors, which risks exceeding the operating system's maximum open files limit. While this limit can be increased, I'd rather not have to worry about it all.

[jackTabsCode]

The reason for this is the fact that Notify 6's debouncer provides incorrect results when using the FSEvents backend.

Perhaps we should get in contact with the Notify maintainers rather than just cutting our losses?

[kennethloeffler]

I don't think we materially lose much by sticking with Notify 4 for now, but yes, the problem ought to be raised with Notify's maintainers

[Dekkonot]

Thank you for taking the time to handle this! The codechurn is bad, but I'm glad we're actually solving this problem.

I imagine this also improves performance on Windows and Linux, but that's just a guess.

I do agree with Jack that having to downgrade Notify feels like a loss in general but I'm not terribly concerned about it. While we should report this to the Notify maintainers, I don't want to wait on them to fix it before we can make a new release and as far as I'm aware there's nothing wrong with Notify 4 beyond it being old.

We can revisit an upgrade to Notify later though.