Draft: Implement watching file paths via oslib.watch
#5068
Conversation
…hing # Conflicts: # runner/package.mill
…provement/fs-watching
|
What are the CPU numbers on your laptop with this change applied? |
I haven't tested it yet, as my repo uses mill 0.12.10. With #5073 I'll be able to test it and report it later. |
I tested the 0.12.x branch, the CPU now hover's around 0.1%-2.5%, not enough to trigger the CPU fan, whilst previously CPU fan was constantly on. Screencast_20250507_200300.webm |
arturaz
force-pushed
the
improvement/fs-watching
branch
from
95c753d to
3e7c22c
Compare
…hing # Conflicts: # build.mill # runner/daemon/package.mill # runner/daemon/src/mill/daemon/MillMain.scala
…hing' into improvement/fs-watching
…hing' into improvement/fs-watching
arturaz
force-pushed
the
improvement/fs-watching
branch
from
a2d3e4b to
73b6b6f
Compare
arturaz
force-pushed
the
improvement/fs-watching
branch
from
3ef16e8 to
1b797e9
Compare
…hing' into improvement/fs-watching
| @@ -97,6 +97,11 @@ case class MillCliConfig( | |||
| doc = """Watch and re-run the given tasks when when their inputs change.""" | |||
| ) | |||
| watch: Flag = Flag(), | |||
| @arg( | |||
| name = "watch-via-fs-notify", | |||
There was a problem hiding this comment.
Let's shorten this to --notify-watch here and in the other PR, the current name is a bit of a mouthful
| private[mill] sealed trait Pollable extends Watchable | ||
|
|
||
| /** A [[Watchable]] that is being watched via a notification system (like inotify). */ | ||
| private[mill] sealed trait Notifiable extends Watchable |
There was a problem hiding this comment.
Do we actually need these two additional marker traits? It seems to me that working directly with Watchable.Path and Watchable.Value will be enough. There aren't any compatibility/extensibility concerns since all this stuff is private[mill] anyway
| * | ||
| * Normally you should use [[validate]] so that types would guide your implementation. | ||
| */ | ||
| def validateAnyWatchable(w: Watchable): Boolean = poll(w) == signature(w) |
There was a problem hiding this comment.
This name is pretty weird; it wasn't great before but if we're going to change it we should use a better one. Maybe def checkForUpdate or something?
| // If I have 'root/a/b/c' | ||
| // | ||
| // Then I want to watch: | ||
| // root/a/b/c | ||
| // root/a/b | ||
| // root/a | ||
| // root |
There was a problem hiding this comment.
This comment is a bit confusing. We're only setting one os.watch.watch on the root, and this makes it sound like we're setting multiple. What we're actually doing is choosing the paths we need to watch recursively in Linux since inotify is non-recursive by default, since changes in any enclosing folder could result in the watched file or folder disappearing (e.g. if the enclosing folder was renamed) and we want to pick up such changes. Not sure how to phrase it in the comment but maybe you can come up with something
Currently watching files uses polling which is inefficient with large codebases. On my laptop
./mill --watchuses 20% CPU just to watch the files for changes.This uses
oslib'swatchmodule to watch for the file changes instead of polling them.We had to update oslib (com-lihaoyi/os-lib#386) to:
outor.bloopemits Java FS watcherOVERFLOWevents.println's thatoslib.watchhad.The current approach to watching is to watch the workspace root via FS watching. FS watching only works with folders and we have to watch
root/build.mill, and thus,root/, so everything else falls under it.Mac OS watcher performs watching recursively natively, but on linux oslib adds recursive watches itself, so we use
oslib.watchfilter to prevent watching anything that is not an ancestor of watched root. For example, if we have source atroot/module-a/src/, we'll watchroot/,root/module-a/androot/.Finally, events are filtered so that unrelated changes (like creating
root/random-file.txt) does not trigger reevaluation.The fs watching is enabled by default and can be disabled via
--watch-via-fs-notify=false.0.12.xversion: #5073