[heap] Add retained size counter to array buffer tracking

Bug: 
Change-Id: If4a5408f8ff63a8a2b189f60fda37896a9403d3d
Reviewed-on: https://chromium-review.googlesource.com/519386Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45646}

src/heap/array-buffer-tracker-inl.h

@@ -24,7 +24,7 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
       tracker = page->local_tracker();
     }
     DCHECK_NOT_NULL(tracker);
-    tracker->Add(buffer);
+    tracker->Add(buffer, length);
   }
   // We may go over the limit of externally allocated memory here. We call the
   // api function to trigger a GC in this case.
@@ -42,12 +42,14 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
     base::LockGuard<base::RecursiveMutex> guard(page->mutex());
     LocalArrayBufferTracker* tracker = page->local_tracker();
     DCHECK_NOT_NULL(tracker);
-    tracker->Remove(buffer);
+    tracker->Remove(buffer, length);
   }
   heap->update_external_memory(-static_cast<intptr_t>(length));
 }
 
-void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer) {
+void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
+  DCHECK_GE(retained_size_ + length, retained_size_);
+  retained_size_ += length;
   auto ret = array_buffers_.insert(buffer);
   USE(ret);
   // Check that we indeed inserted a new value and did not overwrite an existing
@@ -55,7 +57,9 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer) {
   DCHECK(ret.second);
 }
 
-void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer) {
+void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer, size_t length) {
+  DCHECK_GE(retained_size_, retained_size_ - length);
+  retained_size_ -= length;
   TrackingData::iterator it = array_buffers_.find(buffer);
   // Check that we indeed find a key to remove.
   DCHECK(it != array_buffers_.end());

src/heap/array-buffer-tracker.cc

@@ -17,19 +17,21 @@ LocalArrayBufferTracker::~LocalArrayBufferTracker() {
 template <typename Callback>
 void LocalArrayBufferTracker::Free(Callback should_free) {
   size_t freed_memory = 0;
+  size_t retained_size = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
     JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
+    const size_t length = buffer->allocation_length();
     if (should_free(buffer)) {
-      const size_t len = buffer->allocation_length();
+      freed_memory += length;
       buffer->FreeBackingStore();
-
-      freed_memory += len;
       it = array_buffers_.erase(it);
     } else {
+      retained_size += length;
       ++it;
     }
   }
+  retained_size_ = retained_size;
   if (freed_memory > 0) {
     heap_->update_external_memory_concurrently_freed(
         static_cast<intptr_t>(freed_memory));
@@ -39,11 +41,16 @@ void LocalArrayBufferTracker::Free(Callback should_free) {
 template <typename Callback>
 void LocalArrayBufferTracker::Process(Callback callback) {
   JSArrayBuffer* new_buffer = nullptr;
+  JSArrayBuffer* old_buffer = nullptr;
   size_t freed_memory = 0;
+  size_t retained_size = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    const CallbackResult result = callback(*it, &new_buffer);
+    old_buffer = reinterpret_cast<JSArrayBuffer*>(*it);
+    const size_t length = old_buffer->allocation_length();
+    const CallbackResult result = callback(old_buffer, &new_buffer);
     if (result == kKeepEntry) {
+      retained_size += length;
       ++it;
     } else if (result == kUpdateEntry) {
       DCHECK_NOT_NULL(new_buffer);
@@ -57,19 +64,19 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         tracker = target_page->local_tracker();
       }
       DCHECK_NOT_NULL(tracker);
-      tracker->Add(new_buffer);
+      DCHECK_EQ(length, new_buffer->allocation_length());
+      tracker->Add(new_buffer, length);
       if (target_page->InNewSpace()) target_page->mutex()->Unlock();
       it = array_buffers_.erase(it);
     } else if (result == kRemoveEntry) {
-      JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
-      const size_t len = buffer->allocation_length();
-      buffer->FreeBackingStore();
-      freed_memory += len;
+      freed_memory += length;
+      old_buffer->FreeBackingStore();
       it = array_buffers_.erase(it);
     } else {
       UNREACHABLE();
     }
   }
+  retained_size_ = retained_size;
   if (freed_memory > 0) {
     heap_->update_external_memory_concurrently_freed(
         static_cast<intptr_t>(freed_memory));
@@ -86,6 +93,17 @@ void ArrayBufferTracker::FreeDeadInNewSpace(Heap* heap) {
   heap->account_external_memory_concurrently_freed();
 }
 
+size_t ArrayBufferTracker::RetainedInNewSpace(Heap* heap) {
+  size_t retained_size = 0;
+  for (Page* page : PageRange(heap->new_space()->ToSpaceStart(),
+                              heap->new_space()->ToSpaceEnd())) {
+    LocalArrayBufferTracker* tracker = page->local_tracker();
+    if (tracker == nullptr) continue;
+    retained_size += tracker->retained_size();
+  }
+  return retained_size;
+}
+
 void ArrayBufferTracker::FreeDead(Page* page,
                                   const MarkingState& marking_state) {
   // Callers need to ensure having the page lock.

src/heap/array-buffer-tracker.h

@@ -38,6 +38,9 @@ class ArrayBufferTracker : public AllStatic {
   // Does not take any locks and can only be called during Scavenge.
   static void FreeDeadInNewSpace(Heap* heap);
 
+  // Number of array buffer bytes retained from new space.
+  static size_t RetainedInNewSpace(Heap* heap);
+
   // Frees all backing store pointers for dead JSArrayBuffer on a given page.
   // Requires marking information to be present. Requires the page lock to be
   // taken by the caller.
@@ -63,11 +66,12 @@ class LocalArrayBufferTracker {
   enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry };
   enum FreeMode { kFreeDead, kFreeAll };
 
-  explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {}
+  explicit LocalArrayBufferTracker(Heap* heap)
+      : heap_(heap), retained_size_(0) {}
   ~LocalArrayBufferTracker();
 
-  inline void Add(JSArrayBuffer* buffer);
-  inline void Remove(JSArrayBuffer* buffer);
+  inline void Add(JSArrayBuffer* buffer, size_t length);
+  inline void Remove(JSArrayBuffer* buffer, size_t length);
 
   // Frees up array buffers.
   //
@@ -87,12 +91,14 @@ class LocalArrayBufferTracker {
   template <typename Callback>
   void Process(Callback callback);
 
-  bool IsEmpty() { return array_buffers_.empty(); }
+  bool IsEmpty() const { return array_buffers_.empty(); }
 
-  bool IsTracked(JSArrayBuffer* buffer) {
+  bool IsTracked(JSArrayBuffer* buffer) const {
     return array_buffers_.find(buffer) != array_buffers_.end();
   }
 
+  size_t retained_size() const { return retained_size_; }
+
  private:
   typedef std::unordered_set<JSArrayBuffer*> TrackingData;
 
@@ -100,6 +106,8 @@ class LocalArrayBufferTracker {
   // The set contains raw heap pointers which are removed by the GC upon
   // processing the tracker through its owning page.
   TrackingData array_buffers_;
+  // Retained size of array buffers for this tracker in bytes.
+  size_t retained_size_;
 };
 
 }  // namespace internal

test/cctest/heap/test-array-buffer-tracker.cc

@@ -329,5 +329,40 @@ UNINITIALIZED_TEST(ArrayBuffer_SemiSpaceCopyMultipleTasks) {
   }
 }
 
+TEST(ArrayBuffer_RetainedSizeIncreases) {
+  CcTest::InitializeVM();
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
+
+  const size_t retained_before = ArrayBufferTracker::RetainedInNewSpace(heap);
+  {
+    const size_t kArraybufferSize = 117;
+    v8::HandleScope handle_scope(isolate);
+    Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, kArraybufferSize);
+    USE(ab);
+    const size_t retained_after = ArrayBufferTracker::RetainedInNewSpace(heap);
+    CHECK_EQ(kArraybufferSize, retained_after - retained_before);
+  }
+}
+
+TEST(ArrayBuffer_RetainedSizeDecreases) {
+  CcTest::InitializeVM();
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
+
+  const size_t retained_before = ArrayBufferTracker::RetainedInNewSpace(heap);
+  {
+    const size_t kArraybufferSize = 117;
+    v8::HandleScope handle_scope(isolate);
+    Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, kArraybufferSize);
+    USE(ab);
+  }
+  heap::GcAndSweep(heap, OLD_SPACE);
+  const size_t retained_after = ArrayBufferTracker::RetainedInNewSpace(heap);
+  CHECK_EQ(0, retained_after - retained_before);
+}
+
 }  // namespace internal
 }  // namespace v8

test/mjsunit/regress/regress-crbug-380671.js

@@ -2,7 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --mock-arraybuffer-allocator
+// Flags: --mock-arraybuffer-allocator --expose-gc
 
 var buffer = new ArrayBuffer(0xc0000000);
 assertEquals(0xc0000000, buffer.byteLength);
+// We call the GC here to free up the large array buffer. Otherwise, the
+// mock allocator would allow us to allocate more than the physical memory
+// available on 32bit platforms, leaving the internal counters in an invalid
+// state.
+gc();