Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tweak: dont enqueue filesystem events #5631

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

mitchellwrosen
Copy link
Member

@mitchellwrosen mitchellwrosen commented Mar 21, 2025

Overview

Our implementation of filesystem event queueing is simple and straightforward (well the code wasn't, somehow): we essentially just buffer all filesystem events (with a little bit of debounce logic), and processes them one at a time, with user inputs interleaved.

This has some undesirable properties, though. When executing a long-running IO action, or a local http microservice, a bunch of file change events can build up (if editing while the thing is running), which then slowly get worked through upon the IO action completing or web server being torn down.

I had gotten in the bad habit of just holding down Ctrl+C when killing a local http microservice, because of the high likelihood that I had been editing a unison file while it was running and had enqueued a bunch of unwanted file change events.

I changed the implementation to basically just remove the queue. (The diff is big because I also cleaned up a ton of complex stuff). The behavior in this PR is roughly as follows: when the main loop is ready to wait for either an event or a user input, it does so with a freshly-emptied filesystem event queue. I kept the existing debounce logic we already had, which is sort of a two-layered thing: only emit a filesystem event after 50ms elapse without an event (to avoid reacting to the flurry of writes that modern editors typically make), and also don't emit events for files that haven't changed, if the events are within 500ms of each other (I guess this is to save a little CPU?)

Test coverage

I tested this manually at the prompt, it seems to work.

Loose ends

The user experience of editing a large .u file that takes multiple seconds to typecheck could perhaps be further improved.

The trunk behavior is to enqueue one slow typecheck after another, for every edit the user makes, which obviously isn't very nice. This PR's behavior is to basically ignore user edits that are made while a slow type check is in progress. We might instead want to interrupt a typecheck on file change, so that we don't waste time typechecking an old thing, and can just start over with the latest contents.

@mitchellwrosen mitchellwrosen changed the title tweak: dont enqueue events tweak: dont enqueue filesystem events Mar 21, 2025
@mitchellwrosen mitchellwrosen marked this pull request as ready for review March 21, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant