Skip to content
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

Implement an initial implementation of DataView class #205

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,88 @@ inline void ArrayBuffer::EnsureInfo() const {
}
}

#if NAPI_DATA_VIEW_FEATURE
////////////////////////////////////////////////////////////////////////////////
// DataView class
////////////////////////////////////////////////////////////////////////////////
inline DataView DataView::New(napi_env env,
Napi::ArrayBuffer arrayBuffer) {
return New(env, arrayBuffer, 0, arrayBuffer.ByteLength());
}

inline DataView DataView::New(napi_env env,
Napi::ArrayBuffer arrayBuffer,
size_t byteOffset) {
if (byteOffset > arrayBuffer.ByteLength()) {
NAPI_THROW(RangeError::New(env,
"Start offset is outside the bounds of the buffer"));
return DataView();
}
return New(env, arrayBuffer, byteOffset,
arrayBuffer.ByteLength() - byteOffset);
}

inline DataView DataView::New(napi_env env,
Napi::ArrayBuffer arrayBuffer,
size_t byteOffset,
size_t byteLength) {
if (byteOffset + byteLength > arrayBuffer.ByteLength()) {
NAPI_THROW(RangeError::New(env, "Invalid DataView length"));
return DataView();
}
napi_value value;
napi_status status = napi_create_dataview(
env, byteLength, arrayBuffer, byteOffset, &value);
NAPI_THROW_IF_FAILED(env, status, DataView());
return DataView(env, value);
}

inline DataView::DataView() : Object() {
}

inline DataView::DataView(napi_env env, napi_value value) : Object(env, value) {
}

inline Napi::ArrayBuffer DataView::ArrayBuffer() const {
napi_value arrayBuffer;
napi_status status = napi_get_dataview_info(
_env,
_value /* dataView */,
nullptr /* byteLength */,
nullptr /* data */,
&arrayBuffer /* arrayBuffer */,
nullptr /* byteOffset */);
NAPI_THROW_IF_FAILED(_env, status, Napi::ArrayBuffer());
return Napi::ArrayBuffer(_env, arrayBuffer);
}

inline size_t DataView::ByteOffset() const {
size_t byteOffset;
napi_status status = napi_get_dataview_info(
_env,
_value /* dataView */,
nullptr /* byteLength */,
nullptr /* data */,
nullptr /* arrayBuffer */,
&byteOffset /* byteOffset */);
NAPI_THROW_IF_FAILED(_env, status, 0);
return byteOffset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make any sense to get the data once and cache it as opposed to requesting it every time, I think at least some of them will not change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that but I thought that it would be better to request the data everytime than spending additional memory. I think that ByteOffset(), ByteLength(), and ArrayBuffer() methods are necessary but I'm not sure if they are very often used. So, isn't it better to reduce memory usage? (Also, if users want to cache it, they can do it themselves.) WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with keeping it simple for now, if we find it makes sense later on we can always add the cache.

}

inline size_t DataView::ByteLength() const {
size_t byteLength;
napi_status status = napi_get_dataview_info(
_env,
_value /* dataView */,
&byteLength /* byteLength */,
nullptr /* data */,
nullptr /* arrayBuffer */,
nullptr /* byteOffset */);
NAPI_THROW_IF_FAILED(_env, status, 0);
return byteLength;
}
#endif

////////////////////////////////////////////////////////////////////////////////
// TypedArray class
////////////////////////////////////////////////////////////////////////////////
Expand Down
30 changes: 30 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,36 @@ namespace Napi {
T* data);
};

#if NAPI_DATA_VIEW_FEATURE
/// The DataView provides a low-level interface for reading/writing multiple
/// number types in an ArrayBuffer irrespective of the platform's endianness.
class DataView : public Object {
public:
static DataView New(napi_env env,
Napi::ArrayBuffer arrayBuffer);
static DataView New(napi_env env,
Napi::ArrayBuffer arrayBuffer,
size_t byteOffset);
static DataView New(napi_env env,
Napi::ArrayBuffer arrayBuffer,
size_t byteOffset,
size_t byteLength);

DataView(); ///< Creates a new _empty_ DataView instance.
DataView(napi_env env, napi_value value); ///< Wraps a N-API value primitive.

Napi::ArrayBuffer ArrayBuffer() const; ///< Gets the backing array buffer.
size_t ByteOffset() const; ///< Gets the offset into the buffer where the array starts.
size_t ByteLength() const; ///< Gets the length of the array in bytes.

// TODO: This class isn't a complete implementation yet, and will
// incrementally add additional methods to read/write data into buffer.
// Currently, this class is wrapped by the NAPI_DATA_VIEW_FEATURE macro flag
// and this should be enabled only in the tests until the implementation is
// completed.
};
#endif

class Function : public Object {
public:
/// Callable must implement operator() accepting a const CallbackInfo&
Expand Down
2 changes: 2 additions & 0 deletions test/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Object InitAsyncWorker(Env env);
Object InitBasicTypesNumber(Env env);
Object InitBasicTypesValue(Env env);
Object InitBuffer(Env env);
Object InitDataView(Env env);
Object InitError(Env env);
Object InitExternal(Env env);
Object InitFunction(Env env);
Expand All @@ -22,6 +23,7 @@ Object Init(Env env, Object exports) {
exports.Set("basic_types_number", InitBasicTypesNumber(env));
exports.Set("basic_types_value", InitBasicTypesValue(env));
exports.Set("buffer", InitBuffer(env));
exports.Set("dataview", InitDataView(env));
exports.Set("error", InitError(env));
exports.Set("external", InitExternal(env));
exports.Set("function", InitFunction(env));
Expand Down
1 change: 1 addition & 0 deletions test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
'basic_types/value.cc',
'binding.cc',
'buffer.cc',
'dataview/dataview.cc',
'error.cc',
'external.cc',
'function.cc',
Expand Down
46 changes: 46 additions & 0 deletions test/dataview/dataview.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#include "napi.h"

using namespace Napi;

static Value CreateDataView1(const CallbackInfo& info) {
ArrayBuffer arrayBuffer = info[0].As<ArrayBuffer>();
return DataView::New(info.Env(), arrayBuffer);
}

static Value CreateDataView2(const CallbackInfo& info) {
ArrayBuffer arrayBuffer = info[0].As<ArrayBuffer>();
size_t byteOffset = info[1].As<Number>().Uint32Value();
return DataView::New(info.Env(), arrayBuffer, byteOffset);
}

static Value CreateDataView3(const CallbackInfo& info) {
ArrayBuffer arrayBuffer = info[0].As<ArrayBuffer>();
size_t byteOffset = info[1].As<Number>().Uint32Value();
size_t byteLength = info[2].As<Number>().Uint32Value();
return DataView::New(info.Env(), arrayBuffer, byteOffset, byteLength);
}

static Value GetArrayBuffer(const CallbackInfo& info) {
return info[0].As<DataView>().ArrayBuffer();
}

static Value GetByteOffset(const CallbackInfo& info) {
return Number::New(info.Env(), info[0].As<DataView>().ByteOffset());
}

static Value GetByteLength(const CallbackInfo& info) {
return Number::New(info.Env(), info[0].As<DataView>().ByteLength());
}

Object InitDataView(Env env) {
Object exports = Object::New(env);

exports["createDataView1"] = Function::New(env, CreateDataView1);
exports["createDataView2"] = Function::New(env, CreateDataView2);
exports["createDataView3"] = Function::New(env, CreateDataView3);
exports["getArrayBuffer"] = Function::New(env, GetArrayBuffer);
exports["getByteOffset"] = Function::New(env, GetByteOffset);
exports["getByteLength"] = Function::New(env, GetByteLength);

return exports;
}
38 changes: 38 additions & 0 deletions test/dataview/dataview.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const buildType = process.config.target_defaults.default_configuration;
const assert = require('assert');

test(require(`../build/${buildType}/binding.node`));
test(require(`../build/${buildType}/binding_noexcept.node`));

function test(binding) {
function testDataViewCreation(factory, arrayBuffer, offset, length) {
const view = factory(arrayBuffer, offset, length);
offset = offset ? offset : 0;
assert.ok(dataview.getArrayBuffer(view) instanceof ArrayBuffer);
assert.strictEqual(dataview.getArrayBuffer(view), arrayBuffer);
assert.strictEqual(dataview.getByteOffset(view), offset);
assert.strictEqual(dataview.getByteLength(view),
length ? length : arrayBuffer.byteLength - offset);
}

function testInvalidRange(factory, arrayBuffer, offset, length) {
assert.throws(() => {
factory(arrayBuffer, offset, length);
}, RangeError);
}

const dataview = binding.dataview;
const arrayBuffer = new ArrayBuffer(10);

testDataViewCreation(dataview.createDataView1, arrayBuffer);
testDataViewCreation(dataview.createDataView2, arrayBuffer, 2);
testDataViewCreation(dataview.createDataView2, arrayBuffer, 10);
testDataViewCreation(dataview.createDataView3, arrayBuffer, 2, 4);
testDataViewCreation(dataview.createDataView3, arrayBuffer, 10, 0);

testInvalidRange(dataview.createDataView2, arrayBuffer, 11);
testInvalidRange(dataview.createDataView3, arrayBuffer, 11, 0);
testInvalidRange(dataview.createDataView3, arrayBuffer, 6, 5);
}
1 change: 1 addition & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ let testModules = [
'basic_types/number',
'basic_types/value',
'buffer',
'dataview/dataview',
'error',
'external',
'function',
Expand Down