Skip to content

Commit

Permalink
Avoid excessive GCs triggered by AdjustAmountOfExternalMemory
Browse files Browse the repository at this point in the history
When the memory pressure level is critical and there are managed objects
that call AdjustAmountOfExternalMemory in their finalizer, we trigger
GC for each dying managed object. See the test for an example.

This fixes the bug by clearing the memory pressure level before GC.

Bug: v8:8014
Change-Id: Id5144430a52fb8545aa23f33229a11b1714cbf10
Reviewed-on: https://chromium-review.googlesource.com/1169011
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55021}
  • Loading branch information
ulan committed Aug 9, 2018
1 parent d407053 commit 27aecd5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/heap/heap.cc
Expand Up @@ -1405,7 +1405,6 @@ bool Heap::CollectGarbage(AllocationSpace space,
if (deserialization_complete_) {
memory_reducer_->NotifyMarkCompact(event);
}
memory_pressure_level_ = MemoryPressureLevel::kNone;
}

tracer()->Stop(collector);
Expand Down Expand Up @@ -3510,9 +3509,14 @@ void Heap::CheckMemoryPressure() {
// The optimizing compiler may be unnecessarily holding on to memory.
isolate()->AbortConcurrentOptimization(BlockingBehavior::kDontBlock);
}
if (memory_pressure_level_ == MemoryPressureLevel::kCritical) {
MemoryPressureLevel memory_pressure_level = memory_pressure_level_;
// Reset the memory pressure level to avoid recursive GCs triggered by
// CheckMemoryPressure from AdjustAmountOfExternalMemory called by
// the finalizers.
memory_pressure_level_ = MemoryPressureLevel::kNone;
if (memory_pressure_level == MemoryPressureLevel::kCritical) {
CollectGarbageOnMemoryPressure();
} else if (memory_pressure_level_ == MemoryPressureLevel::kModerate) {
} else if (memory_pressure_level == MemoryPressureLevel::kModerate) {
if (FLAG_incremental_marking && incremental_marking()->IsStopped()) {
StartIncrementalMarking(kReduceMemoryFootprintMask,
GarbageCollectionReason::kMemoryPressure);
Expand Down
26 changes: 26 additions & 0 deletions test/cctest/heap/test-heap.cc
Expand Up @@ -48,6 +48,7 @@
#include "src/objects-inl.h"
#include "src/objects/js-array-inl.h"
#include "src/objects/js-collection-inl.h"
#include "src/objects/managed.h"
#include "src/regexp/jsregexp.h"
#include "src/snapshot/snapshot.h"
#include "src/transitions.h"
Expand Down Expand Up @@ -6199,6 +6200,31 @@ void HeapTester::UncommitFromSpace(Heap* heap) {
heap->memory_allocator()->unmapper()->EnsureUnmappingCompleted();
}

class DeleteNative {
public:
static void Deleter(void* arg) {
delete reinterpret_cast<DeleteNative*>(arg);
}
};

TEST(Regress8014) {
Isolate* isolate = CcTest::InitIsolateOnce();
Heap* heap = isolate->heap();
{
HandleScope scope(isolate);
for (int i = 0; i < 10000; i++) {
auto handle = Managed<DeleteNative>::FromRawPtr(isolate, 1000000,
new DeleteNative());
USE(handle);
}
}
int ms_count = heap->ms_count();
heap->MemoryPressureNotification(MemoryPressureLevel::kCritical, true);
// Several GCs can be triggred by the above call.
// The bad case triggers 10000 GCs.
CHECK_LE(heap->ms_count(), ms_count + 10);
}

} // namespace heap
} // namespace internal
} // namespace v8
Expand Down

0 comments on commit 27aecd5

Please sign in to comment.