Docs: Further improvements to Addon section, specifically numeric and binary IO

[metabench]

Asking about IO between C++ and JavaScript has been a common theme to the issues I have opened. I have had some questions answered before, and prompted some discussions on what kind of documentation is best. See:

#883
#2977
#3296

I have done some work on examples that show the interface between C++ and node.js by internally performing the simple operation of adding one to a number. I now wish to make it output a Float64Array, but have not seen example code of how to do it, nor got it work when I tried to make one in C++ using V8.

Here is the beginnings of some code which could be of use within the node documentation to demonstrate the outputting of Typed Arrays from C++: https://github.com/metabench/addone/tree/typed_arrays

In order to help developers make use of C++ for more numerically or computationally intensive tasks, it's worth improving the documentation (lowering the learning curve) for the most efficient types of IO between C++ and JavaScript.

[metabench]

I am making use of this code so far:

v8::Local<v8::Float64Array> fa_input = args[0].As<v8::Float64Array>();
int l = fa_input -> ByteLength() / sizeof(double);
const double *x = reinterpret_cast<double*>(fa_input -> Buffer() -> GetContents().Data());

double* res = new double[l];

// Set the values in the res array.

Local<ArrayBuffer> ab = v8::ArrayBuffer::New (args.GetIsolate(), res, l * sizeof(double));
v8::Local<v8::Float64Array> f64a_res = v8::Float64Array::New(ab, 0, l);

args.GetReturnValue().Set(f64a_res);

I could do with opinions on if this is the best way to do it.

[addaleax]

fa_input -> Buffer() -> GetContents().Data()

Does this work correctly with subarrays, e.g. new Float64Array([1,2,3]).subarray(1,2)?

[metabench]

@addaleax No, it's not working properly with subarrays. I created a subarray with the code you provided. The only item in that subarray is 2. The resulting array from addone_ta is still [ 2 ].

I have merged the typed array code into master on https://github.com/metabench/addone/blob/master/addone.cc

Suggestions or code fixes to handle subarrays would be very much appreciated. Thanks for pointing me towards finding this bug.

[metabench]

The current best code that I have, which takes account of subarrays, is as follows:
(Updated 8 September 2016)

void Addone_TA_F64(const FunctionCallbackInfo<Value>& args) {
  // Get the input values, accounting for subarrays
  v8::Local<v8::Float64Array> fa_input = args[0].As<v8::Float64Array>();
  int l = fa_input -> ByteLength() / sizeof(double);
  double *x = reinterpret_cast<double*>(static_cast<unsigned char*>(fa_input->Buffer()->GetContents().Data()) + fa_input->ByteOffset());
  double* res = new double[l];

  // Inner processing
  for (int c = 0; c < l; c++) {
    res[c] = addone(x[c]);
  }

  // Output a new typed array
  args.GetReturnValue().Set(v8::Float64Array::New(v8::ArrayBuffer::New (args.GetIsolate(), res, l * sizeof(double)), 0, l));
}

I don't fully understand how it works, and I am hoping to get good advice on it and for it or something better to be established as the best-practice way of doing IO of Typed Array data when not using nan (https://github.com/nodejs/nan). It's possible that advances made in understanding the IO of different data types between node and C++ could be useful for the nan project, and it's also possible that code from within nan has patterns which are of use in documenting how to do things when not using nan.

[addaleax]

Well,

node/src/node_buffer.cc

Lines 32 to 41 in da0651a

	#define SPREAD_ARG(val, name) \
	CHECK((val)->IsUint8Array()); \
	Local<Uint8Array> name = (val).As<Uint8Array>(); \
	ArrayBuffer::Contents name##_c = name->Buffer()->GetContents(); \
	const size_t name##_offset = name->ByteOffset(); \
	const size_t name##_length = name->ByteLength(); \
	char* const name##_data = \
	static_cast<char*>(name##_c.Data()) + name##_offset; \
	if (name##_length > 0) \
	CHECK_NE(name##_data, nullptr);

is what Node core does… it’s not the prettiest thing in the world but I think it kind of gives an idea of how to tackle this?

[metabench]

Yes, it does give an idea on how to tackle this, but it's making use of some quite advanced C++ concepts, or at least ones I don't fully understand. Please correct me if I'm wrong, but it's a template definition, and that the convention is to use capital letters when naming them. I've not made them myself in my C++ programming, but have used them with NAN.

I can't see exactly where I could make use of SPREAD_ARG, and if it's better in some ways than the way I have found to do it. It would definitely be useful if the node.js team made further documentation that showed IO of a variety of TypedArrays as well as Buffer. If there is code within the node engine already that could be used for it,. then my question is about if there is a way to link to that code to where it is in Buffer, or even if the node team think it's worthwhile to expand the core capabilities to interact with C++.

[addaleax]

Please correct me if I'm wrong, but it's a template definition, and that the convention is to use capital letters when naming them.

I think you’re understanding everything correctly, yes! Just so no confusion comes up, the #defines are usually referred to as macros. C++ has something called templates, but that’s a bit of a different concept.

I’m not actually suggesting that you need to include SPREAD_ARG in your own code; you may be very happy just using its contents.

It would definitely be useful if the node.js team made further documentation that showed IO of a variety of TypedArrays as well as Buffer.

Hmm. I’m not disagreeing, this is a problem, but maybe the question is not what should be documented, but where that documentation should be. Handling TypedArrays from C++ is not a Node.js core thing – all of the APIs you have seen so far (.As<Uint8Array>(), GetContents, etc.) comes from V8; they are not controlled by Node.js.

Also, there’s nan, which provides helpers/abstractions of the JS APIs to Node.js addons, and it has a TypedArrayContents helper (docs) that you might be interested in.

[mateogianolio]

The main issue is that the v8 documentation is not very approachable. It's basically auto-generated from v8.h.

I think the best bet is to put some pressure on the v8 team to improve it, rather than have node take over the responsibility (which isn't feasible since features can still change drastically in the future).

Hopefully it won't take too long. Better documentation is key to widespread adoption. I know I would enjoy seeing more node addons used creatively in high performance applications.

Honestly, I don't feel like nan is the right way to move forward. A header file with abstractions would have been OK, but making it an npm package adds unnecessary overhead to both the installation and build steps. It's a good idea executed poorly, at least in my opinion.

[addaleax]

I think the best bet is to put some pressure on the v8 team to improve it

I think the V8 team is kind of aware that their documentation is not optimal. If you want to see something improved in an open source project, it’s always better to spend time yourself on improving things than to “put pressure” on someone.

A header file with abstractions would have been OK, but making it an npm package adds unnecessary overhead to both the installation and build steps. It's a good idea executed poorly, at least in my opinion.

Are you asking for inclusion of the nan features into node core? That would kind of defeat nan’s primary purpose of abstracting away the differences between Node/V8 versions.

[mateogianolio]

I thought V8 was funded and developed by Google, which is why I added 'put pressure' part. I didn't intend to be derogatory. I will look into helping them with the documentation!

I'm not at all proposing an inclusion of nan into node core. I meant that it might be better (for high performance applications) developing addons without using nan, hence the need for better v8 documentation.

[addaleax]

I thought V8 was funded and developed by Google

It is. :)

[fhinkel]

Closing, but feel free to re-open or comment if you feel like the issue has not been adequately addressed.