Skip to content

Commit

Permalink
deps: V8: cherry-pick 163d360 from upstream
Browse files Browse the repository at this point in the history
Original commit message
  [heap] Fix memory leak in the remembered set.

  Empty slot set buckets can leak in the following scenarios.

  Scenario 1 (large object space):
  1) A large array is allocated in the large object space.
  2) The array is filled with old->new references, which
    allocates new slot set buckets.
  3) The references are overwritten with smis or old space
    pointers, which make the slots set buckets empty.
  4) Garbage collection (scavenge or mark-compact) iterates the
    slots set of the array and pre-frees the empty buckets.
  5) Steps 2-4 repeated many times and leak arbitary many empty buckets.
  The fix to free empty buckets for large object space in mark-compact.

  Scenario 2 (no mark-compact):
  1) A small array is allocated in the old space.
  2) The array is filled with old->new references, which allocates new
    slot set buckets.
  3) The references are overwritten with smis or old space pointers,
    which make the slots set buckets empty.
  4) Scavenge iterates the slots set of the array and pre-frees the
    empty buckets.
  5) Steps 2-4 repeated many times and leak arbitary many empty buckets.
  The fix to free empty buckets for swept pages in scavenger.

  Bug: v8:6800
  TBR: [email protected]
  Change-Id: I48d94870f5acf4f6208858271886911c895a9126
  Reviewed-on: https://chromium-review.googlesource.com/668442
  Reviewed-by: Ulan Degenbaev <[email protected]>
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#48041}

PR-URL: #15664
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benedikt Meurer <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
ofrobots authored and MylesBorins committed Oct 11, 2017
1 parent f68f572 commit 1c0ae10
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 10 deletions.
6 changes: 5 additions & 1 deletion deps/v8/src/heap/heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1814,7 +1814,11 @@ void Heap::Scavenge() {
ArrayBufferTracker::FreeDeadInNewSpace(this);

RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(this, [](MemoryChunk* chunk) {
RememberedSet<OLD_TO_NEW>::PreFreeEmptyBuckets(chunk);
if (chunk->SweepingDone()) {
RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
} else {
RememberedSet<OLD_TO_NEW>::PreFreeEmptyBuckets(chunk);
}
});

// Update how much has survived scavenge.
Expand Down
25 changes: 25 additions & 0 deletions deps/v8/src/heap/remembered-set.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,19 @@ class RememberedSet : public AllStatic {
}
}

static int NumberOfPreFreedEmptyBuckets(MemoryChunk* chunk) {
DCHECK(type == OLD_TO_NEW);
int result = 0;
SlotSet* slots = chunk->slot_set<type>();
if (slots != nullptr) {
size_t pages = (chunk->size() + Page::kPageSize - 1) / Page::kPageSize;
for (size_t page = 0; page < pages; page++) {
result += slots[page].NumberOfPreFreedEmptyBuckets();
}
}
return result;
}

static void PreFreeEmptyBuckets(MemoryChunk* chunk) {
DCHECK(type == OLD_TO_NEW);
SlotSet* slots = chunk->slot_set<type>();
Expand All @@ -161,6 +174,18 @@ class RememberedSet : public AllStatic {
}
}

static void FreeEmptyBuckets(MemoryChunk* chunk) {
DCHECK(type == OLD_TO_NEW);
SlotSet* slots = chunk->slot_set<type>();
if (slots != nullptr) {
size_t pages = (chunk->size() + Page::kPageSize - 1) / Page::kPageSize;
for (size_t page = 0; page < pages; page++) {
slots[page].FreeEmptyBuckets();
slots[page].FreeToBeFreedBuckets();
}
}
}

// Given a page and a typed slot in that page, this function adds the slot
// to the remembered set.
static void InsertTyped(Page* page, Address host_addr, SlotType slot_type,
Expand Down
36 changes: 27 additions & 9 deletions deps/v8/src/heap/slot-set.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,32 +225,41 @@ class SlotSet : public Malloced {
return new_count;
}

int NumberOfPreFreedEmptyBuckets() {
base::LockGuard<base::Mutex> guard(&to_be_freed_buckets_mutex_);
return static_cast<int>(to_be_freed_buckets_.size());
}

void PreFreeEmptyBuckets() {
for (int bucket_index = 0; bucket_index < kBuckets; bucket_index++) {
Bucket bucket = LoadBucket(&buckets_[bucket_index]);
if (bucket != nullptr) {
bool found_non_empty_cell = false;
int cell_offset = bucket_index * kBitsPerBucket;
for (int i = 0; i < kCellsPerBucket; i++, cell_offset += kBitsPerCell) {
if (LoadCell(&bucket[i])) {
found_non_empty_cell = true;
break;
}
}
if (!found_non_empty_cell) {
if (IsEmptyBucket(bucket)) {
PreFreeEmptyBucket(bucket_index);
}
}
}
}

void FreeEmptyBuckets() {
for (int bucket_index = 0; bucket_index < kBuckets; bucket_index++) {
Bucket bucket = LoadBucket(&buckets_[bucket_index]);
if (bucket != nullptr) {
if (IsEmptyBucket(bucket)) {
ReleaseBucket(bucket_index);
}
}
}
}

void FreeToBeFreedBuckets() {
base::LockGuard<base::Mutex> guard(&to_be_freed_buckets_mutex_);
while (!to_be_freed_buckets_.empty()) {
Bucket top = to_be_freed_buckets_.top();
to_be_freed_buckets_.pop();
DeleteArray<uint32_t>(top);
}
DCHECK_EQ(0u, to_be_freed_buckets_.size());
}

private:
Expand Down Expand Up @@ -313,6 +322,15 @@ class SlotSet : public Malloced {
}
}

bool IsEmptyBucket(Bucket bucket) {
for (int i = 0; i < kCellsPerBucket; i++) {
if (LoadCell(&bucket[i])) {
return false;
}
}
return true;
}

template <AccessMode access_mode = AccessMode::ATOMIC>
bool SwapInNewBucket(Bucket* bucket, Bucket value) {
if (access_mode == AccessMode::ATOMIC) {
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/heap/spaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3308,6 +3308,7 @@ void LargeObjectSpace::ClearMarkingStateOfLiveObjects() {
Marking::MarkWhite(
ObjectMarking::MarkBitFrom(obj, MarkingState::Internal(obj)));
MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address());
RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
chunk->ResetProgressBar();
MarkingState::Internal(chunk).SetLiveBytes(0);
}
Expand Down
47 changes: 47 additions & 0 deletions deps/v8/test/cctest/heap/test-heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6237,6 +6237,53 @@ HEAP_TEST(Regress5831) {
CHECK(chunk->NeverEvacuate());
}

TEST(Regress6800) {
CcTest::InitializeVM();
Isolate* isolate = CcTest::i_isolate();
HandleScope handle_scope(isolate);

const int kRootLength = 1000;
Handle<FixedArray> root =
isolate->factory()->NewFixedArray(kRootLength, TENURED);
{
HandleScope inner_scope(isolate);
Handle<FixedArray> new_space_array = isolate->factory()->NewFixedArray(1);
for (int i = 0; i < kRootLength; i++) {
root->set(i, *new_space_array);
}
for (int i = 0; i < kRootLength; i++) {
root->set(i, CcTest::heap()->undefined_value());
}
}
CcTest::CollectGarbage(NEW_SPACE);
CHECK_EQ(0, RememberedSet<OLD_TO_NEW>::NumberOfPreFreedEmptyBuckets(
MemoryChunk::FromAddress(root->address())));
}

TEST(Regress6800LargeObject) {
CcTest::InitializeVM();
Isolate* isolate = CcTest::i_isolate();
HandleScope handle_scope(isolate);

const int kRootLength = i::kMaxRegularHeapObjectSize / kPointerSize;
Handle<FixedArray> root =
isolate->factory()->NewFixedArray(kRootLength, TENURED);
CcTest::heap()->lo_space()->Contains(*root);
{
HandleScope inner_scope(isolate);
Handle<FixedArray> new_space_array = isolate->factory()->NewFixedArray(1);
for (int i = 0; i < kRootLength; i++) {
root->set(i, *new_space_array);
}
for (int i = 0; i < kRootLength; i++) {
root->set(i, CcTest::heap()->undefined_value());
}
}
CcTest::CollectGarbage(OLD_SPACE);
CHECK_EQ(0, RememberedSet<OLD_TO_NEW>::NumberOfPreFreedEmptyBuckets(
MemoryChunk::FromAddress(root->address())));
}

HEAP_TEST(RegressMissingWriteBarrierInAllocate) {
if (!FLAG_incremental_marking) return;
FLAG_black_allocation = true;
Expand Down

0 comments on commit 1c0ae10

Please sign in to comment.