Revert of [heap] Optimize ArrayBuffer tracking (patchset #5 id:80001 of...

Revert of [heap] Optimize ArrayBuffer tracking (patchset #5 id:80001 of https://codereview.chromium.org/2107443002/ )

Reason for revert:
Seems to break GPU bots

Original issue's description:
> [heap] Optimize ArrayBuffer tracking
>
> With the current approach we only need to track using an unordered set as we can
> still access the backing store pointer and length by the time we free the
> backing store.
>
> BUG=chromium:619491, chromium:611688
> LOG=N
> R=ulan@chromium.org
>
> Committed: https://crrev.com/8d2ae27808f047ca8b8f90e63a9c8735321d2ad0
> Cr-Commit-Position: refs/heads/master@{#37318}

TBR=ulan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:619491, chromium:611688

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

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, std::make_pair(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.
@@ -37,29 +37,30 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
   if (!data) return;
 
   Page* page = Page::FromAddress(buffer->address());
-  size_t length = NumberToSize(heap->isolate(), buffer->byte_length());
+  size_t length = 0;
   {
     base::LockGuard<base::Mutex> guard(page->mutex());
     LocalArrayBufferTracker* tracker = page->local_tracker();
     DCHECK_NOT_NULL(tracker);
-    tracker->Remove(buffer);
+    length = tracker->Remove(buffer).second;
   }
   heap->update_external_memory(-static_cast<intptr_t>(length));
 }
 
-void LocalArrayBufferTracker::Add(Key key) {
-  auto ret = array_buffers_.insert(key);
+void LocalArrayBufferTracker::Add(Key key, const Value& value) {
+  auto ret = array_buffers_.insert(std::make_pair(key, value));
   USE(ret);
   // Check that we indeed inserted a new value and did not overwrite an existing
   // one (which would be a bug).
   DCHECK(ret.second);
 }
 
-void LocalArrayBufferTracker::Remove(Key key) {
-  TrackingData::iterator it = array_buffers_.find(key);
-  // Check that we indeed find a key to remove.
+LocalArrayBufferTracker::Value LocalArrayBufferTracker::Remove(Key key) {
+  TrackingMap::iterator it = array_buffers_.find(key);
   DCHECK(it != array_buffers_.end());
+  Value value = it->second;
   array_buffers_.erase(it);
+  return value;
 }
 
 }  // namespace internal

src/heap/array-buffer-tracker.cc

@@ -16,18 +16,16 @@ LocalArrayBufferTracker::~LocalArrayBufferTracker() {
 template <LocalArrayBufferTracker::FreeMode free_mode>
 void LocalArrayBufferTracker::Free() {
   size_t freed_memory = 0;
-  for (TrackingData::iterator it = array_buffers_.begin();
+  for (TrackingMap::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
     if ((free_mode == kFreeAll) ||
-        Marking::IsWhite(Marking::MarkBitFrom(buffer))) {
-      const size_t len = NumberToSize(heap_->isolate(), buffer->byte_length());
-      heap_->isolate()->array_buffer_allocator()->Free(buffer->backing_store(),
-                                                       len);
-      freed_memory += len;
+        Marking::IsWhite(Marking::MarkBitFrom(it->first))) {
+      heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
+                                                       it->second.second);
+      freed_memory += it->second.second;
       it = array_buffers_.erase(it);
     } else {
-      ++it;
+      it++;
     }
   }
   if (freed_memory > 0) {
@@ -40,11 +38,11 @@ template <typename Callback>
 void LocalArrayBufferTracker::Process(Callback callback) {
   JSArrayBuffer* new_buffer = nullptr;
   size_t freed_memory = 0;
-  for (TrackingData::iterator it = array_buffers_.begin();
+  for (TrackingMap::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    const CallbackResult result = callback(*it, &new_buffer);
+    const CallbackResult result = callback(it->first, &new_buffer);
     if (result == kKeepEntry) {
-      ++it;
+      it++;
     } else if (result == kUpdateEntry) {
       DCHECK_NOT_NULL(new_buffer);
       Page* target_page = Page::FromAddress(new_buffer->address());
@@ -57,14 +55,13 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         tracker = target_page->local_tracker();
       }
       DCHECK_NOT_NULL(tracker);
-      tracker->Add(new_buffer);
+      tracker->Add(new_buffer, it->second);
       if (target_page->InNewSpace()) target_page->mutex()->Unlock();
       it = array_buffers_.erase(it);
     } else if (result == kRemoveEntry) {
-      const size_t len = NumberToSize(heap_->isolate(), (*it)->byte_length());
-      heap_->isolate()->array_buffer_allocator()->Free((*it)->backing_store(),
-                                                       len);
-      freed_memory += len;
+      heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
+                                                       it->second.second);
+      freed_memory += it->second.second;
       it = array_buffers_.erase(it);
     } else {
       UNREACHABLE();

src/heap/array-buffer-tracker.h

@@ -5,7 +5,7 @@
 #ifndef V8_HEAP_ARRAY_BUFFER_TRACKER_H_
 #define V8_HEAP_ARRAY_BUFFER_TRACKER_H_
 
-#include <unordered_set>
+#include <unordered_map>
 
 #include "src/allocation.h"
 #include "src/base/platform/mutex.h"
@@ -59,6 +59,7 @@ class ArrayBufferTracker : public AllStatic {
 // Never use directly but instead always call through |ArrayBufferTracker|.
 class LocalArrayBufferTracker {
  public:
+  typedef std::pair<void*, size_t> Value;
   typedef JSArrayBuffer* Key;
 
   enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry };
@@ -67,8 +68,8 @@ class LocalArrayBufferTracker {
   explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {}
   ~LocalArrayBufferTracker();
 
-  inline void Add(Key key);
-  inline void Remove(Key key);
+  inline void Add(Key key, const Value& value);
+  inline Value Remove(Key key);
 
   // Frees up array buffers determined by |free_mode|.
   template <FreeMode free_mode>
@@ -80,7 +81,7 @@ class LocalArrayBufferTracker {
   // Callback should be of type:
   //   CallbackResult fn(JSArrayBuffer* buffer, JSArrayBuffer** new_buffer);
   template <typename Callback>
-  void Process(Callback callback);
+  inline void Process(Callback callback);
 
   bool IsEmpty() { return array_buffers_.empty(); }
 
@@ -89,10 +90,10 @@ class LocalArrayBufferTracker {
   }
 
  private:
-  typedef std::unordered_set<Key> TrackingData;
+  typedef std::unordered_map<Key, Value> TrackingMap;
 
   Heap* heap_;
-  TrackingData array_buffers_;
+  TrackingMap array_buffers_;
 };
 
 }  // namespace internal