N-API: no ability to schedule work on different thread? (via uv_async_t)

[gpean]

Correct me if I'm wrong, but I didn't see a way to schedule work on a different thread using N-API, and/or a way to notify the main loop that some work is done from some other thread and that we need to wake up with a callback call (via uv_async_t and a persistent callback).

I think that's a major use case of native modules (doing stuff in // of the JS thread), so it's kind of surprising. What was the rationale during N-API design?

I was able to hack something using v8.h and uv.h, of course. But it kinda defeats the purpose of having a portable API.

[mscdex]

/cc @nodejs/n-api

[gpean]

(AFAIK napi_create_async_work allows to queue uv work to the default loop - it does not really offload actual work to a separate thread - it just fills the JS thread with more work, although it's native work)

[mscdex]

Looking at the code now, napi_create_async_work()/napi_queue_async_work() utilize uv_work_t and uv_queue_work() behind the scenes. uv_queue_work() enqueues a task to the libuv thread pool, so the napi_async_execute_callback value you pass to napi_create_async_work() will be executed in a separate thread when a thread becomes available. The napi_async_complete_callback value will be called from the main thread after the work thread callback is finished.

[jasongin]

At this time, it is not a goal for N-API to cover all of the functionality offered by libuv. We're certainly open to feedback about this; it's possible we don't have quite the right balance.

We (the N-API working group) discussed extensively how much of libuv should be covered, if any at all. Since libuv is a C API and has infrequent major-version updates, it is already a very stable API (unlike V8 which tends to make breaking changes often). And any forks of Node are likely to still use libuv even if they swap out the JS engine. So, there is clearly less need for the N-API abstraction there.

What we settled on for now was to just include basic async functionality in N-API, because it can be a little tricky to get right without some higher-level abstractions. The N-API async APIs (and the Napi::AsyncWorker class that uses those APIs) are meant to provide a migration path from NAN's AsyncWorker. The NAN APIs also do not allow scheduling work on a different thread. We are assuming that the functionality offered by NAN is good enough for most native addons. Addons that need more advanced threading capabilities can use the libuv APIs directly, as they always have.

I was able to hack something using v8.h and uv.h, of course. But it kinda defeats the purpose of having a portable API.

Why did you need v8.h? As explained above, the expectation is that you should be able to mix in just libuv APIs for that purpose, while still using N-API instead of V8 for everything else.

[mscdex]

The NAN APIs also do not allow scheduling work on a different thread.

Doesn't Nan::AsyncQueueWorker do this?

[jasongin]

Nan::AsyncQueueWorker() uses the uv default loop. napi_queue_async_work() does also.

[mscdex]

Ok, I guess I'm confused about what's being asked then. I thought @gpean was looking to do work on another thread, which those methods do (in the libuv thread pool).

[jasongin]

I think the issue is that Node also uses the uv default loop as its main event loop.

[gpean]

Looking at the code now, napi_create_async_work()/napi_queue_async_work() utilize uv_work_t and uv_queue_work() behind the scenes. uv_queue_work() enqueues a task to the libuv thread pool, so the napi_async_execute_callback value you pass to napi_create_async_work() will be executed in a separate thread when a thread becomes available.

I was not very familiar with libuv and the builtin thread pool you mention - I just read http://docs.libuv.org/en/v1.x/threadpool.html. One issue I see is that you cannot control the pool size other than via an environment variable? It also still competes with the JS work and operations, since this is the same loop. But, I agree it covers most of the use cases for async work.

There can still be use cases where a native library you want to integrate already has its threading done, or you don't want to use the libuv thread pool, and you might just want to notify JS when stuff is available, using uv_async_t. Not the majority of use cases, but still a valid and broad use case IMO.

RE why you need to include v8.h: you won't be able to get a napi_env from the uv_async callback- You have to use v8 API to get the current Isolate, and cast that into a napi_env, basically. We would need a napi_get_current_env or similar?

[gpean]

Said otherwise, the issue I see is that, napi_create_reference allows to persist a value, and I can use that to persist the JS callback function that the uv_async callback should call later. But, it's not possible persist the napi_env alongside with the persisted JS callback, and there is no function to get the current env (isolate), either.

[mhdawson]

@gpean is the need to schedule work on a separate thread for an existing module or something new ?

[mhdawson]

Adding to @jasongin comments we only included coverage for the use case provided by AsyncWorker as it seemed to be broadly used in existing modules. If there is another pattern that is also broadly used in the existing modules we can take a look. Best way to start would be a list of modules that use the additional pattern.

[gpean]

@mhdawson it's for new stuff

I propose that we focus on the following, non major, actionable issues:

• No way to get current isolate/current napi_env (see my last comment) prevents extending functionality through libuv

• No way to use napi_queue_async_work with something else than the default uv loop prevents using the existing async API really asynchronously from JS initiated work (so, not so async)

[jasongin]

No way to get current isolate/current napi_env (see my last comment) prevents extending functionality through libuv

You can stash the napi_env in the data field of uv_work_t, either directly or as part of a context structure you might already need for other purposes. That uv_work_t is passed back to the complete callback, where you would then use it when invoking a JS callback. So, I don't think you're actually blocked.

We had a napi_get_current_env() API for a time, but we removed it because we were concerned we might have to deprecate it eventually, because use of v8::Isolate::GetCurrent() is discouraged within Node now and may be deprecated later. And we didn't see any scenarios where it is really needed; as explained above, there should always be some way to pass the napi_env around as part of some context.

[jasongin]

use v8 API to get the current Isolate, and cast that into a napi_env

Don't do this! A napi_env is more than just a v8::Isolate. While that cast might work for some things because a v8::Isolate happens to be the first field in the structure, you shouldn't rely on that internal implementation detail, and it will lead to crashes as soon as access to some other field is attempted, such as when getting the last error info.

[gpean]

You can stash the napi_env in the data field of uv_work_t

You probably mean the uv_async_t data field.
I thought it was "unsafe" to keep napi_env's around? Can I really just use the same isolate to perform the callback call from the uv_async callback? Is it always safe to reuse an isolate from one context to another?

& What about this statement from the doc:

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

?

Don't do this! A napi_env is more than just a v8::Isolate

I know, I copied pasted the __napi_env struct decl from node_api.cpp to do that "safely". Casting was just a language shortcut. I'm adventurous but not crazy :-)

[sam-github]

No way to use napi_queue_async_work with something else than the default uv loop prevents using the existing async API really asynchronously from JS initiated work (so, not so async)

Please elaborate on how is this "not so async". Its what all the async operations in node do, so its as async as fs, dns.loopup, etc. Its possible to create your own uv loop, and there are a very, very few occaisons when it is necessary. execSync and friends do it, for example, so they can use uv to do the exec, but they use a different loop so as to prevent any concurrent processing or progress on the main loop. Inspector does it as well, because it needs a way to interact with a remote debugger when the main thread is blocked in the debugger. Its pretty rare, and note that both of them involve wanting to do I/O while causing the default loop to be completely blocked.

[gpean]

Please elaborate on how is this "not so async"

You justly mentioned two use cases where blocking syscalls, or libc or other system library calls, or potentially long computations can impact the main loop thread pool availability and that for this reason, you create a separate loop/pool for them. This is the "not so async" i'm talking about - Having everybody on the same loop means potential resource fighting (and so, implicit synchronization) for an available slot on the (fixed 4 threads wide!) thread pool, which we could make it possible to avoid easily in N-API by just letting people be able to request a new thread pool.

[kkoopa]

V8 isolates have completely separate states. Objects from one isolate must not be used in other isolates.

That is, you cannot call a persisted Function with another Isolate than its owner -- which implies that you must cache the Isolate in a libuv callback which calls back to JavaScript.

[gpean]

@kkoopa isn't that what napi_get_reference_value is for? getting a valid value (persisted function instance) from another isolate (napi_env)?

[gpean]

(But, I get the point, I'm happy to use the initial isolate to perform the call back- Makes things simpler!)

So, effectively, one issue remains: that you're forcing N-API users to use 4 threads to perform async work which can be potentially already used by Javascript IO work. When you have a 32 cores server, I'd think you might want to make it scale better- if you don't want to expose loop control, at least the default pool size should match the vcpu count.

[kkoopa]

Without looking up the function in question: No, that is not how it works. IIRC, napi_get_reference_value is the equivalent of v8::Local<T>(v8::Persistent<T>), which converts a Persistent to a Local. IIRC, this conversion does not even take or use an Isolate. The napi variant might use one for some reason, maybe it was for setting error codes. Persistent (reference) and Local (value) are handles (actually pointers in structs) to v8::Values. When you do v8::Function::Call on the handle, you pass the Isolate so the handle can be dereferenced and operated on correctly.

[gpean]

@kkoopa how does it work wrt reference tracking though? how do I guarantee that my JS function value won't have been collected, by the time my uv_async callback fires? I still need the reference creation to have things protected from GC no?

[gpean]

Also. I need to create a HandleScope in the uv_async_t callback right? If I want to create result values to pass to my JS callback. That's a v8 API not surfaced via N-API. Or am I missing something?

EDIT: yeah, visibly I missed napi_handle_scope!

[kkoopa]

A v8::Persistent is not reference tracked (unless made weak). It always exists. When you take a v8::Local<v8::Function> and make a v8::Persistent<v8::Function>, the Persistent lives until you call v8::Persistent::Reset. On the other hand, a v8::Local<v8::Function> is destroyed when its v8::HandleScope is destroyed. Then this reference is gone and the v8::Function (which is the JavaScript function) might be garbage collected if there are no other live references to it.

void foo(v8::FunctionCallbackInfo<v8::Value> info) {
v8::HandleScope scope;

v8::Local<v8::Function> my_callback = info[0].As<v8::Function>();

schedule_async_stuff(my_callback);
} //<--- All (local) C++ variables, HandleScope dies, along with the local handle, now there might be a risk of garbage collection

void foo(v8::FunctionCallbackInfo<v8::Value> info) {
v8::HandleScope scope;

v8::Local<v8::Function> my_callback = info[0].As<v8::Function>();

v8::Persistent<v8::Function> leaky_reference(info.GetIsolate(), my_callback);
schedule_async_stuff(my_callback);
}  //<-- There is a persistent reference stored to the function referenced by my_callback, so the function lives. However, the v8::Persistent is destroyed by C++ scoping rules, but its underlying storage cell in V8 is not, now you have no way of clearing up the storage cell

[kkoopa]

Yes you must have a scope in libuv callbacks, since they are outside Node (which sets up a scope before going from V8 to a C++ callback in an addon). There is both a napi_handle_scope and napi_escapable_handle_scope or something similarly named.

[gpean]

Thanks. At least I can remove the v8.h dependency. w00t!

[kkoopa]

To clear things up a bit further: A v8::Local<T> is similar to a T**. A v8::Persistent<T> is also similar to a T**, but the T* (T** derferenced once, which is the "actual JavaScript reference") is stored outside the v8::Persistent<T>. Thus, when a v8::Local<T> goes out of C++ scope, the T** also does. When the surrounding v8::HandleScope goes out of C++ scope, the T* dies. When a v8::Persistent<T> goes out of C++ scope, the T** also does, but now the T* is stored in a "global storage cell" for the v8::Isolate and lives on until you explicitly get rid of it by calling v8::Persistent<T>::Reset().

[ebickle]

@jasongin

Can you please clarify the comments you made last June regarding the lack of multi-threading support in NAN and N-API?

The NAN APIs also do not allow scheduling work on a different thread. We are assuming that the functionality offered by NAN is good enough for most native addons.

The original author of the issue was questioning whether napi_create_async_work, and therefore uv_queue_work scheduled work on a separate thread. The impression given is that the answer is no, but that seems to contradict the official libuv documentation:

http://docs.libuv.org/en/v1.x/guide/threads.html#id1

uv_queue_work() is a convenience function that allows an application to run a task in a separate thread.

My understanding based on the documentation and source is that create_async_work will run a task in a separate thread. It's true the main node.js event loop is used, but that appears to be for the event signalling (i.e. results) not for the actual processing itself.

uv_queue_work is defined right inside of threadpool.c.. In the logic for uv_queue_work, line 287, it's placing the work onto the uv__queue_work worker queue and not the event loop.

While I may be interpreting this wrong, both the source and official libuv documentation indicate separate threads are used. So why did you say this isn't the case for N-API?

[bnoordhuis]

@ebickle "different thread" in this context means starting a new thread. Nan and n-api use existing threads.

[ebickle]

@bnoordhuis I'm still not seeing that in the source.

https://github.com/nodejs/node/blob/master/src/node_api.cc#L2871
CALL_UV(env, uv_queue_work(event_loop, w->Request(), uvimpl::Work::ExecuteCallback, uvimpl::Work::CompleteCallback));

http://docs.libuv.org/en/v1.x/threadpool.html

When a particular function makes use of the threadpool (i.e. when using uv_queue_work()) libuv preallocates and initializes the maximum number of threads allowed by UV_THREADPOOL_SIZE.

int uv_queue_work(uv_loop_t* loop, uv_work_t* req, uv_work_cb work_cb, uv_after_work_cb after_work_cb)
Initializes a work request which will run the given work_cb in a thread from the threadpool. Once work_cb is completed, after_work_cb will be called on the loop thread.

Documentation is unambiguous. uv_queue_work runs the work on one of the threads (multiple threads) in the threadpool and the result is then synchronized back onto the main event loop.

https://nodejs.org/en/docs/guides/dont-block-the-event-loop/

Node's Worker Pool is implemented in libuv (docs), which exposes a general task submission API.
Node uses the Worker Pool to handle "expensive" tasks. This includes I/O for which an operating system does not provide a non-blocking version, as well as particularly CPU-intensive tasks.

We use the Node.js OracleDB driver extensively and tuned the number of worker threads it uses via UV_THREADPOOL_SIZE since we're DB I/O heavy. From their source:
if (uv_queue_work(uv_default_loop(), &req, AsyncWorkCallback, AsyncAfterWorkCallback)) {

Not much different from N-API:
CALL_UV(env, uv_queue_work(event_loop, w->Request(), uvimpl::Work::ExecuteCallback, uvimpl::Work::CompleteCallback));

I'm not seeing any logic inside of n-api that's forcing a single threaded worker pool? What am I missing? :)

[bnoordhuis]

Okay, let me try again. Read "different thread" as "new standalone thread made with uv_thread_create()". Nan and n-api don't do that, they use the thread pool.