Commit 1791d7bb authored by machenbach's avatar machenbach Committed by Commit bot

Revert of [heap] Track length for array buffers to avoid free-ing dependency...

Revert of [heap] Track length for array buffers to avoid free-ing dependency (patchset #2 id:20001 of https://codereview.chromium.org/2122603004/ )

Reason for revert:
[Sheriff] This makes mjsunit/regress/regress-625752 extremely slow on all gc stress bots and leads to timeouts with custom snapshot:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/builds/6602

Original issue's description:
> [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:625748,chromium:625752
> LOG=N
> TEST=test/mjsunit/regress/regress-625752.js
> R=hpayer@chromium.org
>
> Committed: https://crrev.com/ddc75cc1356a58b6cfd63f9da0586e1150496b3d
> Cr-Commit-Position: refs/heads/master@{#37530}

TBR=hpayer@chromium.org,mlippautz@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:625748,chromium:625752

Review-Url: https://codereview.chromium.org/2127483003
Cr-Commit-Position: refs/heads/master@{#37533}
parent 446232f1
...@@ -24,7 +24,7 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) { ...@@ -24,7 +24,7 @@ void ArrayBufferTracker::RegisterNew(Heap* heap, JSArrayBuffer* buffer) {
tracker = page->local_tracker(); tracker = page->local_tracker();
} }
DCHECK_NOT_NULL(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 // We may go over the limit of externally allocated memory here. We call the
// api function to trigger a GC in this case. // api function to trigger a GC in this case.
...@@ -37,31 +37,29 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) { ...@@ -37,31 +37,29 @@ void ArrayBufferTracker::Unregister(Heap* heap, JSArrayBuffer* buffer) {
if (!data) return; if (!data) return;
Page* page = Page::FromAddress(buffer->address()); 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()); base::LockGuard<base::Mutex> guard(page->mutex());
LocalArrayBufferTracker* tracker = page->local_tracker(); LocalArrayBufferTracker* tracker = page->local_tracker();
DCHECK_NOT_NULL(tracker); DCHECK_NOT_NULL(tracker);
length = tracker->Remove(buffer); tracker->Remove(buffer);
} }
heap->update_external_memory(-static_cast<intptr_t>(length)); heap->update_external_memory(-static_cast<intptr_t>(length));
} }
void LocalArrayBufferTracker::Add(Key key, const Value& value) { void LocalArrayBufferTracker::Add(Key key) {
auto ret = array_buffers_.insert(std::make_pair(key, value)); auto ret = array_buffers_.insert(key);
USE(ret); USE(ret);
// Check that we indeed inserted a new value and did not overwrite an existing // Check that we indeed inserted a new value and did not overwrite an existing
// one (which would be a bug). // one (which would be a bug).
DCHECK(ret.second); DCHECK(ret.second);
} }
LocalArrayBufferTracker::Value LocalArrayBufferTracker::Remove(Key key) { void LocalArrayBufferTracker::Remove(Key key) {
TrackingData::iterator it = array_buffers_.find(key); TrackingData::iterator it = array_buffers_.find(key);
// Check that we indeed find a key to remove. // Check that we indeed find a key to remove.
DCHECK(it != array_buffers_.end()); DCHECK(it != array_buffers_.end());
Value value = it->second;
array_buffers_.erase(it); array_buffers_.erase(it);
return value;
} }
} // namespace internal } // namespace internal
......
...@@ -18,10 +18,10 @@ void LocalArrayBufferTracker::Free() { ...@@ -18,10 +18,10 @@ void LocalArrayBufferTracker::Free() {
size_t freed_memory = 0; size_t freed_memory = 0;
for (TrackingData::iterator it = array_buffers_.begin(); for (TrackingData::iterator it = array_buffers_.begin();
it != array_buffers_.end();) { it != array_buffers_.end();) {
JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(it->first); JSArrayBuffer* buffer = reinterpret_cast<JSArrayBuffer*>(*it);
if ((free_mode == kFreeAll) || if ((free_mode == kFreeAll) ||
Marking::IsWhite(Marking::MarkBitFrom(buffer))) { Marking::IsWhite(Marking::MarkBitFrom(buffer))) {
const size_t len = it->second; const size_t len = NumberToSize(heap_->isolate(), buffer->byte_length());
heap_->isolate()->array_buffer_allocator()->Free(buffer->backing_store(), heap_->isolate()->array_buffer_allocator()->Free(buffer->backing_store(),
len); len);
freed_memory += len; freed_memory += len;
...@@ -42,7 +42,7 @@ void LocalArrayBufferTracker::Process(Callback callback) { ...@@ -42,7 +42,7 @@ void LocalArrayBufferTracker::Process(Callback callback) {
size_t freed_memory = 0; size_t freed_memory = 0;
for (TrackingData::iterator it = array_buffers_.begin(); for (TrackingData::iterator it = array_buffers_.begin();
it != array_buffers_.end();) { it != array_buffers_.end();) {
const CallbackResult result = callback(it->first, &new_buffer); const CallbackResult result = callback(*it, &new_buffer);
if (result == kKeepEntry) { if (result == kKeepEntry) {
++it; ++it;
} else if (result == kUpdateEntry) { } else if (result == kUpdateEntry) {
...@@ -57,13 +57,13 @@ void LocalArrayBufferTracker::Process(Callback callback) { ...@@ -57,13 +57,13 @@ void LocalArrayBufferTracker::Process(Callback callback) {
tracker = target_page->local_tracker(); tracker = target_page->local_tracker();
} }
DCHECK_NOT_NULL(tracker); DCHECK_NOT_NULL(tracker);
tracker->Add(new_buffer, it->second); tracker->Add(new_buffer);
if (target_page->InNewSpace()) target_page->mutex()->Unlock(); if (target_page->InNewSpace()) target_page->mutex()->Unlock();
it = array_buffers_.erase(it); it = array_buffers_.erase(it);
} else if (result == kRemoveEntry) { } else if (result == kRemoveEntry) {
const size_t len = it->second; const size_t len = NumberToSize(heap_->isolate(), (*it)->byte_length());
heap_->isolate()->array_buffer_allocator()->Free( heap_->isolate()->array_buffer_allocator()->Free((*it)->backing_store(),
it->first->backing_store(), len); len);
freed_memory += len; freed_memory += len;
it = array_buffers_.erase(it); it = array_buffers_.erase(it);
} else { } else {
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
#ifndef V8_HEAP_ARRAY_BUFFER_TRACKER_H_ #ifndef V8_HEAP_ARRAY_BUFFER_TRACKER_H_
#define 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/allocation.h"
#include "src/base/platform/mutex.h" #include "src/base/platform/mutex.h"
...@@ -60,7 +60,6 @@ class ArrayBufferTracker : public AllStatic { ...@@ -60,7 +60,6 @@ class ArrayBufferTracker : public AllStatic {
class LocalArrayBufferTracker { class LocalArrayBufferTracker {
public: public:
typedef JSArrayBuffer* Key; typedef JSArrayBuffer* Key;
typedef size_t Value;
enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry }; enum CallbackResult { kKeepEntry, kUpdateEntry, kRemoveEntry };
enum FreeMode { kFreeDead, kFreeAll }; enum FreeMode { kFreeDead, kFreeAll };
...@@ -68,8 +67,8 @@ class LocalArrayBufferTracker { ...@@ -68,8 +67,8 @@ class LocalArrayBufferTracker {
explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {} explicit LocalArrayBufferTracker(Heap* heap) : heap_(heap) {}
~LocalArrayBufferTracker(); ~LocalArrayBufferTracker();
inline void Add(Key key, const Value& value); inline void Add(Key key);
inline Value Remove(Key key); inline void Remove(Key key);
// Frees up array buffers determined by |free_mode|. // Frees up array buffers determined by |free_mode|.
template <FreeMode free_mode> template <FreeMode free_mode>
...@@ -90,7 +89,7 @@ class LocalArrayBufferTracker { ...@@ -90,7 +89,7 @@ class LocalArrayBufferTracker {
} }
private: private:
typedef std::unordered_map<Key, Value> TrackingData; typedef std::unordered_set<Key> TrackingData;
Heap* heap_; Heap* heap_;
TrackingData array_buffers_; TrackingData array_buffers_;
......
// 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) {}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment