nodejs / nan

Native Abstractions for Node.js
MIT License
3.28k stars 502 forks source link

[discussion] integrating nan or similar with io.js #349

Open trevnorris opened 9 years ago

trevnorris commented 9 years ago

nan has done wonders for the community, but there are still missing bits. If these can be addressed there is a good desire to bring this into io.js itself. This thread should serve as a discussion on what are all the missing bits, what we can do about them and how to eventually get this into io.js.

The most prominent issue is lack of ABI compatibility. Needing to recompile with every new release, especially now with more frequent V8 updates, is difficult. @bnoordhuis mentioned that using templates is not a good form of ABI compatibility because they don't play well with the linker. (Ben, if you have any additional thoughts on this please feel free to include anything I've missed).

Venemo commented 9 years ago

Just a thought about C++ and ABI compatibility. If the guys at Qt and KDE can do it, it's not impossible. Let's not rule out C++ yet. Here's what they have to say about it: https://techbase.kde.org/Policies/Binary_Compatibility_Issues_With_C++ — there are some do's and don'ts in there as well. If you use the “d-pointer” technique and treat virtual functions with care, you don't have much to worry about.

geoffkizer commented 9 years ago

Hey folks, I’m Geoff Kizer from the Chakra team at Microsoft.

Re @imyller:

I think many packages writing their native addons in C/C++ would just need simple "binding" between their native codebase and JS; not the overly fancy V8 facade, data types etc.

This sounds like a simple FFI mechanism. Is that what you meant?

The nice thing about a simple FFI mechanism is that it makes the native code completely independent of the JS engine API. As the V8 API changes, you only need to update the FFI implementation; users of the FFI don’t need to change.

This is basically what the CLR does. Pretty much all native code is invoked through P/Invoke, including performance critical stuff like System.Net.

There’s some discussion of FFI for Node here, but I’m not sure where it stands currently.

trevnorris commented 9 years ago

@geoffkizer Though, to be honest, I doubt the FFI bindings would be fast enough to meet your needs. I've tried some simple operations and it shows considerable overhead compared to directly making the call to C++.

geoffkizer commented 9 years ago

@trevnorris

Though, to be honest, I doubt the FFI bindings would be fast enough to meet your needs. I've tried some simple operations and it shows considerable overhead compared to directly making the call to C++.

I assume you're referring to node-ffi here? I'd be curious to see what you tried and what the perf results were.

Perf is always a concern with an FFI. It's easy to make the FFI slow, much harder to make it fast -- but not necessarily impossible. The CLR's P/Invoke seems to do just fine here. And I believe other languages (Java, Python) rely on an FFI for similar functionality.

I haven't looked at node-ffi in any detail, so I don't know how well it performs for particular scenarios. Maybe it can be optimized further. Or maybe a different (but still FFI-based) approach would be better for performance.

geoffkizer commented 9 years ago

Yeah, node-ffi has a ton of overhead.

Simple "add two ints" function, 1 million iterations: Native C++ call: 17 ms node-ffi to external library: 62535 ms

So using node-ffi seems like a non-starter.

I still think a "simple C/C++ binding" model would be useful. Maybe we could do something like this with C++ templates, as @bnoordhuis suggested on the FFI thread...

TooTallNate commented 9 years ago

FWIW, the newly proposed ffi module in https://github.com/nodejs/io.js/pull/1865 is a lot more low-level of an API, and you could probably squeeze some performance out (or at least less memory usage) by using the new API directly.

geoffkizer commented 9 years ago

Here's an example of how we could use C++ templates to implement a simple native binding mechanism.

Consider a trivial native function, one that adds two integers and returns the result. You’d implement it like this:

void AddMethod(const FunctionCallbackInfo<Value>& args) {
  int32_t x = args[0]->Int32Value();
  int32_t y = args[1]->Int32Value();
  int32_t result = x + y;
  args.GetReturnValue().Set(result);
}

void Initialize(Handle<Object> exports) {
  NODE_SET_METHOD(exports, "add", AddMethod);
}

Most of this is just glue code, to deal with extracting and converting arguments and setting the return value.

With templates, we can make all this glue code automatic, based on the native function signature. Here’s what this would look like:

int32_t AddMethod(int32_t x, int32_t y) {
    return x + y;
}

void Initialize(Handle<Object> exports) {
  NODE_SET_METHOD(exports, "add", BindNativeFunction<AddMethod>);
}

Here, all the V8 “glue” is gone, replaced with simple native types (int32_t here). The magic is in the “BindNativeFunction” template used in the NODE_SET_METHOD macro. It understands how to marshal int32_t to/from JS and provides the necessary glue automatically, based on the argument and return types.

Because this all happens at compile time, the performance is virtually identical. For 1 billion iterations of each: Original implementation: 14929 ms “Template binding” approach: 15074 ms

That’s a difference of ~1% on a completely trivial function. (Likely, one of the helper functions isn’t getting inlined fully; I haven’t checked the actual code.)

There are several nice things about this approach. To begin with, it means you don’t have to write all that glue code – just use the right types in your signature and marshalling happens automatically.

More importantly, though, it isolates the module code from any V8 API changes. If “Int32Value()” suddenly becomes “GetInt32Value()”, the only thing that needs to change is the template code itself.

Obviously this is a very simple example, and we'd need to flesh this out a lot more to support real world scenarios. But I think there's a lot of potential goodness here.

domenic commented 9 years ago

This is pretty similar to how Chrome's JS bindings occur actually, although they have a WebIDL intermediation layer (which has its own pluses and minuses...). It does indeed mostly protect us from V8 API changes, in that most of the code doesn't need to use V8 types at all, as long as we update the binding generators.

trevnorris commented 9 years ago

I like how sleek that looks, but seems to depend on the call being monomorphic. Have a way around that?

geoffkizer commented 9 years ago

@trevnorris: do you have a particular example in mind?

trevnorris commented 9 years ago

Any time a call is made directly from user facing API to native it's a possibility. To get around that we'd need to pad all calls by going through JS first. Fundamentally that's probably the better approach, but not how it's always done now.

Example would be Buffer#copy(). Direct call to native and has optional parameters.

geoffkizer commented 9 years ago

Yes, I'd suggest doing the processing in JS first and call out to a monomorphic native function. As you say, this is generally a better approach.

Also, we could support native argument types like optional<T> if that's generally useful. That would help with Buffer#copy.

trevnorris commented 9 years ago

if we take this approach, might as well just always go through JS first.

using this approach will it throw or abort if there's an argument type mismatch?

geoffkizer commented 9 years ago

Throw on argument type mismatch, I think.

domenic commented 9 years ago

In case it is helpful here is a very simple example of the binding generation in Blink

obastemur commented 9 years ago

Depending on statically defined functions or templates may lower the flexibility of JavaScript kind of method declarations.

Let me share you a sample code from JXcore's native interface. This is JavaScript engine independent.

void sum(JXValue *args, int argc) {
  if ( argc < 2 || !JX_IsDouble(0) || !JX_IsDouble(1) ) {
     return; // or throw an exception..
  }
  double a = JX_GetDouble(0);
  double b = JX_GetDouble(1);

  JX_SetDouble(args+argc, a+b);
}

This interface also covers;

I borrowed the root of this interface from SpiderMonkey while keeping V8 in mind.

JXValue structure given below is intended to be an engine independent data store;

enum _JXType {
  RT_Int32 = 1,
  RT_Double = 2,
  RT_Boolean = 3,
  RT_String = 4,
  RT_Object = 5,
  RT_Buffer = 6,
  RT_Undefined = 7,
  RT_Null = 8,
  RT_Error = 9,
  RT_Function = 10
};

struct _JXValue {
  void *com_;
  bool persistent_;
  bool was_stored_;

  void *data_;
  size_t size_;
  JXValueType type_;
};

I don't see this API is changing / breaking because of an update with V8 or SpiderMonkey. However, this structure is new and needs some updates (i.e. byte ordering) or can be smarter. Yet it does the most of the task needed so far.

I hope the interface for node.js will be abstracting v8 types.

trevnorris commented 9 years ago

Problem I see with creating argument type specific methods is that the number of types is very large: https://github.com/v8/v8-git-mirror/blob/4.4.65/include/v8.h#L1647-L1894

geoffkizer commented 9 years ago

@domenic: Interesting, thanks. IE uses something similar internally.

I think a template-based approach and a WebIDL-based approach are pretty similar; both provide automatic marshalling to/from native types with minimal performance overhead, and both hide the engine API details from the native code author.

The nice thing about the template-based approach is that the mapping is defined and generated by the native function signature itself. No separate WebIDL file, no codegen step, etc.

WebIDL may enable more expressivity in the mapping. I'm not that familiar with WebIDL and so I don't know if this matters much. It might be interesting to see how much of WebIDL semantics we could implement using templates.

domenic commented 9 years ago

I think it does give more expressivity, but at the cost of a lot of obtuse behavior. I wouldn't really want to use Web IDL itself. The template thing sounds cool and hopefully will work if we wrap up anything complicated (like options objects) into a JS wrapper.

geoffkizer commented 9 years ago

@obastemur: I agree that a sufficiently rich engine abstraction API would be more flexible.

I think the interesting question is, how often do you actually need/want this flexibility?

My intuition is that you can get quite far with a simple native binding plus some JS code in front to make the object model pretty. And unless you need the full power and flexibility of the engine abstraction API, you’re better off just using the simple native bindings.

For example, I think many of the internal native modules (e.g. tcp_wrap) would work well with a simple native binding; these are already fairly low-level native wrappers and the kinds of conversions they do are straightforward. They don’t need the full flexibility of an engine abstraction API; they just need a simple mechanism to bind native functions into JS.

That said, it’s hard to know for sure without writing much more code.

obastemur commented 9 years ago

@geoffkizer Agree, the flexibility part is not that critical. Speaking of critical part, I could pick a basic list randomly from the same TCPWrap example.

Nice to have;

Implementing such an interface is not that tricky and It's proven to work. I would like to know if there is a bottleneck that I'm not aware of ?

trevnorris commented 9 years ago

Just to clarity, are we still talking about an abstracted user facing API or an interface to abstract away V8 entirely?

obastemur commented 9 years ago

Just to clarity, are we still talking about an abstracted user facing API or an interface to abstract away V8 entirely?

+1B for abstract away V8 entirely

trevnorris commented 9 years ago

The list of needs to abstract away V8 entirely is much larger. A few of those are:

These are things currently used in core, and I've attempted to keep it to essentials. Nothing too V8 specific. Like being able to have a persistent handle visitor that can walk the list of all Persistent<T> that have been assigned a wrapper class id.

Though loss of all the existing tooling for analysis, debugging and performance would be a big hit.

I'm not trying to poop on the parade. Just want to give a realistic view of what abstracting away the entire V8 API would look like for core.

geoffkizer commented 9 years ago

I think the goal is an abstracted user facing API.

I think the requirements for a user facing API vs the requirements for Node core are quite different. As you list above indicates, trying to provide a full abstraction for Node core makes the problem much larger.

To me, the key observation here is that most modules don’t actually need anything like the full V8 API. What they need is a much smaller set of functionality that allows them to marshal simple types between JS and native. Do JS stuff in JS, do native stuff in native, and provide a simple, minimal, and efficient mapping between the two.

The “template binding” approach I’ve suggested is intended to do that. That said, as @obastemur indicated above, this could also be done with a minimalist abstraction API. Either way, the key is to keep this minimal so we don’t have to abstract the entire V8 API.

obastemur commented 9 years ago

Either way, the key is to keep this minimal so we don’t have to abstract the entire V8 API.

I Agree. A lightweight, engine independent API is very critical for the future of node and it's ecosystem. If an addon requires a specific V8 etc. feature then it can use the current approach.

Long story short, minimalist abstraction approach doesn't limit the capabilities. If you are using an engine specific feature, you better keep the addon updated to the latest changes (as usual). However, minimalistic API saves the majority of the modules from breaking.

Nothing too V8 specific.

@trevnorris Thanks for the list. It may not sound a major difference but Persistent<..> is v8 (precisely v8 GC) specific. It does one basic thing in common. Keeps an 'Object' from garbage collection. The rest of it not that similar.

For the rest of the non-v8 specific items, I didn't see any bottleneck or a too complicated API.

trevnorris commented 9 years ago

@obastemur The key need from Persistent<T> is not keeping objects from being GC'd. It's allowing us to know when the object is about to be GC'd. We have internal resources attached to objects, and those resources need to be cleaned up when the object is about disappear.

obastemur commented 9 years ago

@trevnorris Agree that's the key need but there is no matching feature on SM side. Lots of unrelated details though but if let's say the minimalistic API depends anything but keeping objects from being GC'd that would be engine specific.

[UPDATE] It's not just SM. JSC, Duktape etc. have different approaches.

trevnorris commented 9 years ago

@obastemur To clarify, are you saying that despite the different approaches, each VM has at least some way to achieve the functionality of being notified by the object when it's about to be GC'd?

geoffkizer commented 9 years ago

I don't know how SM works, but Chakra allows you to create finalizable objects which will notify you when they are about to be GC'd. This is different than V8's MakeWeak mechanism, since it's specified as part of object creation instead of later. But it provides the functionality that Node needs.

trevnorris commented 9 years ago

@geoffkizer Thanks for the insight. I can't think of a place within core where an object is made weak after the fact.

Another piece of functionality that is very heavily used in core, and by more than a few modules, is the ability to attach void pointers to the object for later access. Mostly this is used to attach class instances to the object so they can be retrieved later.

@obastemur Want to clarify one more thing you said. Sounded like the concept of preventing an object from being cleaned up by GC through native API isn't possible through other VMs. Or were you just saying it is done differently?

obastemur commented 9 years ago

To clarify, are you saying that despite the different approaches, each VM has at least some way to achieve the functionality of being notified by the object when it's about to be GC'd?

If lets say an object is created on JS land, it doesn't have a kind of "on garbage collected" event listener assigned and there is no way to mark this flag later.

SM lets you set the flags for an object that you've created on the native side. However, we did combine multiple approaches to track the JS side objects for GC time. I was experimenting with JSC and it's similar to SM than V8 on this part.

Sounded like the concept of preventing an object from being cleaned up by GC through native API isn't possible through other VMs. Or were you just saying it is done differently?

Keeping an object from GC is kind of a common result. In details it's different.

Let me share you one of the tricky devils. Here is a generic node scenario;

The minimalistic API may provide the infrastructure for keeping an object from GC. Raising an event for GC is also possible but it's just so delicate to locate properly. Giving away V8 interface as is may break with both future V8 versions and other engines.

I don't know how SM works, but Chakra allows you to create finalizable objects which will notify you when they are about to be GC'd.

@geoffkizer Is it possible to mark a JS side object finalizable ? Or it's defined on creation (similar to SM)

jianchun commented 9 years ago

Is it possible to mark a JS side object finalizable ? Or it's defined on creation (similar to SM)

@obastemur Chakra just added a new API in Win10 release to mark any object (including JS side object) finalizable: JsSetObjectBeforeCollectCallback. The callback can do things like destructing native wrapper, but attempt to create object / execute code will be rejected by Chakra runtime because of the same reason as you mentioned.

trevnorris commented 9 years ago

@obastemur Currently we only need to track JS objects made on the native side. The technical reasons have to do with creating an object that allows tracking a void pointer.

While technically the weak callback in V8 is called post-gc, we aren't doing any object creation or the like during that time anyway. So it shouldn't be a problem. The most important thing is that we can gain access to the void pointers that were attached to the object so we can do all necessary cleanup.

Giving away V8 interface as is may break with both future V8 versions and other engines.

I haven't addressed anything specific to V8's implementation details. Instead what basic functionality is absolutely necessary in order to get node running. Like being able to be notified when an object has been cleaned up by GC, attach void pointers to an object's internal fields, or prevent an object from being collected by GC when it's no longer referenced in JS.

bmeck commented 9 years ago

I think we all need to remember that this is about providing a generic abstract set of Runtime behaviors that Node uses. If an engine does not support the behavior in the same manner as v8 whatever Runtime it provides for Node compatibility will just have to be documented. I have no desire for Node to try and satisfy all the implementation differences between engines. Documenting and specifying what Runtime behaviors we need is doable however, even if not all engines currently support the behavior.

obastemur commented 9 years ago

I have no desire for Node to try and satisfy all the implementation differences between engines. Documenting and specifying what Runtime behaviors we need is doable however, even if not all engines currently support the behavior.

I would suggest you to check the changes to V8's GC over time. There are no thousands of ways for GC and not a long standing best one. This is a delicate place that may change from one version to another. Even if you have no interest for other JS engines, taking other engines into reference would help the final interface less vulnerable.

While technically the weak callback in V8 is called post-gc,

Sweep or post sweep doesn't make much difference since it pauses the main thread. The problem is that the object is still reachable on event but it's memory location is vulnerable (for post sweep case).

we aren't doing any object creation or the like during that time anyway.

Actually we do. There are scenarios to mention but saw this basic example on https://github.com/joyent/node/blob/master/src/node_object_wrap.h#L119-L130 While we are using it inside the node, addon developers may do the same.

I'm optimistic. It may sound complicated but actually it's not. We can develop a minimalistic addon interface that will

Actually some of these engines are the only options on some platforms. Chakra on Windows ARM and SM/JSC on iOS. So, let native node addons reaching to other platforms.. node apps are already there.

geoffkizer commented 9 years ago

@obastemur: how do you get rid of handle<> and local<>?

bmeck commented 9 years ago

@obastemur

I would suggest you to check the changes to V8's GC over time. There are no thousands of ways for GC and not a long standing best one. This is a delicate place that may change from one version to another. Even if you have no interest for other JS engines, taking other engines into reference would help the final interface less vulnerable.

I don't understand the implication here with respect to needing abstract behaviors to be listed for compatibility.

trevnorris commented 9 years ago

I feel like the proposal is moving from "let's create an abstraction layer from how node currently works" to "let's rethink the fundamental architecture to accommodate all possible variances". I'm skeptical that would fly with the TSC.

There's functionality that Node needs. Like knowing specific information about the life of an object. I remember a few years ago Ben attempted to implement node on top of SM, but found it wasn't possible to keep feature parity. Node won't sacrifice features to achieve an abstraction. The majority of the community doesn't care about the abstraction. We need to make the case why it's in their best interest to do so. Regardless of preventing V8 breakage, if features are gimped then it won't be welcome.

kzc commented 9 years ago

a few years ago Ben attempted to implement node on top of SM, but found it wasn't possible to keep feature parity.

@trevnorris, what v8 feature(s) specifically? JXCore is a working Node fork on top of an engine abstraction API supporting both SM and V8. So it's clearly possible.

Are you suggesting that the node foundation would only entertain supporting non-v8 engines only if they were wrapped by a v8-compatible C++ API as Microsoft has done with Chakra? But even so, the v8 API is a moving target - hence this discussion to have an engine agnostic API wrapper.

geoffkizer commented 9 years ago

The majority of the community doesn't care about the abstraction. We need to make the case why it's in their best interest to do so.

I agree.

Here's the case as I see it for a "simple native binding", based on templates or similar:

(1) Simplifies your native code. No need to learn a JS engine API, V8 or abstracted or otherwise. Just use the native types supported by the binding layer and everything works. Easy to use, easy to get right. (2) Isolates you from V8 API breakage. (3) Makes it easier to support other JS engines.

Most module authors don't care much about (3). They care about (1) and (2). Done properly, I think this could be compelling enough to make them want to use it.

trevnorris commented 9 years ago

@kzc It was years ago, and most likely that whatever was lacking at the time now exists. And in terms of V8, I'm saying that no existing functionality can can't be sacrificed because some VM doesn't support it. The abstraction needs to allow all public APIs to continue working as they do today.

@geoffkizer Those are good points, and it sounds like you are more directly addressing native module compatibility. The discussion does seem to have diverged into whether all of core should be abstracted or just the module interface. Which are you looking for?

EDIT: Fixed typo

obastemur commented 9 years ago

how do you get rid of handle<> and local<>?

@geoffkizer I've mentioned on my previous comments a bit. JXcore's embedding interface is JS engine independent. API The approach can be improved and used for native add-ons.

I have no desire for Node to try and satisfy all the implementation differences between engines. I don't understand the implication here with respect to needing abstract behaviors to be listed for compatibility.

@bmeck What I was saying that; we better know the implementation differences among the engines, so we know what is minimalistic and less likely to be broken. I agree with the documentation part.

I feel like the proposal is moving from "let's create an abstraction layer from how node currently works" to "let's rethink the fundamental architecture to accommodate all possible variances".

I thought the discussion was a minimalistic API for native addons.

kzc commented 9 years ago

The abstraction needs to allow all public APIs to continue working as they do today

@trevnorris do you consider the (ever changing) v8 API to be part of Node's public API? Where is the line drawn for the public API?

trevnorris commented 9 years ago

@kzc I'm only considering the APIs we have direct control over. Don't care about the V8 API.

@obastemur This thread has gotten long, and wires were crossed somewhere. Thought that some of this discussion had shifted to removing V8 from core. Native modules is a much easier target. Thanks for clarifying.

obastemur commented 9 years ago

Agree it's been a long thread. Luckily, many experienced developers participating on this discussion. How about prepare and share a minimalistic API (coverage) that is compatible to the engines discussed here.

We might have a better picture on the actual facts / problems. Moreover, we may see if it's reasonable or not.

I can spend some time / or collaborate on the initial list upcoming days.

geoffkizer commented 9 years ago

@trevnorris:

The discussion does seem to have diverged into whether all of core should be abstracted or just the module interface. Which are you looking for?

I think the pressing need is to abstract the module interface, so that's what I think we should focus on.

That's what I meant above re "abstracted user facing API"; I should have said "abstracted user facing module API" to be clearer.

trevnorris commented 9 years ago

For anyone that's participated, I'll be writing up a summary of what these abstractions are attempting to achieve on https://github.com/nodejs/io.js/issues/1993

Basically we're just in the process of properly forming a WG to handle these things.

trevnorris commented 9 years ago

For everyone's information, the API WG repo has been created. I'm still writing the initial README, but feel free to join if you wish to get involved: https://github.com/nodejs/api

I plan for all communication possible to be async. So submit issues and whatnot and we'll get things going.

trevnorris commented 9 years ago

Everyone, @obastemur was awesome enough to write up the first PR for the API WG. This has to do w/ the native API, and those that have participated in this thread are more qualified to give feedback from myself: https://github.com/nodejs/api/pull/4