Add WebAssembly port of N-API #2

OhadRau · 2019-08-01T18:09:45Z

This PR builds on top of #1 by adding a WebAssembly version of the Node API.

This is split into a few parts:

wasm_node_api/wasm_node_api.cc = Additions to Node that export N-API to Wasm modules using V8 WASM embedder builtins. These also add some extra functions that are needed to translate N-API values to WebAssembly. See comments below for an explanation of some of the template magic used.
wasm_node_api/wasm_napi_lib/node_api_overrides.{cc,h} = Changes to N-API that are required to make it work in WebAssembly. These do things like change the ArrayBuffer implementation so that we can conform to the original API (because a NAPI-created ArrayBuffer will be in native memory, which WASM can't access) and copy WASM structs to native structs.
wasm_node_api/wasm_napi_lib/{js_native_api.h,js_native_api_types.h,node_api.h,node_api_types.h} = Copies of N-API headers modified to work with WASM32. Because of the pointer size in WASM32 programs, these headers have to be specially compiled to allow for your WASM module to deal with native (64-bit) pointers. This is not compatible with the existing N-API implementation, but can be removed as soon as WASM64 becomes a usable compiler target in LLVM.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

OhadRau · 2019-08-01T21:26:39Z

wasm_node_api/wasm_node_api.cc

+constexpr auto is_function_pointer_v = is_function_pointer<T>::value;
+
+template<typename T>
+class Native {


Native and Wasm are essentially type annotations that tell us whether a value was created in native code or in WASM code. This is important because of the different pointer sizes between systems, different memory spaces of native code vs. WASM, and other differences like function pointer representations. These types are defined such that kind<Wasm<T>> == T and kind<Native<T>> == T, allowing us to "unwrap" the types when we actually use them.

It sounds like this should be converted to a comment in the code.

OhadRau · 2019-08-01T21:27:58Z

wasm_node_api/wasm_node_api.cc

+using kind = typename K::type;
+
+template<typename T>
+constexpr w::ValKind wasmType() {


Converts a given C++ type into its canonical WASM representation. In this case, floats become F32, doubles become F64, native pointers become I64, and everything else becomes I32.

This can be a comment in the code.

OhadRau · 2019-08-01T21:29:32Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<w::ValKind... Args>
+constexpr auto exportVoid(callback cb) -> w::Func {


These two functions (exportVoid and exportFn) allow us to turn a given WASM callback into a v8::wasm::Func value (that can be shared with WASM code). It does this by constructing the appropriate v8::wasm::FuncType based on the type parameters passed in and creating a v8::wasm::Func from the callback. The difference between the two functions is that exportVoid deals with functions that return void, while exportFn deals with functions that return a single value. Note that the WASM API allows us to create functions with >1 return value, but that functionality isn't necessary to embed N-API.

This should be a comment in the code.

OhadRau · 2019-08-01T21:31:07Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Return, typename... Args>
+constexpr auto exportNative(callback cb) -> w::Func {


Determines what kind of function type we're dealing with and uses either exportVoid or exportFn as appropriate.

OhadRau · 2019-08-01T21:32:20Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<typename Arg>
+constexpr Arg napiCast(const w::Val& v) {


napiCast implements the v8::wasm::Val -> C++ type conversion. For types other than floating point numbers, this just takes the 32-bit number and casts it to the proper type.

OhadRau · 2019-08-01T21:41:56Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<typename Result, typename... Args>
+struct napiApply {


This function is analogous to std::apply in that it allows us to call a generic function using an array of arguments but is specifically designed to unwrap arguments using napiUnwrap. This allows us to statically create WASM call wrappers for N-API functions (i.e. converting every parameter into a N-API value before making the call).

The use of std::index_sequence is a bit of a hack that's needed in order to implement calls for an arbitrary number of parameters. I use this to iterate over the arguments array and "spread" them out as the arguments to the function pointer fn.

The use of a function inside of a struct enables us to have two templates. This is useful for two reasons:

Because we can infer some of the template parameters (specifically the indices, which would be a pain to write manually each time) but not others (the type of the function pointer), we can separate them out to two sets where only one is inferred.

Index sequences require us to use a variadic template. Since we can't have two sets of variadic parameters, we have to split this into two different template invocations to get both the ...Args and ...Indices

Part of this should be converted to a code comment.

OhadRau · 2019-08-01T21:55:35Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<typename Result>
+constexpr kind<Result> napiUnwrap(const w::Context* ctx, const w::Val& v) {


This function builds on top of napiCast, but allows for more complex interpretations of WASM values. Specifically, this automatically dereferences pointers from the WASM memory block and discerns between different pointer sizes as necessary.

OhadRau · 2019-08-01T21:56:09Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Result>
+constexpr w::Val napiWrap(kind<Result> v) {


This function is the inverse of napiUnwrap and is able to "re-wrap" the C++ value in a v8::wasm::Val.

OhadRau · 2019-08-01T21:58:22Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Result, typename... Args>
+struct napiCall {


napiCall is an abstraction over napiApply that performs the entire call operation into a N-API function. By taking the function pointer as a template parameter, this generates a static call wrapper for the N-API function. It includes some of the boilerplate for making napiApply calls and also handles the return values from napiApply and places them into the results array if necessary.

OhadRau · 2019-08-01T22:00:19Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Result, typename... Args>
+struct napiBind {


This is the final "step" in code generation for N-API wrappers, and performs the registration of the N-API function as a WASM embedder builtin. This is done in three parts:

Generate the napiCall wrapper for the function pointer.

Create a v8::wasm::Func for that call wrapper

Register that v8::wasm::Func as a builtin using the V8 WASM API. This takes an isolate and name, which tells us where to register the function.

Make this a code comment.

kjin · 2019-08-02T17:23:00Z

wasm_node_api/wasm_node_api.cc

+                 is_function_pointer_v<kind<Result>>) {
+      return (kind<Result>) v.i32();/*(ctx->table->get(v.i32()));*/
+    } else {
+      return (kind<Result>) (&ctx->memory->data()[(uint32_t) v.i32()]);


Would you need special logic to handle null pointers here?

Along a similar line of thought, if you have double pointers, is there any special handling needed there?

Null pointers: I think the appropriate change here is:

uint32_t ptr = v.i32(); return (kind<Result>) ptr == 0 ? nullptr : (&ctx->memory->data()[(uint32_t) v.i32()]);

This will check if the WASM pointer is null and return a proper null pointer if that's the case. The issue in the current code is that if the function tries to return a null pointer the C++ wrapper wouldn't know since it would be the base memory address of the WASM module.

In regards to double pointers, the only case where these come up in N-API is when the underlying value is a pointer to a NAPI value. Since these will always exist in the native memory space and won't be accessed by the WASM program, we don't have to do any conversions.

kjin · 2019-08-02T17:26:00Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Result, typename... Args>
+struct napiBind {


More of a question rather than a change request -- for these napi* structs, how much of it is actually specific to N-API vs. more generally applicable? Other than the assumption that only 0-1 return values are used, it seems like rather generic behavior to need to wrap/unwrap arguments.

Yeah, these are pretty generic actually and can probably be renamed to something more descriptive.

kjin · 2019-08-02T18:07:31Z

wasm_node_api/wasm_node_api.cc

+  mod->nm_filename = nm_filename;
+}
+
+// TODO(ohadrau): Is this a safe operation?


What would make it not safe? Is it that you are passing in a function pointer?

The concern here is that the function pointer that gets passed in is not a valid function pointer, since it refers to an index in the WASM indirect function table. Since we theoretically control 100% of the use of this function pointer, we're able to get away with using this hack but it is a big assumption to make in my opinion.

ofrobots

How is test_wasm_napi.node.wasm generated? Where's the source code and how would it need to be compiled to get to the form it is in here?

There are many instances of comments added as github comments which should be code comments.

ofrobots · 2019-08-02T19:42:05Z

src/node.cc

 #include "node_v8_platform-inl.h"
 #include "node_version.h"

+#define WASM_BUILTIN_NODE_API 1


Add a TODO comment to move this to a configure flag.

ofrobots · 2019-08-02T19:43:28Z

wasm_node_api/wasm_node_api.cc

+constexpr auto is_function_pointer_v = is_function_pointer<T>::value;
+
+template<typename T>
+class Native {


It sounds like this should be converted to a comment in the code.

ofrobots · 2019-08-02T19:47:24Z

wasm_node_api/wasm_napi_lib/node_api.h

+#ifndef WASM_NODE_API_NODE_API_H_
+#define WASM_NODE_API_NODE_API_H_
+
+#include "js_native_api.h"


Add a comment here indicating where this file comes from.

ofrobots · 2019-08-02T19:48:24Z

wasm_node_api/wasm_napi_lib/js_native_api.h

@@ -0,0 +1,497 @@
+#ifndef WASM_NODE_API_JS_NATIVE_API_H_
+#define WASM_NODE_API_JS_NATIVE_API_H_
+


Add a comment here explaining where this file comes from

ofrobots · 2019-08-02T19:49:29Z

wasm_node_api/wasm_node_api.cc

+using kind = typename K::type;
+
+template<typename T>
+constexpr w::ValKind wasmType() {


This can be a comment in the code.

ofrobots · 2019-08-02T19:49:50Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<w::ValKind... Args>
+constexpr auto exportVoid(callback cb) -> w::Func {


This should be a comment in the code.

ofrobots · 2019-08-02T19:52:58Z

wasm_node_api/wasm_node_api.cc

+}
+
+template<typename Result, typename... Args>
+struct napiApply {


Part of this should be converted to a code comment.

ofrobots · 2019-08-02T19:54:35Z

wasm_node_api/wasm_node_api.cc

+};
+
+template<typename Result, typename... Args>
+struct napiBind {


Make this a code comment.

OhadRau · 2019-08-02T20:35:31Z

How is test_wasm_napi.node.wasm generated? Where's the source code and how would it need to be compiled to get to the form it is in here?

There are many instances of comments added as github comments which should be code comments.

Thanks for the feedback, added the test_wasm_napi.node.wasm code + Makefile and applied the other changes requested.

Add WASM port of NAPI

2f3c1c2

OhadRau requested a review from kjin August 1, 2019 18:09

Implement WASM callbacks for NAPI properties

b469559

OhadRau force-pushed the feat/wasm-napi-port branch from eda78f7 to b469559 Compare August 1, 2019 18:15

OhadRau mentioned this pull request Aug 1, 2019

Add WASM native module loader #3

Open

4 tasks

OhadRau commented Aug 1, 2019

View reviewed changes

kjin reviewed Aug 2, 2019

View reviewed changes

ofrobots reviewed Aug 2, 2019

View reviewed changes

Cleanup wasm node api

362776b

		@@ -0,0 +1,497 @@
		#ifndef WASM_NODE_API_JS_NATIVE_API_H_
		#define WASM_NODE_API_JS_NATIVE_API_H_

Add WebAssembly port of N-API #2

Are you sure you want to change the base?

Add WebAssembly port of N-API #2

Uh oh!

Conversation

OhadRau commented Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OhadRau Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OhadRau Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OhadRau Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ofrobots left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OhadRau commented Aug 2, 2019

Uh oh!

OhadRau commented Aug 1, 2019 •

edited

Loading

OhadRau Aug 1, 2019 •

edited

Loading

OhadRau Aug 1, 2019 •

edited

Loading

OhadRau Aug 1, 2019 •

edited

Loading