Feat/libjs

[telamon]

moved to libjs and napi-compat

[kasperisager]

We're mostly missing microtask checkpoints in I/O callbacks so quite a few repeated comments to hopefully catch all the cases.

[telamon]

@kasperisager

We're mostly missing microtask checkpoints in I/O callbacks so quite a few repeated comments to hopefully catch all the cases.

I'm a little bit upset about the js calls with checkpoints still, because I originally just ported previous calls that used checkpoints (or rather all calls were checkpointed before since most run on I/O) but that blew up..

Is my understanding correct?

• Scenario A) JS -> native_func() -> js_call_function(); // js already executing, checkpoint forbidden

• Scenario B) on uv_async|uv_poll|(any non-js entry points incl uv_timers) -> native_func() -> js_call_function_with_checkpoint() // checkpoint required

Also in scenario B; if any recoverable exceptions occurs during js-call;

Then after native-recovery branch taken; the exception must be cleared and forwarded to uncaught handler to avoid process exit

[kasperisager]

That's almost correct, yes, with the exception of the last bit. Exceptions aren't allowed to escape to the outermost call site where no JavaScript remains to handle them. For this reason, by the time the error code reaches the outermost call site, the uncaught exception handler will have already been invoked. This is the distinction between the two possible libjs error codes, js_pending_exception and js_uncaught_exception. Only pending exceptions can be handled and there will never be exceptions pending by the time the outermost call site returns.

Imagine calling into libjs directly from main(): Any function you call will automatically perform a microtask checkpoint and invoke the uncaught exception handler if necessary. There's no risk of forgetting to perform a microtask checkpoint or let an exception go unhandled.

[telamon]

Ah! I see, yes that makes sense

[telamon]

@kasperisager what is the best course of action here?
tl;dr on_udx_stream_remote_changed can be called both by JS and I/O.

rewrite to an intermediary callback that uv_async_send() real callback?

[kasperisager]

@mafintosh Anything we can do on the libudx side to avoid this? Having a callback where we can't reason about the state of the JavaScript execution stack makes Node-API compatibility difficult.

[telamon]

fixed via holepunchto/libudx#243 + 6f61b85

Now libUDX's udx_stream_change_remote(..., callback) only every calls the callback from I/O.

If change is possible immediately the udx function returns 1;
In which case we invoke the remote-changed callback without a checkpoint.

[kasperisager]

Has this been tested? It'll likely abort if called as both the function itself and udx_napi__stream_writev() allocate local handles which isn't possible without a handle scope.

[telamon]

@kasperisager yes it's been tested & callgrinded, no aborts were triggered.
But that does not diminish your concern.. which handle allocations do you mean?

(this was my reference for typedarray access in typed function paths)

edit: the callgrind also showed that helper function udx_napi__stream_writev() was not inlined as i had hoped.
So would like to redo this path right.

[kasperisager]

js_get_typedarray_info() will allocate a local handle to the underlying ArrayBuffer, which ought to fail if there's no open handle scope. Granted, that does rely on my likely outdated assumption that V8 still doesn't allow active handle scopes in fast functions. That seems to have changed if the callback works just fine though!

[telamon]

hang on... which runtime are you referring to? I have a feeling the lack of abort might be accidental
I just re-did the grinds and discovered that node.js never enters the typed calls

bare on the other hand does

re-measured without valgrind using counters to verify:
Bare: { write: 5160, write_typed: 2008, writev: 1117, writev_typed: 27555 }
Node: { write: 7168, write_typed: 0, writev: 28672, writev_typed: 0 }

But back to the original topic, would it be safer to js_open_handle_scope() before allocating the local handles?

[kasperisager]

Node.js, specifically Node-API, doesn't support fast calls so that's expected. To keep performance optimal, typed callbacks should ideally never allocate local handles. For TypedArray access, this can be achieved by using js_get_typedarray_view() which is guaranteed to never allocate any handles. The gotcha is that js_get_typedarray_info() is usually faster than js_get_typedarray_view() if you already have an open handle scope so it's probably a good idea to measure the impact.

[telamon]

I did a quick measure of typed calls, unsure if my use of js_get_typed_view() is correct.
But fetching views is indeed slower than even the untyped calls:

# bench typedarray access
    # call counters (warmup) { untyped: 127848, scoped: 46928, view: 5224 }
    # peforming 30'000'000 calls on each method
    # untyped ms 1261
    # scoped ms 1232
    # view ms 2525
    # call counters (post bench) { untyped: 30371313, scoped: 29932689, view: 29875998 }

Something that's been bugging me for a while is that js_get_typedarray_info() is only marginally faster than untyped calls.
I don't know if the call-overhead is imagined or we just lose our gains elswhere;

As i said, i don't know if i'm doing something wrong, but all the numbers i've seen so far are a tad bleak.

[kasperisager]

Has this been tested? It'll likely abort if called as both the function itself and udx_napi__stream_write() allocate local handles which isn't possible without a handle scope.