From fc73052dc2970aac5c232e74c8dce9215c9137db Mon Sep 17 00:00:00 2001 From: "dslomov@chromium.org" Date: Thu, 23 May 2013 10:01:42 +0000 Subject: [PATCH] Externalization API for ArrayBuffer R=svenpanne@chromium.org Review URL: https://codereview.chromium.org/15001041 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14770 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- include/v8.h | 50 +++++++++++++++++++--- src/api.cc | 36 ++++++++++++---- src/bootstrapper.cc | 10 +++-- src/d8.cc | 21 ++++++++-- src/objects-inl.h | 12 +++++- src/objects.h | 12 +++++- src/runtime.cc | 26 ++++++++---- src/runtime.h | 3 +- test/cctest/test-api.cc | 91 +++++++++++++++++++++++++++++++---------- 9 files changed, 207 insertions(+), 54 deletions(-) diff --git a/include/v8.h b/include/v8.h index 96da0c26de..7dfc9189aa 100644 --- a/include/v8.h +++ b/include/v8.h @@ -2325,6 +2325,33 @@ class V8EXPORT Function : public Object { static void CheckCast(Value* obj); }; +/** + * The contents of an |ArrayBuffer|. Externalization of |ArrayBuffer| + * populates an instance of this class with a pointer to data and byte length. + * + * |ArrayBufferContents| is the owner of its data. When an instance of + * this class is destructed, the |Data| is freed. + * + * This API is experimental and may change significantly. + */ +class V8EXPORT ArrayBufferContents { + public: + ArrayBufferContents() : data_(NULL), byte_length_(0) {} + ~ArrayBufferContents(); + + void* Data() const { return data_; } + size_t ByteLength() const { return byte_length_; } + + private: + void* data_; + size_t byte_length_; + + friend class ArrayBuffer; +}; + +#ifndef V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT +#define V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT 2 +#endif /** * An instance of the built-in ArrayBuffer constructor (ES6 draft 15.13.5). @@ -2336,27 +2363,40 @@ class V8EXPORT ArrayBuffer : public Object { * Data length in bytes. */ size_t ByteLength() const; - /** - * Raw pointer to the array buffer data - */ - void* Data() const; /** * Create a new ArrayBuffer. Allocate |byte_length| bytes. * Allocated memory will be owned by a created ArrayBuffer and - * will be deallocated when it is garbage-collected. + * will be deallocated when it is garbage-collected, + * unless the object is externalized. */ static Local New(size_t byte_length); /** * Create a new ArrayBuffer over an existing memory block. + * The created array buffer is immediately in externalized state. * The memory block will not be reclaimed when a created ArrayBuffer * is garbage-collected. */ static Local New(void* data, size_t byte_length); + /** + * Returns true if ArrayBuffer is extrenalized, that is, does not + * own its memory block. + */ + bool IsExternal() const; + + /** + * Pass the ownership of this ArrayBuffer's backing store to + * a given ArrayBufferContents. + */ + void Externalize(ArrayBufferContents* contents); + V8_INLINE(static ArrayBuffer* Cast(Value* obj)); + + static const int kInternalFieldCount = V8_ARRAY_BUFFER_INTERNAL_FIELD_COUNT; + private: ArrayBuffer(); static void CheckCast(Value* obj); diff --git a/src/api.cc b/src/api.cc index eb99257d78..5fb810312a 100644 --- a/src/api.cc +++ b/src/api.cc @@ -6016,6 +6016,32 @@ Local Array::CloneElementAt(uint32_t index) { } +bool v8::ArrayBuffer::IsExternal() const { + return Utils::OpenHandle(this)->is_external(); +} + +v8::ArrayBufferContents::~ArrayBufferContents() { + free(data_); + data_ = NULL; + byte_length_ = 0; +} + + +void v8::ArrayBuffer::Externalize(ArrayBufferContents* contents) { + i::Handle obj = Utils::OpenHandle(this); + ApiCheck(!obj->is_external(), + "v8::ArrayBuffer::Externalize", + "ArrayBuffer already externalized"); + obj->set_is_external(true); + size_t byte_length = static_cast(obj->byte_length()->Number()); + ApiCheck(contents->data_ == NULL, + "v8::ArrayBuffer::Externalize", + "Externalizing into non-empty ArrayBufferContents"); + contents->data_ = obj->backing_store(); + contents->byte_length_ = byte_length; +} + + size_t v8::ArrayBuffer::ByteLength() const { i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); if (IsDeadCheck(isolate, "v8::ArrayBuffer::ByteLength()")) return 0; @@ -6024,14 +6050,6 @@ size_t v8::ArrayBuffer::ByteLength() const { } -void* v8::ArrayBuffer::Data() const { - i::Isolate* isolate = Utils::OpenHandle(this)->GetIsolate(); - if (IsDeadCheck(isolate, "v8::ArrayBuffer::Data()")) return 0; - i::Handle obj = Utils::OpenHandle(this); - return obj->backing_store(); -} - - Local v8::ArrayBuffer::New(size_t byte_length) { i::Isolate* isolate = i::Isolate::Current(); EnsureInitializedForIsolate(isolate, "v8::ArrayBuffer::New(size_t)"); @@ -6051,7 +6069,7 @@ Local v8::ArrayBuffer::New(void* data, size_t byte_length) { ENTER_V8(isolate); i::Handle obj = isolate->factory()->NewJSArrayBuffer(); - i::Runtime::SetupArrayBuffer(isolate, obj, data, byte_length); + i::Runtime::SetupArrayBuffer(isolate, obj, true, data, byte_length); return Utils::ToLocal(obj); } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 7c9e4366ed..d7b74f8140 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1320,10 +1320,12 @@ void Genesis::InitializeExperimentalGlobal() { if (FLAG_harmony_array_buffer) { // -- A r r a y B u f f e r Handle array_buffer_fun = - InstallFunction(global, "ArrayBuffer", JS_ARRAY_BUFFER_TYPE, - JSArrayBuffer::kSize, - isolate()->initial_object_prototype(), - Builtins::kIllegal, true, true); + InstallFunction( + global, "ArrayBuffer", JS_ARRAY_BUFFER_TYPE, + JSArrayBuffer::kSize + + v8::ArrayBuffer::kInternalFieldCount * kPointerSize, + isolate()->initial_object_prototype(), + Builtins::kIllegal, true, true); native_context()->set_array_buffer_fun(*array_buffer_fun); } diff --git a/src/d8.cc b/src/d8.cc index b95432e269..3a4ba3b2e9 100644 --- a/src/d8.cc +++ b/src/d8.cc @@ -1048,6 +1048,16 @@ static char* ReadChars(Isolate* isolate, const char* name, int* size_out) { return chars; } +static void ReadBufferWeakCallback(v8::Isolate* isolate, + Persistent* object, + uint8_t* data) { + size_t byte_length = ArrayBuffer::Cast(**object)->ByteLength(); + isolate->AdjustAmountOfExternalAllocatedMemory( + -static_cast(byte_length)); + + delete[] data; + object->Dispose(isolate); +} Handle Shell::ReadBuffer(const Arguments& args) { ASSERT(sizeof(char) == sizeof(uint8_t)); // NOLINT @@ -1057,14 +1067,19 @@ Handle Shell::ReadBuffer(const Arguments& args) { return Throw("Error loading file"); } + Isolate* isolate = args.GetIsolate(); uint8_t* data = reinterpret_cast( ReadChars(args.GetIsolate(), *filename, &length)); if (data == NULL) { return Throw("Error reading file"); } - Handle buffer = ArrayBuffer::New(length); - memcpy(buffer->Data(), data, length); - delete[] data; + Handle buffer = ArrayBuffer::New(data, length); + v8::Persistent weak_handle = + v8::Persistent::New(isolate, buffer); + weak_handle.MakeWeak(isolate, data, ReadBufferWeakCallback); + weak_handle.MarkIndependent(); + isolate->AdjustAmountOfExternalAllocatedMemory(length); + return buffer; } diff --git a/src/objects-inl.h b/src/objects-inl.h index e974a0e374..3eab06c69e 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5313,6 +5313,17 @@ void JSArrayBuffer::set_backing_store(void* value, WriteBarrierMode mode) { ACCESSORS(JSArrayBuffer, byte_length, Object, kByteLengthOffset) +ACCESSORS_TO_SMI(JSArrayBuffer, flag, kFlagOffset) + + +bool JSArrayBuffer::is_external() { + return BooleanBit::get(flag(), kIsExternalBit); +} + + +void JSArrayBuffer::set_is_external(bool value) { + set_flag(BooleanBit::set(flag(), kIsExternalBit, value)); +} ACCESSORS(JSTypedArray, buffer, Object, kBufferOffset) @@ -5320,7 +5331,6 @@ ACCESSORS(JSTypedArray, byte_offset, Object, kByteOffsetOffset) ACCESSORS(JSTypedArray, byte_length, Object, kByteLengthOffset) ACCESSORS(JSTypedArray, length, Object, kLengthOffset) - ACCESSORS(JSRegExp, data, Object, kDataOffset) diff --git a/src/objects.h b/src/objects.h index 4524a139dd..269590ae92 100644 --- a/src/objects.h +++ b/src/objects.h @@ -8763,6 +8763,12 @@ class JSArrayBuffer: public JSObject { // [byte_length]: length in bytes DECL_ACCESSORS(byte_length, Object) + // [flags] + DECL_ACCESSORS(flag, Smi) + + inline bool is_external(); + inline void set_is_external(bool value); + // Casting. static inline JSArrayBuffer* cast(Object* obj); @@ -8772,9 +8778,13 @@ class JSArrayBuffer: public JSObject { static const int kBackingStoreOffset = JSObject::kHeaderSize; static const int kByteLengthOffset = kBackingStoreOffset + kPointerSize; - static const int kSize = kByteLengthOffset + kPointerSize; + static const int kFlagOffset = kByteLengthOffset + kPointerSize; + static const int kSize = kFlagOffset + kPointerSize; private: + // Bit position in a flag + static const int kIsExternalBit = 0; + DISALLOW_IMPLICIT_CONSTRUCTORS(JSArrayBuffer); }; diff --git a/src/runtime.cc b/src/runtime.cc index 64573df474..36cd206913 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -658,28 +658,37 @@ static void ArrayBufferWeakCallback(v8::Isolate* external_isolate, Isolate* isolate = reinterpret_cast(external_isolate); HandleScope scope(isolate); Handle internal_object = Utils::OpenHandle(**object); + Handle array_buffer(JSArrayBuffer::cast(*internal_object)); - size_t allocated_length = NumberToSize( - isolate, JSArrayBuffer::cast(*internal_object)->byte_length()); - isolate->heap()->AdjustAmountOfExternalAllocatedMemory( - -static_cast(allocated_length)); - if (data != NULL) + if (!array_buffer->is_external()) { + size_t allocated_length = NumberToSize( + isolate, array_buffer->byte_length()); + isolate->heap()->AdjustAmountOfExternalAllocatedMemory( + -static_cast(allocated_length)); free(data); + } object->Dispose(external_isolate); } -bool Runtime::SetupArrayBuffer(Isolate* isolate, +void Runtime::SetupArrayBuffer(Isolate* isolate, Handle array_buffer, + bool is_external, void* data, size_t allocated_length) { + ASSERT(array_buffer->GetInternalFieldCount() == + v8::ArrayBuffer::kInternalFieldCount); + for (int i = 0; i < v8::ArrayBuffer::kInternalFieldCount; i++) { + array_buffer->SetInternalField(i, Smi::FromInt(0)); + } array_buffer->set_backing_store(data); + array_buffer->set_flag(Smi::FromInt(0)); + array_buffer->set_is_external(is_external); Handle byte_length = isolate->factory()->NewNumberFromSize(allocated_length); CHECK(byte_length->IsSmi() || byte_length->IsHeapNumber()); array_buffer->set_byte_length(*byte_length); - return true; } @@ -696,8 +705,7 @@ bool Runtime::SetupArrayBufferAllocatingData( data = NULL; } - if (!SetupArrayBuffer(isolate, array_buffer, data, allocated_length)) - return false; + SetupArrayBuffer(isolate, array_buffer, false, data, allocated_length); v8::Isolate* external_isolate = reinterpret_cast(isolate); v8::Persistent weak_handle = v8::Persistent::New( diff --git a/src/runtime.h b/src/runtime.h index b2f2e965a1..f23c666766 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -754,8 +754,9 @@ class Runtime : public AllStatic { Handle object, Handle key); - static bool SetupArrayBuffer(Isolate* isolate, + static void SetupArrayBuffer(Isolate* isolate, Handle array_buffer, + bool is_external, void* data, size_t allocated_length); diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index c8f67de0ab..675ac7fd7e 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -2493,7 +2493,7 @@ THREADED_TEST(SymbolProperties) { } -THREADED_TEST(ArrayBuffer) { +THREADED_TEST(ArrayBuffer_ApiInternalToExternal) { i::FLAG_harmony_array_buffer = true; i::FLAG_harmony_typed_arrays = true; @@ -2502,10 +2502,16 @@ THREADED_TEST(ArrayBuffer) { v8::HandleScope handle_scope(isolate); Local ab = v8::ArrayBuffer::New(1024); + CHECK_EQ(1024, ab->ByteLength()); + CHECK(!ab->IsExternal()); HEAP->CollectAllGarbage(i::Heap::kNoGCFlags); - CHECK_EQ(1024, static_cast(ab->ByteLength())); - uint8_t* data = static_cast(ab->Data()); + v8::ArrayBufferContents ab_contents; + ab->Externalize(&ab_contents); + CHECK(ab->IsExternal()); + + CHECK_EQ(1024, static_cast(ab_contents.ByteLength())); + uint8_t* data = static_cast(ab_contents.Data()); ASSERT(data != NULL); env->Global()->Set(v8_str("ab"), ab); @@ -2523,27 +2529,73 @@ THREADED_TEST(ArrayBuffer) { data[1] = 0x11; result = CompileRun("u8[0] + u8[1]"); CHECK_EQ(0xDD, result->Int32Value()); +} - result = CompileRun("var ab1 = new ArrayBuffer(2);" - "var u8_a = new Uint8Array(ab1);" - "u8_a[0] = 0xAA;" - "u8_a[1] = 0xFF; u8_a.buffer"); + +THREADED_TEST(ArrayBuffer_JSInternalToExternal) { + i::FLAG_harmony_array_buffer = true; + i::FLAG_harmony_typed_arrays = true; + + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope handle_scope(isolate); + + + v8::Handle result = + CompileRun("var ab1 = new ArrayBuffer(2);" + "var u8_a = new Uint8Array(ab1);" + "u8_a[0] = 0xAA;" + "u8_a[1] = 0xFF; u8_a.buffer"); Local ab1 = v8::ArrayBuffer::Cast(*result); CHECK_EQ(2, static_cast(ab1->ByteLength())); - uint8_t* ab1_data = static_cast(ab1->Data()); - CHECK_EQ(0xAA, ab1_data[0]); + CHECK(!ab1->IsExternal()); + v8::ArrayBufferContents ab1_contents; + ab1->Externalize(&ab1_contents); + CHECK(ab1->IsExternal()); + + result = CompileRun("ab1.byteLength"); + CHECK_EQ(2, result->Int32Value()); + result = CompileRun("u8_a[0]"); + CHECK_EQ(0xAA, result->Int32Value()); + result = CompileRun("u8_a[1]"); + CHECK_EQ(0xFF, result->Int32Value()); + result = CompileRun("var u8_b = new Uint8Array(ab1);" + "u8_b[0] = 0xBB;" + "u8_a[0]"); + CHECK_EQ(0xBB, result->Int32Value()); + result = CompileRun("u8_b[1]"); + CHECK_EQ(0xFF, result->Int32Value()); + + CHECK_EQ(2, static_cast(ab1_contents.ByteLength())); + uint8_t* ab1_data = static_cast(ab1_contents.Data()); + CHECK_EQ(0xBB, ab1_data[0]); CHECK_EQ(0xFF, ab1_data[1]); ab1_data[0] = 0xCC; ab1_data[1] = 0x11; result = CompileRun("u8_a[0] + u8_a[1]"); CHECK_EQ(0xDD, result->Int32Value()); +} - uint8_t* my_data = new uint8_t[100]; - memset(my_data, 0, 100); - Local ab3 = v8::ArrayBuffer::New(my_data, 100); + +THREADED_TEST(ArrayBuffer_External) { + i::FLAG_harmony_array_buffer = true; + i::FLAG_harmony_typed_arrays = true; + + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope handle_scope(isolate); + + i::ScopedVector my_data(100); + memset(my_data.start(), 0, 100); + Local ab3 = v8::ArrayBuffer::New(my_data.start(), 100); CHECK_EQ(100, static_cast(ab3->ByteLength())); - CHECK_EQ(my_data, ab3->Data()); + CHECK(ab3->IsExternal()); + env->Global()->Set(v8_str("ab3"), ab3); + + v8::Handle result = CompileRun("ab3.byteLength"); + CHECK_EQ(100, result->Int32Value()); + result = CompileRun("var u8_b = new Uint8Array(ab3);" "u8_b[0] = 0xBB;" "u8_b[1] = 0xCC;" @@ -2555,14 +2607,9 @@ THREADED_TEST(ArrayBuffer) { my_data[1] = 0x11; result = CompileRun("u8_b[0] + u8_b[1]"); CHECK_EQ(0xDD, result->Int32Value()); - - delete[] my_data; } - - - THREADED_TEST(HiddenProperties) { LocalContext env; v8::HandleScope scope(env->GetIsolate()); @@ -15460,12 +15507,14 @@ void TypedArrayTestHelper(v8::ExternalArrayType array_type, i::FLAG_harmony_array_buffer = true; i::FLAG_harmony_typed_arrays = true; + i::ScopedVector backing_store(kElementCount+2); + LocalContext env; v8::Isolate* isolate = env->GetIsolate(); v8::HandleScope handle_scope(isolate); - Local ab = - v8::ArrayBuffer::New((kElementCount+2)*sizeof(ElementType)); + Local ab = v8::ArrayBuffer::New( + backing_store.start(), (kElementCount+2)*sizeof(ElementType)); Local ta = TypedArray::New(ab, 2*sizeof(ElementType), kElementCount); CHECK_EQ(kElementCount, static_cast(ta->Length())); @@ -15474,7 +15523,7 @@ void TypedArrayTestHelper(v8::ExternalArrayType array_type, static_cast(ta->ByteLength())); CHECK_EQ(ab, ta->Buffer()); - ElementType* data = static_cast(ab->Data()) + 2; + ElementType* data = backing_store.start() + 2; for (int i = 0; i < kElementCount; i++) { data[i] = static_cast(i); }