Semantical difference between Napi::ThreadSafeFunction and napi_threadsafe_function

[legendecas]

Napi::ThreadSafeFunction now acts as a bridge between the JavaScript function callback and multiple runtime c++ function callbacks. That is to say, Napi::ThreadSafeFunction has built a 1:m relation on the JavaScript function and c++ function semantically. See call functions of Napi::ThreadSafeFunction and their variants:

napi_status Napi::ThreadSafeFunction::BlockingCall(DataType* data, Callback callback) const
napi_status Napi::ThreadSafeFunction::NonBlockingCall(DataType* data, Callback callback) const

With these signatures we could provide different c++ functions on each thread safe function call. It might be more convenient to be used. Yet while reviewing napi_threadsafe_function docs, we could find that napi_threadsafe_function has a 1:1 relation on JavaScript function and c++ function which is built on the creation function signature:

napi_status
napi_create_threadsafe_function(napi_env env,
                                napi_value func,
                                napi_value async_resource,
                                napi_value async_resource_name,
                                size_t max_queue_size,
                                size_t initial_thread_count,
                                void* thread_finalize_data,
                                napi_finalize thread_finalize_cb,
                                void* context,
                                napi_threadsafe_function_call_js call_js_cb,
                                napi_threadsafe_function* result);
napi_status
napi_call_threadsafe_function(napi_threadsafe_function func,
                              void* data,
                              napi_threadsafe_function_call_mode is_blocking);
/** no function provided on `napi_call_threadsafe_function` */

Either this behavior was intended or not, this may cause confusion on introducing Napi::ThreadSafeFunction to an existing N-API document reader. They may have the impression that a single thread safe function is going to be used for one purpose, and has 1:1 relation on JavaScript function and the c++ function.

Possible solutions (might not be the best solutions):

• Add a caveat section on the doc of Napi::ThreadSafeFunction
• Breaking change (Nah, nope, not this one)

Or it may not be a problem at all.

[KevinEady]

Hi @legendecas ,

Yes, this is a good point. It was briefly mentioned here #556 (comment) and discussed in the subsequent napi meeting, and thus spawning #580.

I personally vote for your first solution "Add a caveat section on the doc of Napi::ThreadSafeFunction" because when #580 lands, you can use the tsfn in 1:1 manner. It's just that the "default behavior" is this wrapped thing.

[legendecas]

Good to see it's already progressing here.

Yet there is still no constructors for creating Napi::ThreadSafeFunction in the manner of napi_create_threadsafe_function. Also, the test in #580 just calls napi_create_threadsafe_function directly to get aside from the problem. Which might not be total Napi way :D

[KevinEady]

Ahhh yes I understand that concern now. Considering that using the current ::New always uses this CallJS wrapper, there may be performance implications.

[gabrielschulhof]

After discussing this with the N-API team, it looks like we may need to remove this functionality from Napi::ThreadSafeFunction because it incurs too high of a performance penalty. Users can still branch to different callbacks for any given data item based on information they choose to record into such an item, but such logic should not be forced on those who do not need it.

Removal per se is not an option for backward compatibility reasons. Thus, we need to figure a way forward so as not to break existing users, while providing as smooth a path away from these shortcomings for those who need it.

[KevinEady]

After discussing this with the N-API team, it looks like we may need to remove this functionality from Napi::ThreadSafeFunction because it incurs too high of a performance penalty.

Regarding the "performance penalty"... I'm not sure if it's possible to add some type-safety / type-convenience without doing some sort of wrapper around the napi_create_threadsafe_function call_js_cb parameter, which takes signature void(napi_env env, napi_value js_callback, void *context, void *data)

I'm working on the new TSFN API and I'm not sure if a wrap would be allowed here (similar to how we have a details::FinalizeData<T, Finalizer> wrapper for type convenience) , as this wrap would be done in every tsfn call, which I guess is the performance problem. Hopefully we can discuss in next meeting

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[KevinEady]

I believe this is fixed by #742 @legendecas , @mhdawson , @gabrielschulhof