Reland "[heap] Simplify array buffer tracker"

Retrieve length from the buffer instead of storing in as value in the
map.

This reverts commit a73323d3.

Bug: 
Change-Id: I2f17b1b121380ec3d70f42d7cd0015720d6dc7d1
Reviewed-on: https://chromium-review.googlesource.com/518132Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45611}

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

@@ -14,7 +14,7 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
   void* data = buffer->backing_store();
   if (!data) return;
 
-  size_t length = NumberToSize(buffer->byte_length());
+  size_t length = buffer->allocation_length();
   Page* page = Page::FromAddress(buffer->address());
   {
     base::LockGuard<base::RecursiveMutex> guard(page->mutex());
@@ -24,7 +24,7 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
       tracker = page->local_tracker();
     }
     DCHECK_NOT_NULL(tracker);
-    tracker->Add(buffer, 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,31 +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 = buffer->allocation_length();
   {
     base::LockGuard<base::RecursiveMutex> guard(page->mutex());
     LocalArrayBufferTracker* tracker = page->local_tracker();
     DCHECK_NOT_NULL(tracker);
-    length = tracker->Remove(buffer);
+    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(JSArrayBuffer* buffer) {
+  auto ret = array_buffers_.insert(buffer);
   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) {
-  TrackingData::iterator it = array_buffers_.find(key);
+void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer) {
+  TrackingData::iterator it = array_buffers_.find(buffer);
   // 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

@@ -19,9 +19,9 @@ void LocalArrayBufferTracker::Free(Callback should_free) {
   size_t freed_memory = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(it->first);
+    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
     if (should_free(buffer)) {
-      const size_t len = it->second;
+      const size_t len = buffer->allocation_length();
       buffer->FreeBackingStore();
 
       freed_memory += len;
@@ -42,7 +42,7 @@ void LocalArrayBufferTracker::Process(Callback callback) {
   size_t freed_memory = 0;
   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;
     } else if (result == kUpdateEntry) {
@@ -57,12 +57,13 @@ 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) {
-      const size_t len = it->second;
-      it->first->FreeBackingStore();
+      JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
+      const size_t len = buffer->allocation_length();
+      buffer->FreeBackingStore();
       freed_memory += len;
       it = array_buffers_.erase(it);
     } else {

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"
@@ -60,17 +60,14 @@ class ArrayBufferTracker : public AllStatic {
 // Never use directly but instead always call through |ArrayBufferTracker|.
 class LocalArrayBufferTracker {
  public:
-  typedef JSArrayBuffer* Key;
-  typedef size_t Value;
-
   enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry };
   enum FreeMode { kFreeDead, kFreeAll };
 
   explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {}
   ~LocalArrayBufferTracker();
 
-  inline void Add(Key key, const Value& value);
-  inline Value Remove(Key key);
+  inline void Add(JSArrayBuffer* buffer);
+  inline void Remove(JSArrayBuffer* buffer);
 
   // Frees up array buffers.
   //
@@ -92,14 +89,16 @@ class LocalArrayBufferTracker {
 
   bool IsEmpty() { return array_buffers_.empty(); }
 
-  bool IsTracked(Key key) {
-    return array_buffers_.find(key) != array_buffers_.end();
+  bool IsTracked(JSArrayBuffer* buffer) {
+    return array_buffers_.find(buffer) != array_buffers_.end();
   }
 
  private:
-  typedef std::unordered_map<Key, Value> TrackingData;
+  typedef std::unordered_set<JSArrayBuffer*> TrackingData;
 
   Heap* heap_;
+  // The set contains raw heap pointers which are removed by the GC upon
+  // processing the tracker through its owning page.
   TrackingData array_buffers_;
 };
 

src/heap/mark-compact.cc

@@ -4,6 +4,8 @@
 
 #include "src/heap/mark-compact.h"
 
+#include <unordered_map>
+
 #include "src/base/atomicops.h"
 #include "src/base/bits.h"
 #include "src/base/sys-info.h"

src/objects.cc

@@ -19625,9 +19625,10 @@ Handle<JSArrayBuffer> JSTypedArray::MaterializeArrayBuffer(
   // registration method below handles the case of registering a buffer that has
   // already been promoted.
   buffer->set_backing_store(backing_store);
-  isolate->heap()->RegisterNewArrayBuffer(*buffer);
   buffer->set_allocation_base(backing_store);
   buffer->set_allocation_length(NumberToSize(buffer->byte_length()));
+  // RegisterNewArrayBuffer expects a valid length for adjusting counters.
+  isolate->heap()->RegisterNewArrayBuffer(*buffer);
   memcpy(buffer->backing_store(),
          fixed_typed_array->DataPtr(),
          fixed_typed_array->DataSize());