From 51f3e2f055156841e6ca23cc41e91f55f771896c Mon Sep 17 00:00:00 2001 From: "dslomov@chromium.org" Date: Wed, 19 Jun 2013 11:53:30 +0000 Subject: [PATCH] Do not use weak handles for ArrayBuffers. Instead of allocating weak handles to free ArrayBuffer backing store, dispose of memory while walking the weak list of ArrayBuffers on GC. Also, free all array buffers on isolate tear-down. R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/16950013 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@15205 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/heap.cc | 27 +++++++++++++++++++++++++++ src/heap.h | 3 +++ src/runtime.cc | 30 ++++++++++-------------------- src/runtime.h | 4 ++++ test/cctest/test-weaktypedarrays.cc | 8 -------- 5 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/heap.cc b/src/heap.cc index 43c83e4..b95af11 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -1568,6 +1568,8 @@ static Object* VisitWeakList(Heap* heap, // tail is a live object, visit it. WeakListVisitor::VisitLiveObject( heap, tail, retainer, record_slots); + } else { + WeakListVisitor::VisitPhantomObject(heap, candidate); } // Move to next element in the list. @@ -1599,6 +1601,9 @@ struct WeakListVisitor { static void VisitLiveObject(Heap*, JSFunction*, WeakObjectRetainer*, bool) { } + + static void VisitPhantomObject(Heap*, JSFunction*) { + } }; @@ -1637,6 +1642,9 @@ struct WeakListVisitor { } } + static void VisitPhantomObject(Heap*, Context*) { + } + static int WeakNextOffset() { return FixedArray::SizeFor(Context::NEXT_CONTEXT_LINK); } @@ -1680,6 +1688,8 @@ struct WeakListVisitor { WeakObjectRetainer* retainer, bool record_slots) {} + static void VisitPhantomObject(Heap*, JSTypedArray*) {} + static int WeakNextOffset() { return JSTypedArray::kWeakNextOffset; } @@ -1713,6 +1723,10 @@ struct WeakListVisitor { } } + static void VisitPhantomObject(Heap* heap, JSArrayBuffer* phantom) { + Runtime::FreeArrayBuffer(heap->isolate(), phantom); + } + static int WeakNextOffset() { return JSArrayBuffer::kWeakNextOffset; } @@ -1729,6 +1743,17 @@ void Heap::ProcessArrayBuffers(WeakObjectRetainer* retainer, } +void Heap::TearDownArrayBuffers() { + Object* undefined = undefined_value(); + for (Object* o = array_buffers_list(); o != undefined;) { + JSArrayBuffer* buffer = JSArrayBuffer::cast(o); + Runtime::FreeArrayBuffer(isolate(), buffer); + o = buffer->weak_next(); + } + array_buffers_list_ = undefined; +} + + void Heap::VisitExternalResources(v8::ExternalResourceVisitor* visitor) { DisallowHeapAllocation no_allocation; @@ -6869,6 +6894,8 @@ void Heap::TearDown() { PrintF("\n\n"); } + TearDownArrayBuffers(); + isolate_->global_handles()->TearDown(); external_string_table_.TearDown(); diff --git a/src/heap.h b/src/heap.h index 78d9093..3192630 100644 --- a/src/heap.h +++ b/src/heap.h @@ -2192,6 +2192,9 @@ class Heap { void ProcessNativeContexts(WeakObjectRetainer* retainer, bool record_slots); void ProcessArrayBuffers(WeakObjectRetainer* retainer, bool record_slots); + // Called on heap tear-down. + void TearDownArrayBuffers(); + // Record statistics before and after garbage collection. void ReportStatisticsBeforeGC(); void ReportStatisticsAfterGC(); diff --git a/src/runtime.cc b/src/runtime.cc index 0b7a7ca..6f96e8b 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -650,23 +650,17 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_Fix) { } -static void ArrayBufferWeakCallback(v8::Isolate* external_isolate, - Persistent* object, - void* data) { - Isolate* isolate = reinterpret_cast(external_isolate); - HandleScope scope(isolate); - Handle internal_object = Utils::OpenPersistent(object); - Handle array_buffer(JSArrayBuffer::cast(*internal_object)); +void Runtime::FreeArrayBuffer(Isolate* isolate, + JSArrayBuffer* phantom_array_buffer) { + if (phantom_array_buffer->is_external()) return; - if (!array_buffer->is_external()) { - size_t allocated_length = NumberToSize( - isolate, array_buffer->byte_length()); - isolate->heap()->AdjustAmountOfExternalAllocatedMemory( - -static_cast(allocated_length)); - CHECK(V8::ArrayBufferAllocator() != NULL); - V8::ArrayBufferAllocator()->Free(data); - } - object->Dispose(external_isolate); + size_t allocated_length = NumberToSize( + isolate, phantom_array_buffer->byte_length()); + + isolate->heap()->AdjustAmountOfExternalAllocatedMemory( + -static_cast(allocated_length)); + CHECK(V8::ArrayBufferAllocator() != NULL); + V8::ArrayBufferAllocator()->Free(phantom_array_buffer->backing_store()); } @@ -711,10 +705,6 @@ bool Runtime::SetupArrayBufferAllocatingData( SetupArrayBuffer(isolate, array_buffer, false, data, allocated_length); - Handle persistent = isolate->global_handles()->Create(*array_buffer); - GlobalHandles::MakeWeak(persistent.location(), data, ArrayBufferWeakCallback); - GlobalHandles::MarkIndependent(persistent.location()); - isolate->heap()->AdjustAmountOfExternalAllocatedMemory(allocated_length); return true; diff --git a/src/runtime.h b/src/runtime.h index d2daddf..9ecf47a 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -769,6 +769,10 @@ class Runtime : public AllStatic { Handle array_buffer, size_t allocated_length); + static void FreeArrayBuffer( + Isolate* isolate, + JSArrayBuffer* phantom_array_buffer); + // Helper functions used stubs. static void PerformGC(Object* result); diff --git a/test/cctest/test-weaktypedarrays.cc b/test/cctest/test-weaktypedarrays.cc index 3699622..c9af668 100644 --- a/test/cctest/test-weaktypedarrays.cc +++ b/test/cctest/test-weaktypedarrays.cc @@ -105,7 +105,6 @@ TEST(WeakArrayBuffersFromApi) { CHECK(HasArrayBufferInWeakList(isolate->heap(), *iab2)); } isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(1, CountArrayBuffersInWeakList(isolate->heap())); { HandleScope scope2(isolate); @@ -116,7 +115,6 @@ TEST(WeakArrayBuffersFromApi) { } isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(0, CountArrayBuffersInWeakList(isolate->heap())); } @@ -158,7 +156,6 @@ TEST(WeakArrayBuffersFromScript) { i::OS::SNPrintF(source, "ab%d = null;", i); CompileRun(source.start()); isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(2, CountArrayBuffersInWeakList(isolate->heap())); @@ -178,7 +175,6 @@ TEST(WeakArrayBuffersFromScript) { } isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(0, CountArrayBuffersInWeakList(isolate->heap())); } } @@ -206,13 +202,11 @@ void TestTypedArrayFromApi() { CHECK(HasTypedArrayInWeakList(*iab, *ita2)); } isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(1, CountTypedArrays(*iab)); Handle ita1 = v8::Utils::OpenHandle(*ta1); CHECK(HasTypedArrayInWeakList(*iab, *ita1)); } isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(0, CountTypedArrays(*iab)); } @@ -305,7 +299,6 @@ static void TestTypedArrayFromScript(const char* constructor) { i::OS::SNPrintF(source, "ta%d = null;", i); CompileRun(source.start()); isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(1, CountArrayBuffersInWeakList(isolate->heap())); @@ -326,7 +319,6 @@ static void TestTypedArrayFromScript(const char* constructor) { CompileRun("ta1 = null; ta2 = null; ta3 = null;"); isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); - isolate->heap()->CollectAllGarbage(Heap::kNoGCFlags); CHECK_EQ(1, CountArrayBuffersInWeakList(isolate->heap())); -- 2.7.4