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

The event loop doesn't play nice with other async code #51

Open
Arshia001 opened this issue Mar 4, 2024 · 0 comments
Open

The event loop doesn't play nice with other async code #51

Arshia001 opened this issue Mar 4, 2024 · 0 comments

Comments

@Arshia001
Copy link
Contributor

(That's a bad issue title, but I can't think of a better one, sorry)

There are two main issues with the current implementation of the event loop:

  • As long as there is work to do, it keeps waking up the task, even if the work is a timer many seconds into the future, leading to busy-waiting and wasted CPU cycles.
  • It does't wake up the task when it receives new work to do (e.g. when a promise continuation or future is enqueued as a result of running some code).

The second point is especially painful. Let's assume there's a select! somewhere:

select! {
  _ = do_stuff() => { ... },
  _ = rt.run_event_loop(), if !rt.event_loop_is_empty() => { ... },
}

and do_stuff does:

fn do_stuff() {
  let foo = fetch_something().await;
  let promise = execute_some_js_with_foo(foo);
  let result = PromiseFuture::new(promise).await;

what happens is that do_stuff will run until it creates the PromiseFuture, then wait for the event loop to progress the promise. However, if the event loop was empty, it won't be polled (this is the only sensible thing to do when running JS alongside native code; otherwise, we're always busy-waiting), so now we're stuck waiting for something else to wake the task up (For context, we didn't discover this so far because the WinterJS event loop was a bit of a patchwork implementation, and relied on being interrupted every millisecond or so.)

Here's my commit that fixes this in our fork: e9e2ec5. I've added a few things:

  • the event loop stores a waker, and when something (future, microtask, macrotask) is enqueued, we wake the task up.
  • the macrotask queue now registers timers when it wants to be woken up later.
  • The event loop itself no longer has a concept of "running to completion". Instead, it has a step function that does as much as it can and register wake-up calls for its pending work. There's a new Future type named RunToEnd that keeps stepping the event loop until it's empty.
  • Runtime::run_event_loop now uses RunToEnd internally, but the semantics should be unchanged.
  • The stepping behavior is exposed through Runtime::step_event_loop. You can see how this is useful here: https://github.com/wasmerio/winterjs/blob/main/src/runners/event_loop_stream.rs
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

No branches or pull requests

1 participant