Revert of Reland "[heap] Fine-grained JSArrayBuffer tracking" (patchset

#7 id:180001 of https://codereview.chromium.org/2026633003/ )

Reason for revert:
Also failing with the new implementation:
  https://build.chromium.org/p/chromium.gpu/builders/Linux%20Debug%20%28NVIDIA%29/builds/62646

Will do a local repro now.

Original issue's description:
> Track based on JSArrayBuffer addresses on pages instead of the attached
> backing store.
>
> Details of tracking:
> - Scavenge: New space pages are processes in bulk on the main thread
> - MC: Unswept pages are processed in bulk in parallel. All other pages
>   are processed by the sweeper concurrently.
>
> BUG=chromium:611688
> LOG=N
> TEST=cctest/test-array-buffer-tracker/*
> CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_tsan_rel,v8_mac64_asan_rel
>
> Committed: https://crrev.com/279e274eccf95fbb4bd41d908b9153acf6ec118a
> Cr-Commit-Position: refs/heads/master@{#36653}

[email protected]
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:611688

Review-Url: https://codereview.chromium.org/2032973002
Cr-Commit-Position: refs/heads/master@{#36663}

include/v8.h

@@ -7417,7 +7417,7 @@ class Internals {
       kAmountOfExternalAllocatedMemoryOffset + kApiInt64Size;
   static const int kIsolateRootsOffset =
       kAmountOfExternalAllocatedMemoryAtLastGlobalGCOffset + kApiInt64Size +
-      kApiPointerSize + kApiPointerSize;
+      kApiPointerSize;
   static const int kUndefinedValueRootIndex = 4;
   static const int kTheHoleValueRootIndex = 5;
   static const int kNullValueRootIndex = 6;

src/heap/array-buffer-tracker.cc

@@ -5,172 +5,136 @@
 #include "src/heap/array-buffer-tracker.h"
 #include "src/heap/heap.h"
 #include "src/isolate.h"
-#include "src/objects-inl.h"
 #include "src/objects.h"
+#include "src/objects-inl.h"
 #include "src/v8.h"
 
 namespace v8 {
 namespace internal {
 
-LocalArrayBufferTracker::~LocalArrayBufferTracker() {
+ArrayBufferTracker::~ArrayBufferTracker() {
+  Isolate* isolate = heap()->isolate();
   size_t freed_memory = 0;
-  for (auto& buffer : array_buffers_) {
-    heap_->isolate()->array_buffer_allocator()->Free(buffer.second.first,
-                                                     buffer.second.second);
-    freed_memory += buffer.second.second;
+  for (auto& buffer : live_array_buffers_) {
+    isolate->array_buffer_allocator()->Free(buffer.first, buffer.second);
+    freed_memory += buffer.second;
   }
-  if (freed_memory > 0) {
-    heap_->update_amount_of_external_allocated_freed_memory(
-        static_cast<intptr_t>(freed_memory));
+  for (auto& buffer : live_array_buffers_for_scavenge_) {
+    isolate->array_buffer_allocator()->Free(buffer.first, buffer.second);
+    freed_memory += buffer.second;
   }
-  array_buffers_.clear();
-}
-
-void LocalArrayBufferTracker::Add(Key key, const Value& value) {
-  array_buffers_[key] = value;
-}
-
-LocalArrayBufferTracker::Value LocalArrayBufferTracker::Remove(Key key) {
-  DCHECK_EQ(array_buffers_.count(key), 1);
-  Value value = array_buffers_[key];
-  array_buffers_.erase(key);
-  return value;
-}
+  live_array_buffers_.clear();
+  live_array_buffers_for_scavenge_.clear();
+  not_yet_discovered_array_buffers_.clear();
+  not_yet_discovered_array_buffers_for_scavenge_.clear();
 
-void LocalArrayBufferTracker::FreeDead() {
-  size_t freed_memory = 0;
-  for (TrackingMap::iterator it = array_buffers_.begin();
-       it != array_buffers_.end();) {
-    if (Marking::IsWhite(Marking::MarkBitFrom(it->first))) {
-      heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
-                                                       it->second.second);
-      freed_memory += it->second.second;
-      array_buffers_.erase(it++);
-    } else {
-      it++;
-    }
-  }
   if (freed_memory > 0) {
-    heap_->update_amount_of_external_allocated_freed_memory(
-        static_cast<intptr_t>(freed_memory));
+    heap()->update_amount_of_external_allocated_memory(
+        -static_cast<int64_t>(freed_memory));
   }
 }
 
-template <typename Callback>
-void LocalArrayBufferTracker::Process(Callback callback) {
-  JSArrayBuffer* new_buffer = nullptr;
-  size_t freed_memory = 0;
-  for (TrackingMap::iterator it = array_buffers_.begin();
-       it != array_buffers_.end();) {
-    switch (callback(it->first, &new_buffer)) {
-      case kKeepEntry:
-        it++;
-        break;
-      case kUpdateEntry:
-        DCHECK_NOT_NULL(new_buffer);
-        Page::FromAddress(new_buffer->address())
-            ->local_tracker<Page::kCreateIfNotPresent>()
-            ->Add(new_buffer, it->second);
-        array_buffers_.erase(it++);
-        break;
-      case kRemoveEntry:
-        heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
-                                                         it->second.second);
-        freed_memory += it->second.second;
-        array_buffers_.erase(it++);
-        break;
-      default:
-        UNREACHABLE();
-    }
-  }
-  if (freed_memory > 0) {
-    heap_->update_amount_of_external_allocated_freed_memory(
-        static_cast<intptr_t>(freed_memory));
-  }
-}
 
-void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
+void ArrayBufferTracker::RegisterNew(JSArrayBuffer* buffer) {
   void* data = buffer->backing_store();
   if (!data) return;
 
-  size_t length = NumberToSize(heap->isolate(), buffer->byte_length());
-  Page* page = Page::FromAddress(buffer->address());
-  LocalArrayBufferTracker* tracker =
-      page->local_tracker<Page::kCreateIfNotPresent>();
-  DCHECK_NOT_NULL(tracker);
-  {
-    base::LockGuard<base::Mutex> guard(page->mutex());
-    tracker->Add(buffer, std::make_pair(data, length));
+  bool in_new_space = heap()->InNewSpace(buffer);
+  size_t length = NumberToSize(heap()->isolate(), buffer->byte_length());
+  if (in_new_space) {
+    live_array_buffers_for_scavenge_[data] = length;
+  } else {
+    live_array_buffers_[data] = length;
   }
+
   // We may go over the limit of externally allocated memory here. We call the
   // api function to trigger a GC in this case.
-  reinterpret_cast<v8::Isolate*>(heap->isolate())
+  reinterpret_cast<v8::Isolate*>(heap()->isolate())
       ->AdjustAmountOfExternalAllocatedMemory(length);
 }
 
-void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
+
+void ArrayBufferTracker::Unregister(JSArrayBuffer* buffer) {
   void* data = buffer->backing_store();
   if (!data) return;
 
-  Page* page = Page::FromAddress(buffer->address());
-  LocalArrayBufferTracker* tracker = page->local_tracker<Page::kDontCreate>();
-  DCHECK_NOT_NULL(tracker);
-  size_t length = 0;
-  {
-    base::LockGuard<base::Mutex> guard(page->mutex());
-    length = tracker->Remove(buffer).second;
-  }
-  heap->update_amount_of_external_allocated_memory(
-      -static_cast<intptr_t>(length));
+  bool in_new_space = heap()->InNewSpace(buffer);
+  std::map<void*, size_t>* live_buffers =
+      in_new_space ? &live_array_buffers_for_scavenge_ : &live_array_buffers_;
+  std::map<void*, size_t>* not_yet_discovered_buffers =
+      in_new_space ? &not_yet_discovered_array_buffers_for_scavenge_
+                   : &not_yet_discovered_array_buffers_;
+
+  DCHECK(live_buffers->count(data) > 0);
+
+  size_t length = (*live_buffers)[data];
+  live_buffers->erase(data);
+  not_yet_discovered_buffers->erase(data);
+
+  heap()->update_amount_of_external_allocated_memory(
+      -static_cast<int64_t>(length));
 }
 
-void ArrayBufferTracker::FreeDeadInNewSpace(Heap* heap) {
-  NewSpacePageIterator from_it(heap->new_space()->FromSpaceStart(),
-                               heap->new_space()->FromSpaceEnd());
-  while (from_it.has_next()) {
-    ProcessBuffers(from_it.next(), kUpdateForwardedRemoveOthers);
+
+void ArrayBufferTracker::MarkLive(JSArrayBuffer* buffer) {
+  base::LockGuard<base::Mutex> guard(&mutex_);
+  void* data = buffer->backing_store();
+
+  // ArrayBuffer might be in the middle of being constructed.
+  if (data == heap()->undefined_value()) return;
+  if (heap()->InNewSpace(buffer)) {
+    not_yet_discovered_array_buffers_for_scavenge_.erase(data);
+  } else {
+    not_yet_discovered_array_buffers_.erase(data);
   }
-  heap->account_amount_of_external_allocated_freed_memory();
 }
 
-void ArrayBufferTracker::FreeDead(Page* page) {
-  // Only called from the sweeper, which already holds the page lock.
-  LocalArrayBufferTracker* tracker = page->local_tracker<Page::kDontCreate>();
-  if (tracker == nullptr) return;
-  DCHECK(!page->SweepingDone());
-  tracker->FreeDead();
-  if (tracker->IsEmpty()) {
-    page->ReleaseLocalTracker();
+
+void ArrayBufferTracker::FreeDead(bool from_scavenge) {
+  size_t freed_memory = 0;
+  Isolate* isolate = heap()->isolate();
+  for (auto& buffer : not_yet_discovered_array_buffers_for_scavenge_) {
+    isolate->array_buffer_allocator()->Free(buffer.first, buffer.second);
+    freed_memory += buffer.second;
+    live_array_buffers_for_scavenge_.erase(buffer.first);
   }
-}
 
-void ArrayBufferTracker::ProcessBuffers(Page* page, ProcessingMode mode) {
-  LocalArrayBufferTracker* tracker = page->local_tracker<Page::kDontCreate>();
-  if (tracker == nullptr) return;
-  {
-    base::LockGuard<base::Mutex> guard(page->mutex());
-    tracker->Process(
-        [mode](JSArrayBuffer* old_buffer, JSArrayBuffer** new_buffer) {
-          MapWord map_word = old_buffer->map_word();
-          if (map_word.IsForwardingAddress()) {
-            *new_buffer = JSArrayBuffer::cast(map_word.ToForwardingAddress());
-            return LocalArrayBufferTracker::kUpdateEntry;
-          }
-          return mode == kUpdateForwardedKeepOthers
-                     ? LocalArrayBufferTracker::kKeepEntry
-                     : LocalArrayBufferTracker::kRemoveEntry;
-        });
+  if (!from_scavenge) {
+    for (auto& buffer : not_yet_discovered_array_buffers_) {
+      isolate->array_buffer_allocator()->Free(buffer.first, buffer.second);
+      freed_memory += buffer.second;
+      live_array_buffers_.erase(buffer.first);
+    }
   }
+
+  not_yet_discovered_array_buffers_for_scavenge_ =
+      live_array_buffers_for_scavenge_;
+  if (!from_scavenge) not_yet_discovered_array_buffers_ = live_array_buffers_;
+
+  // Do not call through the api as this code is triggered while doing a GC.
+  heap()->update_amount_of_external_allocated_memory(
+      -static_cast<int64_t>(freed_memory));
 }
 
-bool ArrayBufferTracker::IsTracked(JSArrayBuffer* buffer) {
-  Page* page = Page::FromAddress(buffer->address());
-  LocalArrayBufferTracker* tracker =
-      page->local_tracker<Page::kCreateIfNotPresent>();
-  {
-    base::LockGuard<base::Mutex> guard(page->mutex());
-    return tracker->IsTracked(buffer);
-  }
+
+void ArrayBufferTracker::PrepareDiscoveryInNewSpace() {
+  not_yet_discovered_array_buffers_for_scavenge_ =
+      live_array_buffers_for_scavenge_;
+}
+
+
+void ArrayBufferTracker::Promote(JSArrayBuffer* buffer) {
+  base::LockGuard<base::Mutex> guard(&mutex_);
+
+  if (buffer->is_external()) return;
+  void* data = buffer->backing_store();
+  if (!data) return;
+  // ArrayBuffer might be in the middle of being constructed.
+  if (data == heap()->undefined_value()) return;
+  DCHECK(live_array_buffers_for_scavenge_.count(data) > 0);
+  live_array_buffers_[data] = live_array_buffers_for_scavenge_[data];
+  live_array_buffers_for_scavenge_.erase(data);
+  not_yet_discovered_array_buffers_for_scavenge_.erase(data);
 }
 
 }  // namespace internal