Commit 4d93fae9 authored by Peter Marshall's avatar Peter Marshall Committed by Commit Bot

[gc] Store the backing store pointer in the ArrayBufferTracker

Currently we have to access the unreachable ArrayBuffer object to get
the backing store pointer when we want to free it. This means we need
the original ArrayBuffer object to stay alive until we collect all of
the pointers to free (currently done sequentially after marking).

We want to move this step to a background task that does not block GC
finishing - to do that we need the backing store pointers so that the
original page (where the ArrayBuffers live) can be freed.

Change-Id: Ifaf070d777939cb23c46da637a25d75f9c863bd8
Reviewed-on: https://chromium-review.googlesource.com/1102434
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53915}
parent dd7bf6f7
...@@ -60,11 +60,11 @@ void LocalArrayBufferTracker::Free(Callback should_free) { ...@@ -60,11 +60,11 @@ void LocalArrayBufferTracker::Free(Callback should_free) {
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->first);
const size_t length = it->second; const size_t length = it->second.length;
if (should_free(buffer)) { if (should_free(buffer)) {
JSArrayBuffer::FreeBackingStore( JSArrayBuffer::FreeBackingStore(
isolate, {buffer->backing_store(), length, buffer->backing_store(), isolate, {it->second.backing_store, length, it->second.backing_store,
buffer->allocation_mode(), buffer->is_wasm_memory()}); buffer->allocation_mode(), buffer->is_wasm_memory()});
it = array_buffers_.erase(it); it = array_buffers_.erase(it);
freed_memory += length; freed_memory += length;
...@@ -99,7 +99,7 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) { ...@@ -99,7 +99,7 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
// Track the backing-store usage against the owning Space. // Track the backing-store usage against the owning Space.
space_->IncrementExternalBackingStoreBytes(length); space_->IncrementExternalBackingStoreBytes(length);
auto ret = array_buffers_.insert({buffer, length}); auto ret = array_buffers_.insert({buffer, {buffer->backing_store(), length}});
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).
...@@ -113,7 +113,7 @@ void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer, size_t length) { ...@@ -113,7 +113,7 @@ void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer, size_t length) {
TrackingData::iterator it = array_buffers_.find(buffer); TrackingData::iterator it = array_buffers_.find(buffer);
// 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());
DCHECK_EQ(length, it->second); DCHECK_EQ(length, it->second.length);
array_buffers_.erase(it); array_buffers_.erase(it);
} }
......
...@@ -47,14 +47,14 @@ void LocalArrayBufferTracker::Process(Callback callback) { ...@@ -47,14 +47,14 @@ void LocalArrayBufferTracker::Process(Callback callback) {
const size_t size = NumberToSize(new_buffer->byte_length()); const size_t size = NumberToSize(new_buffer->byte_length());
tracker->Add(new_buffer, size); tracker->Add(new_buffer, size);
} }
moved_memory += it->second; moved_memory += it->second.length;
} else if (result == kRemoveEntry) { } else if (result == kRemoveEntry) {
freed_memory += it->second; freed_memory += it->second.length;
// We pass backing_store() and stored length to the collector for freeing // We pass backing_store() and stored length to the collector for freeing
// the backing store. Wasm allocations will go through their own tracker // the backing store. Wasm allocations will go through their own tracker
// based on the backing store. // based on the backing store.
backing_stores_to_free.emplace_back( backing_stores_to_free.emplace_back(
old_buffer->backing_store(), it->second, old_buffer->backing_store(), it->second.backing_store, it->second.length, it->second.backing_store,
old_buffer->allocation_mode(), old_buffer->is_wasm_memory()); old_buffer->allocation_mode(), old_buffer->is_wasm_memory());
} else { } else {
UNREACHABLE(); UNREACHABLE();
......
...@@ -105,12 +105,18 @@ class LocalArrayBufferTracker { ...@@ -105,12 +105,18 @@ class LocalArrayBufferTracker {
} }
}; };
struct BackingStoreAndLength {
void* backing_store;
size_t length;
};
// Keep track of the backing store and the corresponding length at time of // Keep track of the backing store and the corresponding length at time of
// registering. The length is accessed from JavaScript and can be a // registering. The length is accessed from JavaScript and can be a
// HeapNumber. The reason for tracking the length is that in the case of // HeapNumber. The reason for tracking the length is that in the case of
// length being a HeapNumber, the buffer and its length may be stored on // length being a HeapNumber, the buffer and its length may be stored on
// different memory pages, making it impossible to guarantee order of freeing. // different memory pages, making it impossible to guarantee order of freeing.
typedef std::unordered_map<JSArrayBuffer*, size_t, Hasher> TrackingData; typedef std::unordered_map<JSArrayBuffer*, BackingStoreAndLength, Hasher>
TrackingData;
Space* space_; Space* space_;
// The set contains raw heap pointers which are removed by the GC upon // The set contains raw heap pointers which are removed by the GC upon
......
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