Commit 163d3604 authored by Ulan Degenbaev's avatar Ulan Degenbaev Committed by Commit Bot

[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: mlippautz@chromium.org
Change-Id: I48d94870f5acf4f6208858271886911c895a9126
Reviewed-on: https://chromium-review.googlesource.com/668442Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48041}
parent 37aa13fe
......@@ -2043,7 +2043,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.
......
......@@ -152,6 +152,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>();
......@@ -163,6 +176,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,
......
......@@ -225,25 +225,33 @@ 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()) {
......@@ -251,6 +259,7 @@ class SlotSet : public Malloced {
to_be_freed_buckets_.pop();
DeleteArray<uint32_t>(top);
}
DCHECK_EQ(0u, to_be_freed_buckets_.size());
}
private:
......@@ -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) {
......
......@@ -3370,6 +3370,7 @@ void LargeObjectSpace::ClearMarkingStateOfLiveObjects() {
if (marking_state->IsBlackOrGrey(obj)) {
Marking::MarkWhite(marking_state->MarkBitFrom(obj));
MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address());
RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
chunk->ResetProgressBar();
marking_state->SetLiveBytes(chunk, 0);
}
......
......@@ -5879,6 +5879,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;
ManualGCScope manual_gc_scope;
......
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