Implement kqueue-based FileWatcher for MacOS#7
Conversation
dom96
left a comment
dom96
left a comment
There was a problem hiding this comment.
Only two compilation errors, nice.
src/workerd/server/workerd.c++
Outdated
There was a problem hiding this comment.
| change.ident = fd; | |
| change.ident = fd.get(); |
src/workerd/server/workerd.c++
Outdated
There was a problem hiding this comment.
| change.fflags = NOTE_WRITE | NOTE_EXTEND | NOTE_DELETE | NODE_RENAME; | |
| change.fflags = NOTE_WRITE | NOTE_EXTEND | NOTE_DELETE | NOTE_RENAME; |
|
As for runtime: It notices changes to both my capnp config and the embedded .js file. But then the server locks up (doesn't respond to requests) and I guess it should eventually output a message stating that it successfully reloaded. |
src/workerd/server/workerd.c++
Outdated
There was a problem hiding this comment.
Found the issue, you want this here:
| change.flags = EV_ADD; | |
| change.flags = EV_ADD | EV_ONESHOT; |
Then it works perfectly 🎉
There was a problem hiding this comment.
I think that will break the mechanism that watches for additional changes within 500ms. I think we actually want EV_CLEAR instead of EV_ONESHOT, can you try that?
The kqueue implementation will benefit from using the already-open FD.
This logic will be common to all platforms so it really shouldn't live inside FileWatcher. It's also much cleaner using coroutines, yay.
kentonv
force-pushed
the
kenton/kqueue-watch
branch
from
55c3e3d to
318aec9
Compare
|
(just a rebase, now making changes) |
kentonv
force-pushed
the
kenton/kqueue-watch
branch
from
318aec9 to
fbabdb4
Compare
|
Updated. I also realized that setting close-on-exec was really important here (since we exec() a new process on change) so added that. This has also been rebased on master so has all the other macos fixes. @dom96 mind testing again? |
| // Class which uses inotify to watch a set of files and alert when they change. | ||
| // | ||
| // This version uses kqueue to watch for changes in files. kqueue typically doesn't scale well | ||
| // to watching whole directory trees, since it must keep a file descriptor opne for each watched |
There was a problem hiding this comment.
| // to watching whole directory trees, since it must keep a file descriptor opne for each watched | |
| // to watching whole directory trees, since it must keep a file descriptor open for each watched |
Just a nit, feel free to ignore
There was a problem hiding this comment.
GitHub at least allows me to click "commit suggestion" but what I really want it to offer is "apply fixup to source commit", oh well.
|
Re-tested. Works well :) |
|
Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
3 participants
WARNING: THE LAST COMMIT WAS CODED BLIND. I don't have a Mac. If someone could kindly try it out and fix my syntax errors, that would be greatly appreciated!
Assuming it works, you should find that when running
workerd serve --watch, the process restarts itself (and logs some messages about this) whenever the input config changes (including any files imported/embedded by the config) or the binary itself changes. It should delay for half a second after the change before actually restarting. If any more file changes are detected in that time, the half-second timeout resets, so if you continuously modify a file thenworkerdshould not restart until you stop modifying it. Again, the textworkerdlogs to the console should be enough to tell you if it's working.