Reland "[heap] Track length for array buffers to avoid free-ing dependency"

The dependency would only happen if we have a smi overflow for the length and
have create a heap number. In this case the heap number would've to survive
until the array buffer is collected.

To avoid this dependency we track the length (as we previously used to).

BUG=chromium:625752
LOG=N
TEST=test/mjsunit/regress/regress-625752.js
R=hpayer@chromium.org

This reverts commit 1791d7bb.

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

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.
@@ -37,29 +37,31 @@ 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);
   }
   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) {
+LocalArrayBufferTracker::Value 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

@@ -18,10 +18,10 @@ void LocalArrayBufferTracker::Free() {
   size_t freed_memory = 0;
   for (TrackingData::iterator it = array_buffers_.begin();
        it != array_buffers_.end();) {
-    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
+    JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(it->first);
     if ((free_mode == kFreeAll) ||
         Marking::IsWhite(Marking::MarkBitFrom(buffer))) {
-      const size_t len = NumberToSize(heap_->isolate(), buffer->byte_length());
+      const size_t len = it->second;
       heap_->isolate()->array_buffer_allocator()->Free(buffer->backing_store(),
                                                        len);
       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, &new_buffer);
+    const CallbackResult result = callback(it->first, &new_buffer);
     if (result == kKeepEntry) {
       ++it;
     } else if (result == kUpdateEntry) {
@@ -57,13 +57,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);
+      const size_t len = it->second;
+      heap_->isolate()->array_buffer_allocator()->Free(
+          it->first->backing_store(), len);
       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_set>
+#include <unordered_map>
 
 #include "src/allocation.h"
 #include "src/base/platform/mutex.h"
@@ -60,6 +60,7 @@ class ArrayBufferTracker : public AllStatic {
 class LocalArrayBufferTracker {
  public:
   typedef JSArrayBuffer* Key;
+  typedef size_t Value;
 
   enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry };
   enum FreeMode { kFreeDead, kFreeAll };
@@ -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>
@@ -89,7 +90,7 @@ class LocalArrayBufferTracker {
   }
 
  private:
-  typedef std::unordered_set<Key> TrackingData;
+  typedef std::unordered_map<Key, Value> TrackingData;
 
   Heap* heap_;
   TrackingData array_buffers_;

test/mjsunit/mjsunit.status

@@ -391,6 +391,9 @@
   # Issue 3723.
   'regress/regress-3717': [SKIP],
 
+  # BUG(chromium:625752): Too slow.
+  'regress/regress-625752': [SKIP],
+
   # BUG(v8:4237)
   'regress/regress-3976': [SKIP],
 

test/mjsunit/regress/regress-625752.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+v3 = Math.floor(0xFFFFFFFF / 4) + 1;
+Object.prototype.__defineGetter__(1, function() {
+this[1] = Array(0x8000).join();
+})
+try { v38 = new ArrayBuffer(v3); } catch (e) {}
+try { v41 = new Intl.DateTimeFormat(); } catch (e) {}