From 01ee551e704776d8547250ac015a5463613afb45 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 5 Feb 2013 14:24:43 +0100 Subject: [PATCH] typed arrays: only share ArrayBuffer backing store Follow browser behavior, only share the backing store when it's a ArrayBuffer. That is: var abuf = new ArrayBuffer(32); var a = new Int8Array(abuf); var b = new Int8Array(abuf); a[0] = 0; b[0] = 1; assert(a[0] === b[0]); // a and b share memory But: var a = new Int8Array(32); var b = new Int8Array(a); a[0] = 0; b[0] = 1; assert(a[0] !== b[0]); // a and b don't share memory The typed arrays spec allows both `a[0] === b[0]` and `a[0] !=== b[0]` but Chrome and Firefox implement the behavior where memory is not shared. Copying the memory is less efficient but let's do it anyway for the sake of the Principle of Least Surprise. Fixes #4714. --- src/v8_typed_array.cc | 108 +++++++++++++++++++++++++-------------- test/simple/test-typed-arrays.js | 33 ++++++++++++ 2 files changed, 103 insertions(+), 38 deletions(-) diff --git a/src/v8_typed_array.cc b/src/v8_typed_array.cc index a74d295..6efb4ae 100644 --- a/src/v8_typed_array.cc +++ b/src/v8_typed_array.cc @@ -35,11 +35,41 @@ using node::ThrowRangeError; using node::ThrowTypeError; using node::ThrowError; +template +class TypedArray; + +typedef TypedArray<1, v8::kExternalByteArray> Int8Array; +typedef TypedArray<1, v8::kExternalUnsignedByteArray> Uint8Array; +typedef TypedArray<1, v8::kExternalPixelArray> Uint8ClampedArray; +typedef TypedArray<2, v8::kExternalShortArray> Int16Array; +typedef TypedArray<2, v8::kExternalUnsignedShortArray> Uint16Array; +typedef TypedArray<4, v8::kExternalIntArray> Int32Array; +typedef TypedArray<4, v8::kExternalUnsignedIntArray> Uint32Array; +typedef TypedArray<4, v8::kExternalFloatArray> Float32Array; +typedef TypedArray<8, v8::kExternalDoubleArray> Float64Array; + struct BatchedMethods { const char* name; v8::Handle (*func)(const v8::Arguments& args); }; +void WeakCallback(v8::Persistent value, void* data) { + v8::Object* obj = v8::Object::Cast(*value); + + void* ptr = obj->GetIndexedPropertiesExternalArrayData(); + int element_size = v8_typed_array::SizeOfArrayElementForType( + obj->GetIndexedPropertiesExternalArrayDataType()); + int size = + obj->GetIndexedPropertiesExternalArrayDataLength() * element_size; + + v8::V8::AdjustAmountOfExternalAllocatedMemory(-size); + + value.ClearWeak(); + value.Dispose(); + + free(ptr); +} + class ArrayBuffer { public: static v8::Persistent GetTemplate() { @@ -69,23 +99,6 @@ class ArrayBuffer { } private: - static void WeakCallback(v8::Persistent value, void* data) { - v8::Object* obj = v8::Object::Cast(*value); - - void* ptr = obj->GetIndexedPropertiesExternalArrayData(); - int element_size = v8_typed_array::SizeOfArrayElementForType( - obj->GetIndexedPropertiesExternalArrayDataType()); - int size = - obj->GetIndexedPropertiesExternalArrayDataLength() * element_size; - - v8::V8::AdjustAmountOfExternalAllocatedMemory(-size); - - value.ClearWeak(); - value.Dispose(); - - free(ptr); - } - static v8::Handle V8New(const v8::Arguments& args) { if (!args.IsConstructCall()) return ThrowTypeError("Constructor cannot be called as a function."); @@ -125,7 +138,7 @@ class ArrayBuffer { v8::Persistent persistent = v8::Persistent::New(args.This()); - persistent.MakeWeak(NULL, &ArrayBuffer::WeakCallback); + persistent.MakeWeak(NULL, WeakCallback); return args.This(); } @@ -251,12 +264,23 @@ class TypedArray { unsigned int length = 0; unsigned int byte_offset = 0; - // [m1k3] added support for Buffer constructor - if (node::Buffer::HasInstance(args[0]) - || ArrayBuffer::HasInstance(args[0])) { // ArrayBuffer constructor. + if (node::Buffer::HasInstance(args[0]) || + ArrayBuffer::HasInstance(args[0])) { buffer = v8::Local::Cast(args[0]); - size_t buflen = - buffer->GetIndexedPropertiesExternalArrayDataLength(); + size_t buflen = buffer->GetIndexedPropertiesExternalArrayDataLength(); + + // Try to short-circuit as early as possible. Assume that in most + // `new TypedArray(other_array)` calls, the old and the new array + // have the same type. + bool make_copy = HasInstance(buffer) || + Int8Array::HasInstance(buffer) || + Uint8Array::HasInstance(buffer) || + Int16Array::HasInstance(buffer) || + Uint16Array::HasInstance(buffer) || + Int32Array::HasInstance(buffer) || + Uint32Array::HasInstance(buffer) || + Float32Array::HasInstance(buffer) || + Float64Array::HasInstance(buffer); if (!args[1]->IsUndefined() && args[1]->Int32Value() < 0) return ThrowRangeError("Byte offset out of range."); @@ -281,13 +305,31 @@ class TypedArray { } void* buf = buffer->GetIndexedPropertiesExternalArrayData(); - char* begin = reinterpret_cast(buf) + byte_offset; - if (!checkAlignment(reinterpret_cast(begin), TBytes)) - return ThrowRangeError("Byte offset is not aligned."); + if (make_copy) { + void* dst = malloc(length); - args.This()->SetIndexedPropertiesToExternalArrayData( - begin, TEAType, length); + if (dst == NULL) + return ThrowError("Out of memory."); + + v8::V8::AdjustAmountOfExternalAllocatedMemory(length); + memcpy(dst, static_cast(buf) + byte_offset, length); + v8::Persistent persistent = + v8::Persistent::New(args.This()); + persistent->SetIndexedPropertiesToExternalArrayData(dst, + TEAType, + length); + persistent.MakeWeak(NULL, WeakCallback); + } + else { + char* begin = reinterpret_cast(buf) + byte_offset; + + if (!checkAlignment(reinterpret_cast(begin), TBytes)) + return ThrowRangeError("Byte offset is not aligned."); + + args.This()->SetIndexedPropertiesToExternalArrayData( + begin, TEAType, length); + } } else if (args[0]->IsObject()) { // TypedArray / type[] constructor. v8::Local obj = v8::Local::Cast(args[0]); @@ -461,16 +503,6 @@ class TypedArray { } }; -class Int8Array : public TypedArray<1, v8::kExternalByteArray> { }; -class Uint8Array : public TypedArray<1, v8::kExternalUnsignedByteArray> { }; -class Uint8ClampedArray : public TypedArray<1, v8::kExternalPixelArray> { }; -class Int16Array : public TypedArray<2, v8::kExternalShortArray> { }; -class Uint16Array : public TypedArray<2, v8::kExternalUnsignedShortArray> { }; -class Int32Array : public TypedArray<4, v8::kExternalIntArray> { }; -class Uint32Array : public TypedArray<4, v8::kExternalUnsignedIntArray> { }; -class Float32Array : public TypedArray<4, v8::kExternalFloatArray> { }; -class Float64Array : public TypedArray<8, v8::kExternalDoubleArray> { }; - template v8::Handle cTypeToValue(T) { return v8::Undefined(); diff --git a/test/simple/test-typed-arrays.js b/test/simple/test-typed-arrays.js index 8e99af0..ed9cc33 100644 --- a/test/simple/test-typed-arrays.js +++ b/test/simple/test-typed-arrays.js @@ -201,3 +201,36 @@ assert.throws(function() { view.setUint16(0, 1); assert.equal(view.getUint16(0), 1); })(); + +(function() { + // Backing store should not be shared. + var a = new Uint8Array(1); + var b = new Uint8Array(a); + a[0] = 0; + b[0] = 1; + assert.equal(a[0], 0); + assert.equal(b[0], 1); +})(); + +(function() { + // Backing store should not be shared. + var a = new Uint8Array(2); + var b = new Uint16Array(a); + a[0] = 0; + a[1] = 0; + b[0] = 257; + assert.equal(a[0], 0); + assert.equal(a[1], 0); + assert.equal(b[0], 257); +})(); + +(function() { + // Backing store should be shared. + var abuf = new ArrayBuffer(32); + var a = new Uint8Array(abuf); + var b = new Uint8Array(abuf); + a[0] = 0; + b[0] = 1; + assert.equal(a[0], 1); + assert.equal(b[0], 1); +})(); -- 2.7.4