spiderfire icon indicating copy to clipboard operation
spiderfire copied to clipboard

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

Open Arshia001 opened this issue 5 months ago • 0 comments

(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: e9e2ec55a005317051334311ee67b052288f9824. 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

Arshia001 avatar Mar 04 '24 11:03 Arshia001