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

@@ -1568,6 +1568,8 @@ static Object* VisitWeakList(Heap* heap,
       // tail is a live object, visit it.
       WeakListVisitor<T>::VisitLiveObject(
           heap, tail, retainer, record_slots);
+    } else {
+      WeakListVisitor<T>::VisitPhantomObject(heap, candidate);
     }
 
     // Move to next element in the list.
@@ -1599,6 +1601,9 @@ struct WeakListVisitor<JSFunction> {
   static void VisitLiveObject(Heap*, JSFunction*,
                               WeakObjectRetainer*, bool) {
   }
+
+  static void VisitPhantomObject(Heap*, JSFunction*) {
+  }
 };
 
 
@@ -1637,6 +1642,9 @@ struct WeakListVisitor<Context> {
     }
   }
 
+  static void VisitPhantomObject(Heap*, Context*) {
+  }
+
   static int WeakNextOffset() {
     return FixedArray::SizeFor(Context::NEXT_CONTEXT_LINK);
   }
@@ -1680,6 +1688,8 @@ struct WeakListVisitor<JSTypedArray> {
                               WeakObjectRetainer* retainer,
                               bool record_slots) {}
 
+  static void VisitPhantomObject(Heap*, JSTypedArray*) {}
+
   static int WeakNextOffset() {
     return JSTypedArray::kWeakNextOffset;
   }
@@ -1713,6 +1723,10 @@ struct WeakListVisitor<JSArrayBuffer> {
     }
   }
 
+  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();

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();

src/runtime.cc

@@ -650,23 +650,17 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_Fix) {
 }
 
 
-static void ArrayBufferWeakCallback(v8::Isolate* external_isolate,
-                                    Persistent<Value>* object,
-                                    void* data) {
-  Isolate* isolate = reinterpret_cast<Isolate*>(external_isolate);
-  HandleScope scope(isolate);
-  Handle<Object> internal_object = Utils::OpenPersistent(object);
-  Handle<JSArrayBuffer> 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<intptr_t>(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<intptr_t>(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<Object> persistent = isolate->global_handles()->Create(*array_buffer);
-  GlobalHandles::MakeWeak(persistent.location(), data, ArrayBufferWeakCallback);
-  GlobalHandles::MarkIndependent(persistent.location());
-
   isolate->heap()->AdjustAmountOfExternalAllocatedMemory(allocated_length);
 
   return true;

src/runtime.h

@@ -769,6 +769,10 @@ class Runtime : public AllStatic {
       Handle<JSArrayBuffer> array_buffer,
       size_t allocated_length);
 
+  static void FreeArrayBuffer(
+      Isolate* isolate,
+      JSArrayBuffer* phantom_array_buffer);
+
   // Helper functions used stubs.
   static void PerformGC(Object* result);
 

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);
@@ -115,7 +114,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()));
 
@@ -177,7 +174,6 @@ TEST(WeakArrayBuffersFromScript) {
       CompileRun("ab1 = null; ab2 = null; ab3 = null;");
     }
 
-    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<JSTypedArray> 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()));