Reland "Add external backing store JS test"

Allow mocking the limits for ArrayBuffer allocation to simulate operating
system OOM.

Fixes:
- Ensure OS limit > hard limit for external memory. This is necessary as
  any processing below the hard limit is opportunistic. E.g. a running
  sweeper may stall the current marking (GC) round.
- Immediately process AB allocations when under memory pressure. Otherwise,
  the allocations may be stuck in a stalled task. Freeing them upon
  adding them to the collector still enables parallelism if possible.

This reverts commit f3ad6cdb9c.

Bug: chromium:845409
Change-Id: Ic3e458f2af231bae3d53afcfd6002a0347d3f12b
Reviewed-on: https://chromium-review.googlesource.com/1206872
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55656}

src/d8.cc

@@ -135,6 +135,7 @@ class ShellArrayBufferAllocator : public ArrayBufferAllocatorBase {
 
 // ArrayBuffer allocator that never allocates over 10MB.
 class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
+ protected:
   void* Allocate(size_t length) override {
     return ArrayBufferAllocatorBase::Allocate(Adjust(length));
   }
@@ -154,6 +155,39 @@ class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
   }
 };
 
+// ArrayBuffer allocator that can be equipped with a limit to simulate system
+// OOM.
+class MockArrayBufferAllocatiorWithLimit : public MockArrayBufferAllocator {
+ public:
+  explicit MockArrayBufferAllocatiorWithLimit(size_t allocation_limit)
+      : space_left_(allocation_limit) {}
+
+ protected:
+  void* Allocate(size_t length) override {
+    if (length > space_left_) {
+      return nullptr;
+    }
+    space_left_ -= length;
+    return MockArrayBufferAllocator::Allocate(length);
+  }
+
+  void* AllocateUninitialized(size_t length) override {
+    if (length > space_left_) {
+      return nullptr;
+    }
+    space_left_ -= length;
+    return MockArrayBufferAllocator::AllocateUninitialized(length);
+  }
+
+  void Free(void* data, size_t length) override {
+    space_left_ += length;
+    return MockArrayBufferAllocator::Free(data, length);
+  }
+
+ private:
+  std::atomic<size_t> space_left_;
+};
+
 // Predictable v8::Platform implementation. Worker threads are disabled, idle
 // tasks are disallowed, and the time reported by {MonotonicallyIncreasingTime}
 // is deterministic.
@@ -2904,6 +2938,8 @@ bool Shell::SetOptions(int argc, char* argv[]) {
 
   v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
   options.mock_arraybuffer_allocator = i::FLAG_mock_arraybuffer_allocator;
+  options.mock_arraybuffer_allocator_limit =
+      i::FLAG_mock_arraybuffer_allocator_limit;
 
   // Set up isolated source groups.
   options.isolate_sources = new SourceGroup[options.num_isolates];
@@ -3366,8 +3402,18 @@ int Shell::Main(int argc, char* argv[]) {
   Isolate::CreateParams create_params;
   ShellArrayBufferAllocator shell_array_buffer_allocator;
   MockArrayBufferAllocator mock_arraybuffer_allocator;
+  const size_t memory_limit =
+      options.mock_arraybuffer_allocator_limit * options.num_isolates;
+  MockArrayBufferAllocatiorWithLimit mock_arraybuffer_allocator_with_limit(
+      memory_limit >= options.mock_arraybuffer_allocator_limit
+          ? memory_limit
+          : std::numeric_limits<size_t>::max());
   if (options.mock_arraybuffer_allocator) {
-    Shell::array_buffer_allocator = &mock_arraybuffer_allocator;
+    if (memory_limit) {
+      Shell::array_buffer_allocator = &mock_arraybuffer_allocator_with_limit;
+    } else {
+      Shell::array_buffer_allocator = &mock_arraybuffer_allocator;
+    }
   } else {
     Shell::array_buffer_allocator = &shell_array_buffer_allocator;
   }

src/d8.h

@@ -369,6 +369,7 @@ class ShellOptions {
   bool test_shell;
   bool expected_to_throw;
   bool mock_arraybuffer_allocator;
+  size_t mock_arraybuffer_allocator_limit = 0;
   bool enable_inspector;
   int num_isolates;
   v8::ScriptCompiler::CompileOptions compile_options;

src/flag-definitions.h

@@ -1152,6 +1152,9 @@ DEFINE_ARGS(js_arguments,
             "Pass all remaining arguments to the script. Alias for \"--\".")
 DEFINE_BOOL(mock_arraybuffer_allocator, false,
             "Use a mock ArrayBuffer allocator for testing.")
+DEFINE_SIZE_T(mock_arraybuffer_allocator_limit, 0,
+              "Memory limit for mock ArrayBuffer allocator used to simulate "
+              "OOM for testing.")
 
 //
 // GDB JIT integration flags.

src/heap/array-buffer-collector.cc

@@ -12,19 +12,32 @@
 namespace v8 {
 namespace internal {
 
-void ArrayBufferCollector::AddGarbageAllocations(
-    std::vector<JSArrayBuffer::Allocation> allocations) {
-  base::LockGuard<base::Mutex> guard(&allocations_mutex_);
-  allocations_.push_back(std::move(allocations));
+namespace {
+
+void FreeAllocationsHelper(
+    Heap* heap, const std::vector<JSArrayBuffer::Allocation>& allocations) {
+  for (JSArrayBuffer::Allocation alloc : allocations) {
+    JSArrayBuffer::FreeBackingStore(heap->isolate(), alloc);
+  }
 }
 
-void ArrayBufferCollector::FreeAllocations() {
+}  // namespace
+
+void ArrayBufferCollector::QueueOrFreeGarbageAllocations(
+    std::vector<JSArrayBuffer::Allocation> allocations) {
+  if (heap_->ShouldReduceMemory()) {
+    FreeAllocationsHelper(heap_, allocations);
+  } else {
+    base::LockGuard<base::Mutex> guard(&allocations_mutex_);
+    allocations_.push_back(std::move(allocations));
+  }
+}
+
+void ArrayBufferCollector::PerformFreeAllocations() {
   base::LockGuard<base::Mutex> guard(&allocations_mutex_);
   for (const std::vector<JSArrayBuffer::Allocation>& allocations :
        allocations_) {
-    for (JSArrayBuffer::Allocation alloc : allocations) {
-      JSArrayBuffer::FreeBackingStore(heap_->isolate(), alloc);
-    }
+    FreeAllocationsHelper(heap_, allocations);
   }
   allocations_.clear();
 }
@@ -41,21 +54,25 @@ class ArrayBufferCollector::FreeingTask final : public CancelableTask {
     TRACE_BACKGROUND_GC(
         heap_->tracer(),
         GCTracer::BackgroundScope::BACKGROUND_ARRAY_BUFFER_FREE);
-    heap_->array_buffer_collector()->FreeAllocations();
+    heap_->array_buffer_collector()->PerformFreeAllocations();
   }
 
   Heap* heap_;
 };
 
-void ArrayBufferCollector::FreeAllocationsOnBackgroundThread() {
+void ArrayBufferCollector::FreeAllocations() {
   // TODO(wez): Remove backing-store from external memory accounting.
   heap_->account_external_memory_concurrently_freed();
-  if (!heap_->IsTearingDown() && FLAG_concurrent_array_buffer_freeing) {
+  if (!heap_->IsTearingDown() && !heap_->ShouldReduceMemory() &&
+      FLAG_concurrent_array_buffer_freeing) {
     V8::GetCurrentPlatform()->CallOnWorkerThread(
         base::make_unique<FreeingTask>(heap_));
   } else {
-    // Fallback for when concurrency is disabled/restricted.
-    FreeAllocations();
+    // Fallback for when concurrency is disabled/restricted. This is e.g. the
+    // case when the GC should reduce memory. For such GCs the
+    // QueueOrFreeGarbageAllocations() call would immediately free the
+    // allocations and this call would free already queued ones.
+    PerformFreeAllocations();
   }
 }
 

src/heap/array-buffer-collector.h

@@ -23,24 +23,27 @@ class ArrayBufferCollector {
  public:
   explicit ArrayBufferCollector(Heap* heap) : heap_(heap) {}
 
-  ~ArrayBufferCollector() { FreeAllocations(); }
+  ~ArrayBufferCollector() { PerformFreeAllocations(); }
 
-  // These allocations will begin to be freed once FreeAllocations() is called,
-  // or on TearDown.
-  void AddGarbageAllocations(
+  // These allocations will be either
+  // - freed immediately when under memory pressure, or
+  // - queued for freeing in FreeAllocations() or during tear down.
+  //
+  // FreeAllocations() potentially triggers a background task for processing.
+  void QueueOrFreeGarbageAllocations(
       std::vector<JSArrayBuffer::Allocation> allocations);
 
   // Calls FreeAllocations() on a background thread.
-  void FreeAllocationsOnBackgroundThread();
+  void FreeAllocations();
 
  private:
   class FreeingTask;
 
-  // Begin freeing the allocations added through AddGarbageAllocations. Also
-  // called by TearDown.
-  void FreeAllocations();
+  // Begin freeing the allocations added through QueueOrFreeGarbageAllocations.
+  // Also called by TearDown.
+  void PerformFreeAllocations();
 
-  Heap* heap_;
+  Heap* const heap_;
   base::Mutex allocations_mutex_;
   std::vector<std::vector<JSArrayBuffer::Allocation>> allocations_;
 };

src/heap/array-buffer-tracker.cc

@@ -76,9 +76,9 @@ void LocalArrayBufferTracker::Process(Callback callback) {
 
   array_buffers_.swap(kept_array_buffers);
 
-  // Pass the backing stores that need to be freed to the main thread for later
-  // distribution.
-  page_->heap()->array_buffer_collector()->AddGarbageAllocations(
+  // Pass the backing stores that need to be freed to the main thread for
+  // potential later distribution.
+  page_->heap()->array_buffer_collector()->QueueOrFreeGarbageAllocations(
       std::move(backing_stores_to_free));
 }
 

src/heap/heap.cc

@@ -2212,7 +2212,7 @@ void Heap::Scavenge() {
     TRACE_GC(tracer(), GCTracer::Scope::SCAVENGER_PROCESS_ARRAY_BUFFERS);
     ArrayBufferTracker::PrepareToFreeDeadInNewSpace(this);
   }
-  array_buffer_collector()->FreeAllocationsOnBackgroundThread();
+  array_buffer_collector()->FreeAllocations();
 
   RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(this, [](MemoryChunk* chunk) {
     if (chunk->SweepingDone()) {

src/heap/heap.h

@@ -2263,6 +2263,7 @@ class Heap {
 
   // Classes in "heap" can be friends.
   friend class AlwaysAllocateScope;
+  friend class ArrayBufferCollector;
   friend class ConcurrentMarking;
   friend class EphemeronHashTableMarkingTask;
   friend class GCCallbacksScope;

src/heap/mark-compact.cc

@@ -3255,7 +3255,7 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
             GCTracer::BackgroundScope::MC_BACKGROUND_EVACUATE_UPDATE_POINTERS));
       }
       updating_job.Run(isolate()->async_counters());
-      heap()->array_buffer_collector()->FreeAllocationsOnBackgroundThread();
+      heap()->array_buffer_collector()->FreeAllocations();
     }
   }
 
@@ -3758,7 +3758,7 @@ void MinorMarkCompactCollector::UpdatePointersAfterEvacuation() {
     TRACE_GC(heap()->tracer(),
              GCTracer::Scope::MINOR_MC_EVACUATE_UPDATE_POINTERS_SLOTS);
     updating_job.Run(isolate()->async_counters());
-    heap()->array_buffer_collector()->FreeAllocationsOnBackgroundThread();
+    heap()->array_buffer_collector()->FreeAllocations();
   }
 
   {

test/mjsunit/external-backing-store-gc.js

@@ -0,0 +1,13 @@
+// Copyright 2018 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.
+
+// Flags: --mock-arraybuffer-allocator --mock-arraybuffer-allocator-limit=1300000000
+
+// --mock-arraybuffer-allocator-limit should be above the hard limit external
+// for memory. Below that limit anything is opportunistic and may be delayed,
+// e.g., by tasks getting stalled and the event loop not being invoked.
+
+for (var i = 0; i < 1536; i++) {
+  let garbage = new ArrayBuffer(1024*1024);
+}