-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor native APIs #159
Refactor native APIs #159
Conversation
I like the overall direction of making the Node-API low implementation pluggable.
|
Please note that current code uses the C++-like delay load stubs to avoid any runtime checks. |
Not all: just function names, symbol names, and error messages, which are almost always going to already be
I considered that, but chose the inheritance approach because:
I was assuming that would impact startup time, but maybe it's not much. I can try to benchmark it. |
I see. If it is only for errors, then it must be fine. For errors we will have much more overhead anyway.
It makes sense. Thanks!
It would be interesting to see. Maybe the whole code could become much simpler. |
704536d
to
6552b17
Compare
I just pushed a big update to this PR:
|
This refactor moves all native APIs (
napi_*
functions) from theJSNativeApi.Interop
class to aJSRuntime
abstract class. Then a specific runtime (NodejsRuntime
) implements the abstract class via (delayed) binding to the native APIs, while also handling some low-level details like GC pinning.This has a few benefits:
TracingJSRuntime
class.The drawback is this adds another level of indirection: every native API call is a virtual method invocation (along with a null check for delay-loading). I know there was a lot of work (and code) put into avoiding both of those costs. But now that we have micro-benchmarks, we can fairly accurately measure the impact of the changes... or lack of impact. From what I can see so far, there is no measurable performance impact: all of the benchmark results are consistently within the margin of error (~4%) before and after the changes. (Sometimes a bit slower, sometimes a bit faster.) I still need to do some more perf measurements on other OS's.
At this point all tests are passing, but some API areas are not yet refactored. And, I want to think more about how to initialize and reference the current runtime thread-static in relation to the current
JSValueScope
andJSRuntimeContext
thread-statics.