reland: [heap] Concurrently free ArrayBuffer allocations.

Free ArrayBuffer backing stores on a background thread, rather than
blocking the main thread after processing. Could potentially cause
contention with the array buffer allocator once JS execution resumes.

The new ArrayBufferCollector class tracks these dead allocations.

Later, the processing of array buffers can happen in parallel.

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng;master.tryserver.v8:v8_linux64_tsan_rel;master.tryserver.v8:v8_linux64_tsan_concurrent_marking_rel_ng;master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel

Bug: v8:6992
Change-Id: I2b74f008f79521414374f607ed510f66508af160
Reviewed-on: https://chromium-review.googlesource.com/779182
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49505}

BUILD.gn

@@ -1617,6 +1617,8 @@ v8_source_set("v8_base") {
     "src/handles.cc",
     "src/handles.h",
     "src/heap-symbols.h",
+    "src/heap/array-buffer-collector.cc",
+    "src/heap/array-buffer-collector.h",
     "src/heap/array-buffer-tracker-inl.h",
     "src/heap/array-buffer-tracker.cc",
     "src/heap/array-buffer-tracker.h",

src/flag-definitions.h

@@ -635,6 +635,8 @@ DEFINE_BOOL(trace_gc_object_stats, false,
             "trace object counts and memory usage")
 DEFINE_BOOL(track_retaining_path, false,
             "enable support for tracking retaining path")
+DEFINE_BOOL(concurrent_array_buffer_freeing, true,
+            "free array buffer allocations on a background thread")
 DEFINE_INT(gc_stats, 0, "Used by tracing internally to enable gc statistics")
 DEFINE_IMPLICATION(trace_gc_object_stats, track_gc_object_stats)
 DEFINE_VALUE_IMPLICATION(track_gc_object_stats, gc_stats, 1)
@@ -1255,6 +1257,7 @@ DEFINE_NEG_IMPLICATION(single_threaded_gc, parallel_pointer_update)
 DEFINE_NEG_IMPLICATION(single_threaded_gc, parallel_scavenge)
 DEFINE_NEG_IMPLICATION(single_threaded_gc, concurrent_store_buffer)
 DEFINE_NEG_IMPLICATION(single_threaded_gc, minor_mc_parallel_marking)
+DEFINE_NEG_IMPLICATION(single_threaded_gc, concurrent_array_buffer_freeing)
 
 #undef FLAG
 

src/heap/array-buffer-collector.cc

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/array-buffer-collector.h"
+
+#include "src/heap/array-buffer-tracker.h"
+#include "src/heap/heap-inl.h"
+
+namespace v8 {
+namespace internal {
+
+void ArrayBufferCollector::AddGarbageAllocations(
+    std::vector<JSArrayBuffer::Allocation>* allocations) {
+  base::LockGuard<base::Mutex> guard(&allocations_mutex_);
+  allocations_.push_back(allocations);
+}
+
+void ArrayBufferCollector::FreeAllocations() {
+  base::LockGuard<base::Mutex> guard(&allocations_mutex_);
+  for (std::vector<JSArrayBuffer::Allocation>* allocations : allocations_) {
+    for (auto alloc : *allocations) {
+      JSArrayBuffer::FreeBackingStore(heap_->isolate(), alloc);
+    }
+    delete allocations;
+  }
+  allocations_.clear();
+}
+
+class ArrayBufferCollector::FreeingTask final : public CancelableTask {
+ public:
+  explicit FreeingTask(Heap* heap)
+      : CancelableTask(heap->isolate()), heap_(heap) {}
+
+  virtual ~FreeingTask() {}
+
+ private:
+  void RunInternal() final {
+    heap_->array_buffer_collector()->FreeAllocations();
+  }
+
+  Heap* heap_;
+};
+
+void ArrayBufferCollector::FreeAllocationsOnBackgroundThread() {
+  heap_->account_external_memory_concurrently_freed();
+  if (heap_->use_tasks() && FLAG_concurrent_array_buffer_freeing) {
+    FreeingTask* task = new FreeingTask(heap_);
+    V8::GetCurrentPlatform()->CallOnBackgroundThread(
+        task, v8::Platform::kShortRunningTask);
+  } else {
+    // Fallback for when concurrency is disabled/restricted.
+    FreeAllocations();
+  }
+}
+
+}  // namespace internal
+}  // namespace v8

src/heap/array-buffer-collector.h

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_ARRAY_BUFFER_COLLECTOR_H_
+#define V8_HEAP_ARRAY_BUFFER_COLLECTOR_H_
+
+#include <vector>
+
+#include "src/base/platform/mutex.h"
+#include "src/objects/js-array.h"
+
+namespace v8 {
+namespace internal {
+
+class Heap;
+
+// To support background processing of array buffer backing stores, we process
+// array buffers using the ArrayBufferTracker class. The ArrayBufferCollector
+// keeps track of garbage backing stores so that they can be freed on a
+// background thread.
+class ArrayBufferCollector {
+ public:
+  explicit ArrayBufferCollector(Heap* heap) : heap_(heap) {}
+
+  ~ArrayBufferCollector() { FreeAllocations(); }
+
+  // These allocations will begin to be freed once FreeAllocations() is called,
+  // or on TearDown.
+  void AddGarbageAllocations(
+      std::vector<JSArrayBuffer::Allocation>* allocations);
+
+  // Calls FreeAllocations() on a background thread.
+  void FreeAllocationsOnBackgroundThread();
+
+ private:
+  class FreeingTask;
+
+  // Begin freeing the allocations added through AddGarbageAllocations. Also
+  // called by TearDown.
+  void FreeAllocations();
+
+  Heap* heap_;
+  base::Mutex allocations_mutex_;
+  std::vector<std::vector<JSArrayBuffer::Allocation>*> allocations_;
+};
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_HEAP_ARRAY_BUFFER_COLLECTOR_H_

src/heap/array-buffer-tracker.cc

@@ -3,6 +3,10 @@
 // found in the LICENSE file.
 
 #include "src/heap/array-buffer-tracker.h"
+
+#include <vector>
+
+#include "src/heap/array-buffer-collector.h"
 #include "src/heap/array-buffer-tracker-inl.h"
 #include "src/heap/heap.h"
 #include "src/heap/spaces.h"
@@ -16,6 +20,9 @@ LocalArrayBufferTracker::~LocalArrayBufferTracker() {
 
 template <typename Callback>
 void LocalArrayBufferTracker::Process(Callback callback) {
+  std::vector<JSArrayBuffer::Allocation>* backing_stores_to_free =
+      new std::vector<JSArrayBuffer::Allocation>();
+
   JSArrayBuffer* new_buffer = nullptr;
   JSArrayBuffer* old_buffer = nullptr;
   size_t new_retained_size = 0;
@@ -45,7 +52,12 @@ void LocalArrayBufferTracker::Process(Callback callback) {
       it = array_buffers_.erase(it);
     } else if (result == kRemoveEntry) {
       // Size of freed memory is computed to avoid looking at dead objects.
-      old_buffer->FreeBackingStore();
+      void* allocation_base = old_buffer->allocation_base();
+      DCHECK_NOT_NULL(allocation_base);
+
+      backing_stores_to_free->emplace_back(allocation_base,
+                                           old_buffer->allocation_length(),
+                                           old_buffer->allocation_mode());
       it = array_buffers_.erase(it);
     } else {
       UNREACHABLE();
@@ -57,16 +69,21 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         static_cast<intptr_t>(freed_memory));
   }
   retained_size_ = new_retained_size;
+
+  // Pass the backing stores that need to be freed to the main thread for later
+  // distribution.
+  // ArrayBufferCollector takes ownership of this pointer.
+  heap_->array_buffer_collector()->AddGarbageAllocations(
+      backing_stores_to_free);
 }
 
-void ArrayBufferTracker::FreeDeadInNewSpace(Heap* heap) {
+void ArrayBufferTracker::PrepareToFreeDeadInNewSpace(Heap* heap) {
   DCHECK_EQ(heap->gc_state(), Heap::HeapState::SCAVENGE);
   for (Page* page : PageRange(heap->new_space()->FromSpaceStart(),
                               heap->new_space()->FromSpaceEnd())) {
     bool empty = ProcessBuffers(page, kUpdateForwardedRemoveOthers);
     CHECK(empty);
   }
-  heap->account_external_memory_concurrently_freed();
 }
 
 size_t ArrayBufferTracker::RetainedInNewSpace(Heap* heap) {

src/heap/array-buffer-tracker.h

@@ -34,9 +34,9 @@ class ArrayBufferTracker : public AllStatic {
   inline static void RegisterNew(Heap* heap, JSArrayBuffer* buffer);
   inline static void Unregister(Heap* heap, JSArrayBuffer* buffer);
 
-  // Frees all backing store pointers for dead JSArrayBuffers in new space.
+  // Identifies all backing store pointers for dead JSArrayBuffers in new space.
   // Does not take any locks and can only be called during Scavenge.
-  static void FreeDeadInNewSpace(Heap* heap);
+  static void PrepareToFreeDeadInNewSpace(Heap* heap);
 
   // Number of array buffer bytes retained from new space.
   static size_t RetainedInNewSpace(Heap* heap);

src/heap/heap.cc

@@ -23,6 +23,7 @@
 #include "src/deoptimizer.h"
 #include "src/feedback-vector.h"
 #include "src/global-handles.h"
+#include "src/heap/array-buffer-collector.h"
 #include "src/heap/array-buffer-tracker-inl.h"
 #include "src/heap/barrier.h"
 #include "src/heap/code-stats.h"
@@ -191,6 +192,7 @@ Heap::Heap()
       last_gc_time_(0.0),
       mark_compact_collector_(nullptr),
       minor_mark_compact_collector_(nullptr),
+      array_buffer_collector_(nullptr),
       memory_allocator_(nullptr),
       store_buffer_(nullptr),
       incremental_marking_(nullptr),
@@ -2059,8 +2061,10 @@ void Heap::Scavenge() {
 
   {
     TRACE_GC(tracer(), GCTracer::Scope::SCAVENGER_PROCESS_ARRAY_BUFFERS);
-    ArrayBufferTracker::FreeDeadInNewSpace(this);
+    ArrayBufferTracker::PrepareToFreeDeadInNewSpace(this);
   }
+  array_buffer_collector()->FreeAllocationsOnBackgroundThread();
+
   RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(this, [](MemoryChunk* chunk) {
     if (chunk->SweepingDone()) {
       RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
@@ -5505,6 +5509,7 @@ bool Heap::SetUp() {
 
   tracer_ = new GCTracer(this);
   minor_mark_compact_collector_ = new MinorMarkCompactCollector(this);
+  array_buffer_collector_ = new ArrayBufferCollector(this);
   gc_idle_time_handler_ = new GCIdleTimeHandler();
   memory_reducer_ = new MemoryReducer(this);
   if (V8_UNLIKELY(FLAG_gc_stats)) {
@@ -5653,6 +5658,11 @@ void Heap::TearDown() {
     minor_mark_compact_collector_ = nullptr;
   }
 
+  if (array_buffer_collector_ != nullptr) {
+    delete array_buffer_collector_;
+    array_buffer_collector_ = nullptr;
+  }
+
   delete incremental_marking_;
   incremental_marking_ = nullptr;
 

src/heap/heap.h

@@ -388,6 +388,7 @@ using v8::MemoryPressureLevel;
   } while (false)
 
 class AllocationObserver;
+class ArrayBufferCollector;
 class ArrayBufferTracker;
 class ConcurrentMarking;
 class GCIdleTimeAction;
@@ -1009,6 +1010,10 @@ class Heap {
     return minor_mark_compact_collector_;
   }
 
+  ArrayBufferCollector* array_buffer_collector() {
+    return array_buffer_collector_;
+  }
+
   // ===========================================================================
   // Root set access. ==========================================================
   // ===========================================================================
@@ -2407,6 +2412,8 @@ class Heap {
   MarkCompactCollector* mark_compact_collector_;
   MinorMarkCompactCollector* minor_mark_compact_collector_;
 
+  ArrayBufferCollector* array_buffer_collector_;
+
   MemoryAllocator* memory_allocator_;
 
   StoreBuffer* store_buffer_;

src/heap/mark-compact.cc

@@ -14,6 +14,7 @@
 #include "src/execution.h"
 #include "src/frames-inl.h"
 #include "src/global-handles.h"
+#include "src/heap/array-buffer-collector.h"
 #include "src/heap/array-buffer-tracker-inl.h"
 #include "src/heap/concurrent-marking.h"
 #include "src/heap/gc-tracer.h"
@@ -4292,6 +4293,7 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
         updating_job.AddTask(new PointersUpdatingTask(isolate()));
       }
       updating_job.Run();
+      heap()->array_buffer_collector()->FreeAllocationsOnBackgroundThread();
     }
   }
 
@@ -4349,6 +4351,7 @@ void MinorMarkCompactCollector::UpdatePointersAfterEvacuation() {
     TRACE_GC(heap()->tracer(),
              GCTracer::Scope::MINOR_MC_EVACUATE_UPDATE_POINTERS_SLOTS);
     updating_job.Run();
+    heap()->array_buffer_collector()->FreeAllocationsOnBackgroundThread();
   }
 
   {

src/objects.cc

@@ -18858,7 +18858,7 @@ void JSArrayBuffer::FreeBackingStore() {
   using AllocationMode = ArrayBuffer::Allocator::AllocationMode;
   const size_t length = allocation_length();
   const AllocationMode mode = allocation_mode();
-  GetIsolate()->array_buffer_allocator()->Free(allocation_base(), length, mode);
+  FreeBackingStore(GetIsolate(), {allocation_base(), length, mode});
 
   // Zero out the backing store and allocation base to avoid dangling
   // pointers.
@@ -18870,6 +18870,12 @@ void JSArrayBuffer::FreeBackingStore() {
   set_allocation_length(0);
 }
 
+// static
+void JSArrayBuffer::FreeBackingStore(Isolate* isolate, Allocation allocation) {
+  isolate->array_buffer_allocator()->Free(allocation.allocation_base,
+                                          allocation.length, allocation.mode);
+}
+
 void JSArrayBuffer::Setup(Handle<JSArrayBuffer> array_buffer, Isolate* isolate,
                           bool is_external, void* data, size_t allocated_length,
                           SharedFlag shared) {

src/objects/js-array.h

@@ -179,7 +179,19 @@ class JSArrayBuffer : public JSObject {
 
   inline ArrayBuffer::Allocator::AllocationMode allocation_mode() const;
 
+  struct Allocation {
+    using AllocationMode = ArrayBuffer::Allocator::AllocationMode;
+
+    Allocation(void* allocation_base, size_t length, AllocationMode mode)
+        : allocation_base(allocation_base), length(length), mode(mode) {}
+
+    void* allocation_base;
+    size_t length;
+    AllocationMode mode;
+  };
+
   void FreeBackingStore();
+  static void FreeBackingStore(Isolate* isolate, Allocation allocation);
 
   V8_EXPORT_PRIVATE static void Setup(
       Handle<JSArrayBuffer> array_buffer, Isolate* isolate, bool is_external,

src/v8.gyp

@@ -990,6 +990,8 @@
         'handles.cc',
         'handles.h',
         'heap-symbols.h',
+        'heap/array-buffer-collector.cc',
+        'heap/array-buffer-collector.h',
         'heap/array-buffer-tracker-inl.h',
         'heap/array-buffer-tracker.cc',
         'heap/array-buffer-tracker.h',