Reland "[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.

Reland:
The issue was fixed in 67b5a501.

BUG=chromium:619491, chromium:611688
LOG=N
R=ulan@chromium.org

This reverts commit 0e1eaec7.

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

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, std::make_pair(data, length));
+    tracker->Add(buffer);
   }
   // We may go over the limit of externally allocated memory here. We call the
   // api function to trigger a GC in this case.
@@ -37,30 +37,29 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
   if (!data) return;
 
   Page* page = Page::FromAddress(buffer->address());
-  size_t length = 0;
+  size_t length = NumberToSize(heap->isolate(), buffer->byte_length());
   {
     base::LockGuard<base::Mutex> guard(page->mutex());
     LocalArrayBufferTracker* tracker = page->local_tracker();
     DCHECK_NOT_NULL(tracker);
-    length = tracker->Remove(buffer).second;
+    tracker->Remove(buffer);
   }
   heap->update_external_memory(-static_cast<intptr_t>(length));
 }
 
-void LocalArrayBufferTracker::Add(Key key, const Value& value) {
-  auto ret = array_buffers_.insert(std::make_pair(key, value));
+void LocalArrayBufferTracker::Add(Key key) {
+  auto ret = array_buffers_.insert(key);
   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);
 }
 
-LocalArrayBufferTracker::Value LocalArrayBufferTracker::Remove(Key key) {
-  TrackingMap::iterator it = array_buffers_.find(key);
+void LocalArrayBufferTracker::Remove(Key key) {
+  TrackingData::iterator it = array_buffers_.find(key);
+  // Check that we indeed find a key to remove.
   DCHECK(it != array_buffers_.end());
-  Value value = it->second;
   array_buffers_.erase(it);
-  return value;
 }
 
 }  // namespace internal

src/heap/array-buffer-tracker.cc

@@ -16,16 +16,18 @@ LocalArrayBufferTracker::~LocalArrayBufferTracker() {
 template <LocalArrayBufferTracker::FreeMode free_mode>
 void LocalArrayBufferTracker::Free() {
   size_t freed_memory = 0;
-  for (TrackingMap::iterator it = array_buffers_.begin();
+  for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
+    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
     if ((free_mode == kFreeAll) ||
-        Marking::IsWhite(Marking::MarkBitFrom(it->first))) {
-      heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
-                                                       it->second.second);
-      freed_memory += it->second.second;
+        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;
       it = array_buffers_.erase(it);
     } else {
-      it++;
+      ++it;
     }
   }
   if (freed_memory > 0) {
@@ -38,11 +40,11 @@ template <typename Callback>
 void LocalArrayBufferTracker::Process(Callback callback) {
   JSArrayBuffer* new_buffer = nullptr;
   size_t freed_memory = 0;
-  for (TrackingMap::iterator it = array_buffers_.begin();
+  for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    const CallbackResult result = callback(it->first, &new_buffer);
+    const CallbackResult result = callback(*it, &new_buffer);
     if (result == kKeepEntry) {
-      it++;
+      ++it;
     } else if (result == kUpdateEntry) {
       DCHECK_NOT_NULL(new_buffer);
       Page* target_page = Page::FromAddress(new_buffer->address());
@@ -55,13 +57,14 @@ void LocalArrayBufferTracker::Process(Callback callback) {
         tracker = target_page->local_tracker();
       }
       DCHECK_NOT_NULL(tracker);
-      tracker->Add(new_buffer, it->second);
+      tracker->Add(new_buffer);
       if (target_page->InNewSpace()) target_page->mutex()->Unlock();
       it = array_buffers_.erase(it);
     } else if (result == kRemoveEntry) {
-      heap_->isolate()->array_buffer_allocator()->Free(it->second.first,
-                                                       it->second.second);
-      freed_memory += it->second.second;
+      const size_t len = NumberToSize(heap_->isolate(), (*it)->byte_length());
+      heap_->isolate()->array_buffer_allocator()->Free((*it)->backing_store(),
+                                                       len);
+      freed_memory += len;
       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_map>
+#include <unordered_set>
 
 #include "src/allocation.h"
 #include "src/base/platform/mutex.h"
@@ -59,7 +59,6 @@ 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 };
@@ -68,8 +67,8 @@ class LocalArrayBufferTracker {
   explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {}
   ~LocalArrayBufferTracker();
 
-  inline void Add(Key key, const Value& value);
-  inline Value Remove(Key key);
+  inline void Add(Key key);
+  inline void Remove(Key key);
 
   // Frees up array buffers determined by |free_mode|.
   template <FreeMode free_mode>
@@ -81,7 +80,7 @@ class LocalArrayBufferTracker {
   // Callback should be of type:
   //   CallbackResult fn(JSArrayBuffer* buffer, JSArrayBuffer** new_buffer);
   template <typename Callback>
-  inline void Process(Callback callback);
+  void Process(Callback callback);
 
   bool IsEmpty() { return array_buffers_.empty(); }
 
@@ -90,10 +89,10 @@ class LocalArrayBufferTracker {
   }
 
  private:
-  typedef std::unordered_map<Key, Value> TrackingMap;
+  typedef std::unordered_set<Key> TrackingData;
 
   Heap* heap_;
-  TrackingMap array_buffers_;
+  TrackingData array_buffers_;
 };
 
 }  // namespace internal