signal-neon-futures: Poll a future once before queuing it

[jrose-signal]

Previously, when using the JavaScript context to run a Rust future, the future would be put into the event queue immediately, without starting it first. The new implementation runs the future synchronously to its first suspension point. There are pros and cons to each approach (and I left the "delayed start" version behind as queue_future), but in practice we're not worried about, say, blocking other work behind this one.

[jrose-signal]

@kjvalencik, you might be interested in this for future Neon API design. @indutny-signal found that the Node / uv event queue is actually fairly slow to tick through a loop, so I'm going through my futures bridge and JS-based executor and seeing which sends I can pull out without too much trouble. Some of them seem inescapable, like calling into JS from a bit of Rust code that doesn't have a Neon Context around (because async implies 'static lifetimes), but others might not be.

I've mentioned LocalEventQueue in the past as a non-Send EventQueue, but what would be really nice is a non-Send 'static type derivable from Context (or even limited to FunctionContext) that lets you synchronously access the JS environment, relying on the non-Send-ness to keep it working as long as you haven't shut down Node. I don't know enough of the details of N-API to know if that's really possible, but it would help a lot with async callbacks.

[jrose-signal]

This was my mistake; we're using Node's event queue rather than JavaScript's microtask queue.

[indutny-signal]

No one calls queue_task and queue_future, right?

[jrose-signal]

Right; they're there for "completeness" but I could just drop them.

[indutny-signal]

I wonder if we can actually continue to run future without rescheduling it on the queue if we are already on the queue. All microtasks should complete when the JS call returns so from the perspective of JS it will look like different event loop ticks.

This might happen in cases where the JS callback returns resolved promise.

[kjvalencik]

@jrose-signal That's interesting! Do you have any detail on which part of it is slow? There's a fair amount of coordination in the napi_threadsafe_function implementation. It registers a callback with libuv, creates a concurrent queue and pushes/pops from the queue. I'm curious if it's the N-API implementation that's slow or if Node is taking a long time to get to the callback.

I'm also not sure how callbacks in libuv get scheduled. It might be possible to starve one queue with another.

I don't think I fully understand what you mean by a LocalEventQueue. It kind of sounds like putting Context is thread local storage.

[jrose-signal]

I wonder if we can actually continue to run future without rescheduling it on the queue if we are already on the queue. All microtasks should complete when the JS call returns so from the perspective of JS it will look like different event loop ticks.

Yeah, even though this isn't okay generally (because of re-entrancy), we know that Promise.then always runs its callback as a distinct microtask, which means we don't have to worry about re-entrancy. Maybe a thread-local "it's okay to run synchronously" flag is the way to go for now—it's clunky, but should work fine.

[jrose-signal]

@jrose-signal That's interesting! Do you have any detail on which part of it is slow? There's a fair amount of coordination in the napi_threadsafe_function implementation. It registers a callback with libuv, creates a concurrent queue and pushes/pops from the queue. I'm curious if it's the N-API implementation that's slow or if Node is taking a long time to get to the callback.

Yeah, I hadn't realized creating a queue was itself expensive, but Fedor that out as well. That'll be my immediate next step: cut down on cx.queue() calls. But we don't think the expensive part is Neon, just libuv deciding to process all its input sources before getting to the task you just enqueued. When trying to do a bunch of bulk tasks, the number of yields adds up even if each one is tiny. (@indutny-signal, anything to add?)

I don't think I fully understand what you mean by a LocalEventQueue. It kind of sounds like putting Context is thread local storage.

The original thing I meant by it was just "EventQueue without Send so that you could put non-Send callbacks on it". In this case I was under the misapprehension that napi_env was like JNI's JNIEnv—something that didn't change from callback to callback—and that as long as you were within some callback it'd be okay to access Node. But N-API documentation explicitly says

Caching the napi_env for the purpose of general reuse […] is not allowed.

So maybe it would come down to a thread-local Context after all. (Which is what the original version of this PR used, sort of, and it felt a lot more hacky.)

[jrose-signal]

Closing in favor of #290, which is a more drastic change that incorporates this.