Commit e14086f0 authored by Bill Budge's avatar Bill Budge Committed by Commit Bot

Revert "[heap] Added per-page array buffer accouting (external memory)."

This reverts commit d4792e8f.

Reason for revert: Breaks V8 Linux - gc stress

https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20gc%20stress/17056

Original change's description:
> [heap] Added per-page array buffer accouting (external memory).
> 
> Bug: chromium:845409
> Change-Id: Ibc568cdc501edf5d84d9c6379aff58be069369af
> Reviewed-on: https://chromium-review.googlesource.com/1114602
> Commit-Queue: Rodrigo Bruno <rfbpb@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54028}

TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org,rfbpb@google.com

Change-Id: I9a354e72df1ab6782bd1c7c4d6b10194bcfaba2b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:845409
Reviewed-on: https://chromium-review.googlesource.com/1115478Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54040}
parent 9aba0159
......@@ -75,8 +75,8 @@ void LocalArrayBufferTracker::Free(Callback should_free) {
}
}
if (freed_memory > 0) {
page_->DecrementExternalBackingStoreBytes(
ExternalBackingStoreType::kArrayBuffer, freed_memory);
// Update the Space with any freed backing-store bytes.
space()->DecrementExternalBackingStoreBytes(freed_memory);
// TODO(wez): Remove backing-store from external memory accounting.
page_->heap()->update_external_memory_concurrently_freed(
......@@ -98,8 +98,8 @@ void ArrayBufferTracker::FreeDead(Page* page, MarkingState* marking_state) {
}
void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
page_->IncrementExternalBackingStoreBytes(
ExternalBackingStoreType::kArrayBuffer, length);
// Track the backing-store usage against the owning Space.
space()->IncrementExternalBackingStoreBytes(length);
auto ret = array_buffers_.insert({buffer, {buffer->backing_store(), length}});
USE(ret);
......@@ -109,8 +109,8 @@ void LocalArrayBufferTracker::Add(JSArrayBuffer* buffer, size_t length) {
}
void LocalArrayBufferTracker::Remove(JSArrayBuffer* buffer, size_t length) {
page_->DecrementExternalBackingStoreBytes(
ExternalBackingStoreType::kArrayBuffer, length);
// Remove the backing-store accounting from the owning Space.
space()->DecrementExternalBackingStoreBytes(length);
TrackingData::iterator it = array_buffers_.find(buffer);
// Check that we indeed find a key to remove.
......
......@@ -30,7 +30,6 @@ void LocalArrayBufferTracker::Process(Callback callback) {
for (TrackingData::iterator it = array_buffers_.begin();
it != array_buffers_.end(); ++it) {
old_buffer = reinterpret_cast<JSArrayBuffer*>(it->first);
Page* old_page = Page::FromAddress(old_buffer->address());
const CallbackResult result = callback(old_buffer, &new_buffer);
if (result == kKeepEntry) {
kept_array_buffers.insert(*it);
......@@ -49,9 +48,6 @@ void LocalArrayBufferTracker::Process(Callback callback) {
tracker->Add(new_buffer, size);
}
moved_memory += it->second.length;
old_page->DecrementExternalBackingStoreBytes(
ExternalBackingStoreType::kArrayBuffer, it->second.length);
} else if (result == kRemoveEntry) {
freed_memory += it->second.length;
// We pass backing_store() and stored length to the collector for freeing
......@@ -60,14 +56,14 @@ void LocalArrayBufferTracker::Process(Callback callback) {
backing_stores_to_free.emplace_back(
it->second.backing_store, it->second.length, it->second.backing_store,
old_buffer->allocation_mode(), old_buffer->is_wasm_memory());
old_page->DecrementExternalBackingStoreBytes(
ExternalBackingStoreType::kArrayBuffer, it->second.length);
} else {
UNREACHABLE();
}
}
if (moved_memory || freed_memory) {
// Update the Space with any moved or freed backing-store bytes.
space()->DecrementExternalBackingStoreBytes(freed_memory + moved_memory);
// TODO(wez): Remove backing-store from external memory accounting.
page_->heap()->update_external_memory_concurrently_freed(
static_cast<intptr_t>(freed_memory));
......
......@@ -631,6 +631,7 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
chunk->young_generation_bitmap_ = nullptr;
chunk->local_tracker_ = nullptr;
chunk->external_backing_store_bytes_[ExternalBackingStoreType::kOther] = 0;
chunk->external_backing_store_bytes_[ExternalBackingStoreType::kArrayBuffer] =
0;
chunk->external_backing_store_bytes_
......@@ -1399,19 +1400,6 @@ void MemoryChunk::ReleaseYoungGenerationBitmap() {
young_generation_bitmap_ = nullptr;
}
void MemoryChunk::IncrementExternalBackingStoreBytes(
ExternalBackingStoreType type, size_t amount) {
external_backing_store_bytes_[type] += amount;
owner()->IncrementExternalBackingStoreBytes(type, amount);
}
void MemoryChunk::DecrementExternalBackingStoreBytes(
ExternalBackingStoreType type, size_t amount) {
DCHECK_GE(external_backing_store_bytes_[type], amount);
external_backing_store_bytes_[type] -= amount;
owner()->DecrementExternalBackingStoreBytes(type, amount);
}
// -----------------------------------------------------------------------------
// PagedSpace implementation
......@@ -1512,6 +1500,9 @@ void PagedSpace::RefillFreeList() {
void PagedSpace::MergeCompactionSpace(CompactionSpace* other) {
base::LockGuard<base::Mutex> guard(mutex());
IncrementExternalBackingStoreBytes(other->ExternalBackingStoreBytes());
other->DecrementExternalBackingStoreBytes(other->ExternalBackingStoreBytes());
DCHECK(identity() == other->identity());
// Unmerged fields:
// area_size_
......@@ -1601,10 +1592,6 @@ size_t PagedSpace::AddPage(Page* page) {
AccountCommitted(page->size());
IncreaseCapacity(page->area_size());
IncreaseAllocatedBytes(page->allocated_bytes(), page);
for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
}
return RelinkFreeListCategories(page);
}
......@@ -1615,10 +1602,6 @@ void PagedSpace::RemovePage(Page* page) {
DecreaseAllocatedBytes(page->allocated_bytes(), page);
DecreaseCapacity(page->area_size());
AccountUncommitted(page->size());
for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
}
}
size_t PagedSpace::ShrinkPageToHighWaterMark(Page* page) {
......@@ -2587,10 +2570,6 @@ void SemiSpace::RemovePage(Page* page) {
}
}
memory_chunk_list_.Remove(page);
for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
DecrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
}
}
void SemiSpace::PrependPage(Page* page) {
......@@ -2599,10 +2578,6 @@ void SemiSpace::PrependPage(Page* page) {
page->set_owner(this);
memory_chunk_list_.PushFront(page);
pages_used_++;
for (size_t i = 0; i < ExternalBackingStoreType::kNumTypes; i++) {
ExternalBackingStoreType t = static_cast<ExternalBackingStoreType>(i);
IncrementExternalBackingStoreBytes(t, page->ExternalBackingStoreBytes(t));
}
}
void SemiSpace::Swap(SemiSpace* from, SemiSpace* to) {
......
......@@ -143,6 +143,7 @@ enum FreeMode { kLinkCategory, kDoNotLinkCategory };
enum class SpaceAccountingMode { kSpaceAccounted, kSpaceUnaccounted };
enum ExternalBackingStoreType {
kOther,
kArrayBuffer,
kExternalString,
kNumTypes
......@@ -537,12 +538,13 @@ class MemoryChunk {
}
}
void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
size_t amount);
void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
size_t amount);
size_t ExternalBackingStoreBytes(ExternalBackingStoreType type) {
return external_backing_store_bytes_[type];
void IncrementExternalBackingStoreBytes(size_t amount,
ExternalBackingStoreType type) {
external_backing_store_bytes_[type] += amount;
}
void DecrementExternalBackingStoreBytes(size_t amount,
ExternalBackingStoreType type) {
external_backing_store_bytes_[type] -= amount;
}
inline uint32_t AddressToMarkbitIndex(Address addr) const {
......@@ -911,6 +913,7 @@ class Space : public Malloced {
max_committed_(0) {
external_backing_store_bytes_ =
new std::atomic<size_t>[ExternalBackingStoreType::kNumTypes];
external_backing_store_bytes_[ExternalBackingStoreType::kOther] = 0;
external_backing_store_bytes_[ExternalBackingStoreType::kArrayBuffer] = 0;
external_backing_store_bytes_[ExternalBackingStoreType::kExternalString] =
0;
......@@ -957,7 +960,7 @@ class Space : public Malloced {
// Returns amount of off-heap memory in-use by objects in this Space.
virtual size_t ExternalBackingStoreBytes(
ExternalBackingStoreType type) const {
ExternalBackingStoreType type = ExternalBackingStoreType::kOther) const {
return external_backing_store_bytes_[type];
}
......@@ -990,13 +993,14 @@ class Space : public Malloced {
committed_ -= bytes;
}
void IncrementExternalBackingStoreBytes(ExternalBackingStoreType type,
size_t amount) {
void IncrementExternalBackingStoreBytes(
size_t amount,
ExternalBackingStoreType type = ExternalBackingStoreType::kOther) {
external_backing_store_bytes_[type] += amount;
}
void DecrementExternalBackingStoreBytes(ExternalBackingStoreType type,
size_t amount) {
DCHECK_GE(external_backing_store_bytes_[type], amount);
void DecrementExternalBackingStoreBytes(
size_t amount,
ExternalBackingStoreType type = ExternalBackingStoreType::kOther) {
external_backing_store_bytes_[type] -= amount;
}
......@@ -2622,7 +2626,8 @@ class NewSpace : public SpaceWithLinearArea {
}
size_t ExternalBackingStoreBytes(
ExternalBackingStoreType type) const override {
ExternalBackingStoreType type =
ExternalBackingStoreType::kOther) const override {
DCHECK_EQ(0, from_space_.ExternalBackingStoreBytes(type));
return to_space_.ExternalBackingStoreBytes(type);
}
......
......@@ -343,17 +343,16 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreases) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
const size_t backing_store_before =
heap->new_space()->ExternalBackingStoreBytes(type);
heap->new_space()->ExternalBackingStoreBytes();
{
const size_t kArraybufferSize = 117;
v8::HandleScope handle_scope(isolate);
Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate, kArraybufferSize);
USE(ab);
const size_t backing_store_after =
heap->new_space()->ExternalBackingStoreBytes(type);
heap->new_space()->ExternalBackingStoreBytes();
CHECK_EQ(kArraybufferSize, backing_store_after - backing_store_before);
}
}
......@@ -363,10 +362,9 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeDecreases) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
const size_t backing_store_before =
heap->new_space()->ExternalBackingStoreBytes(type);
heap->new_space()->ExternalBackingStoreBytes();
{
const size_t kArraybufferSize = 117;
v8::HandleScope handle_scope(isolate);
......@@ -375,7 +373,7 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeDecreases) {
}
heap::GcAndSweep(heap, OLD_SPACE);
const size_t backing_store_after =
heap->new_space()->ExternalBackingStoreBytes(type);
heap->new_space()->ExternalBackingStoreBytes();
CHECK_EQ(0, backing_store_after - backing_store_before);
}
......@@ -388,10 +386,9 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreasesMarkCompact) {
v8::Isolate* isolate = env->GetIsolate();
Heap* heap = reinterpret_cast<Isolate*>(isolate)->heap();
heap::AbandonCurrentlyFreeMemory(heap->old_space());
ExternalBackingStoreType type = ExternalBackingStoreType::kArrayBuffer;
const size_t backing_store_before =
heap->old_space()->ExternalBackingStoreBytes(type);
heap->old_space()->ExternalBackingStoreBytes();
const size_t kArraybufferSize = 117;
{
......@@ -410,13 +407,13 @@ TEST(ArrayBuffer_ExternalBackingStoreSizeIncreasesMarkCompact) {
CcTest::CollectAllGarbage();
const size_t backing_store_after =
heap->old_space()->ExternalBackingStoreBytes(type);
heap->old_space()->ExternalBackingStoreBytes();
CHECK_EQ(kArraybufferSize, backing_store_after - backing_store_before);
}
heap::GcAndSweep(heap, OLD_SPACE);
const size_t backing_store_after =
heap->old_space()->ExternalBackingStoreBytes(type);
heap->old_space()->ExternalBackingStoreBytes();
CHECK_EQ(0, backing_store_after - backing_store_before);
}
......
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