Commit 924cf85e authored by Leon Bettscheider's avatar Leon Bettscheider Committed by V8 LUCI CQ

[heap] IsMarkingComplete only for MajorMC

ShouldFinalize should only be called if major incremental marking is
active, and can crash if minor incremental marking is active, if
MajorMC's local_marking_worklists_ was reset.

The only caller is IsMarkingComplete. This CL changes the IsMarking
check to IsMajorMarking to solve this issue, and renames
IsMarkingComplete to IsMajorMarkingComplete.

Bug: v8:13012
Change-Id: Iba6bd5b7977ec8566c3ab0f047646d8cafd45038
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3879485
Commit-Queue: Leon Bettscheider <bettscheider@google.com>
Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83064}
parent 8641d260
...@@ -470,7 +470,7 @@ GarbageCollector Heap::SelectGarbageCollector(AllocationSpace space, ...@@ -470,7 +470,7 @@ GarbageCollector Heap::SelectGarbageCollector(AllocationSpace space,
} }
if (incremental_marking()->IsMajorMarking() && if (incremental_marking()->IsMajorMarking() &&
incremental_marking()->IsMarkingComplete() && incremental_marking()->IsMajorMarkingComplete() &&
AllocationLimitOvershotByLargeMargin()) { AllocationLimitOvershotByLargeMargin()) {
*reason = "Incremental marking needs finalization"; *reason = "Incremental marking needs finalization";
return GarbageCollector::MARK_COMPACTOR; return GarbageCollector::MARK_COMPACTOR;
...@@ -3792,7 +3792,7 @@ size_t Heap::NewSpaceCapacity() { ...@@ -3792,7 +3792,7 @@ size_t Heap::NewSpaceCapacity() {
void Heap::FinalizeIncrementalMarkingIfComplete( void Heap::FinalizeIncrementalMarkingIfComplete(
GarbageCollectionReason gc_reason) { GarbageCollectionReason gc_reason) {
if (incremental_marking()->IsMarkingComplete()) { if (incremental_marking()->IsMajorMarkingComplete()) {
CollectAllGarbage(current_gc_flags_, gc_reason, current_gc_callback_flags_); CollectAllGarbage(current_gc_flags_, gc_reason, current_gc_callback_flags_);
} }
} }
...@@ -5058,7 +5058,7 @@ bool Heap::ShouldExpandOldGenerationOnSlowAllocation(LocalHeap* local_heap) { ...@@ -5058,7 +5058,7 @@ bool Heap::ShouldExpandOldGenerationOnSlowAllocation(LocalHeap* local_heap) {
if (ShouldOptimizeForLoadTime()) return true; if (ShouldOptimizeForLoadTime()) return true;
if (IsMarkingComplete(local_heap)) { if (IsMajorMarkingComplete(local_heap)) {
return !AllocationLimitOvershotByLargeMargin(); return !AllocationLimitOvershotByLargeMargin();
} }
...@@ -5080,9 +5080,9 @@ bool Heap::IsMainThreadParked(LocalHeap* local_heap) { ...@@ -5080,9 +5080,9 @@ bool Heap::IsMainThreadParked(LocalHeap* local_heap) {
return local_heap->main_thread_parked_; return local_heap->main_thread_parked_;
} }
bool Heap::IsMarkingComplete(LocalHeap* local_heap) { bool Heap::IsMajorMarkingComplete(LocalHeap* local_heap) {
if (!local_heap || !local_heap->is_main_thread()) return false; if (!local_heap || !local_heap->is_main_thread()) return false;
return incremental_marking()->IsMarkingComplete(); return incremental_marking()->IsMajorMarkingComplete();
} }
Heap::HeapGrowingMode Heap::CurrentHeapGrowingMode() { Heap::HeapGrowingMode Heap::CurrentHeapGrowingMode() {
......
...@@ -2011,7 +2011,7 @@ class Heap { ...@@ -2011,7 +2011,7 @@ class Heap {
LocalHeap* local_heap = nullptr); LocalHeap* local_heap = nullptr);
bool IsRetryOfFailedAllocation(LocalHeap* local_heap); bool IsRetryOfFailedAllocation(LocalHeap* local_heap);
bool IsMainThreadParked(LocalHeap* local_heap); bool IsMainThreadParked(LocalHeap* local_heap);
bool IsMarkingComplete(LocalHeap* local_heap); bool IsMajorMarkingComplete(LocalHeap* local_heap);
HeapGrowingMode CurrentHeapGrowingMode(); HeapGrowingMode CurrentHeapGrowingMode();
......
...@@ -757,7 +757,7 @@ void IncrementalMarking::AdvanceOnAllocation() { ...@@ -757,7 +757,7 @@ void IncrementalMarking::AdvanceOnAllocation() {
ScheduleBytesToMarkBasedOnAllocation(); ScheduleBytesToMarkBasedOnAllocation();
Step(kMaxStepSizeInMs, StepOrigin::kV8); Step(kMaxStepSizeInMs, StepOrigin::kV8);
if (IsMarkingComplete()) { if (IsMajorMarkingComplete()) {
// Marking cannot be finalized here. Schedule a completion task instead. // Marking cannot be finalized here. Schedule a completion task instead.
if (!ShouldWaitForTask()) { if (!ShouldWaitForTask()) {
// When task isn't run soon enough, fall back to stack guard to force // When task isn't run soon enough, fall back to stack guard to force
......
...@@ -96,7 +96,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking final { ...@@ -96,7 +96,9 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
bool IsStopped() const { return !IsMarking(); } bool IsStopped() const { return !IsMarking(); }
bool IsMarking() const { return is_marking_; } bool IsMarking() const { return is_marking_; }
bool IsMarkingComplete() const { return IsMarking() && ShouldFinalize(); } bool IsMajorMarkingComplete() const {
return IsMajorMarking() && ShouldFinalize();
}
bool CollectionRequested() const { bool CollectionRequested() const {
return collection_requested_via_stack_guard_; return collection_requested_via_stack_guard_;
......
...@@ -197,7 +197,7 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) { ...@@ -197,7 +197,7 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) {
MarkingBarrier::PublishAll(heap); MarkingBarrier::PublishAll(heap);
marking->MarkRootsForTesting(); marking->MarkRootsForTesting();
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
} }
......
...@@ -2487,7 +2487,7 @@ TEST(InstanceOfStubWriteBarrier) { ...@@ -2487,7 +2487,7 @@ TEST(InstanceOfStubWriteBarrier) {
while (!marking_state->IsBlack(f->code())) { while (!marking_state->IsBlack(f->code())) {
// Discard any pending GC requests otherwise we will get GC when we enter // Discard any pending GC requests otherwise we will get GC when we enter
// code below. // code below.
CHECK(!marking->IsMarkingComplete()); CHECK(!marking->IsMajorMarkingComplete());
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
...@@ -2581,7 +2581,7 @@ TEST(IdleNotificationFinishMarking) { ...@@ -2581,7 +2581,7 @@ TEST(IdleNotificationFinishMarking) {
CHECK_EQ(CcTest::heap()->gc_count(), initial_gc_count); CHECK_EQ(CcTest::heap()->gc_count(), initial_gc_count);
const double kStepSizeInMs = 100; const double kStepSizeInMs = 100;
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
...@@ -3972,7 +3972,7 @@ TEST(IncrementalMarkingStepMakesBigProgressWithLargeObjects) { ...@@ -3972,7 +3972,7 @@ TEST(IncrementalMarkingStepMakesBigProgressWithLargeObjects) {
i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting); i::Heap::kNoGCFlags, i::GarbageCollectionReason::kTesting);
} }
heap::SimulateIncrementalMarking(CcTest::heap()); heap::SimulateIncrementalMarking(CcTest::heap());
CHECK(marking->IsMarkingComplete()); CHECK(marking->IsMajorMarkingComplete());
} }
...@@ -5779,7 +5779,7 @@ TEST(Regress598319) { ...@@ -5779,7 +5779,7 @@ TEST(Regress598319) {
// Now we search for a state where we are in incremental marking and have // Now we search for a state where we are in incremental marking and have
// only partially marked the large object. // only partially marked the large object.
const double kSmallStepSizeInMs = 0.1; const double kSmallStepSizeInMs = 0.1;
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kSmallStepSizeInMs); marking->AdvanceForTesting(kSmallStepSizeInMs);
ProgressBar& progress_bar = page->ProgressBar(); ProgressBar& progress_bar = page->ProgressBar();
if (progress_bar.IsEnabled() && progress_bar.Value() > 0) { if (progress_bar.IsEnabled() && progress_bar.Value() > 0) {
...@@ -5801,10 +5801,10 @@ TEST(Regress598319) { ...@@ -5801,10 +5801,10 @@ TEST(Regress598319) {
// Finish marking with bigger steps to speed up test. // Finish marking with bigger steps to speed up test.
const double kLargeStepSizeInMs = 1000; const double kLargeStepSizeInMs = 1000;
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kLargeStepSizeInMs); marking->AdvanceForTesting(kLargeStepSizeInMs);
} }
CHECK(marking->IsMarkingComplete()); CHECK(marking->IsMajorMarkingComplete());
// All objects need to be black after marking. If a white object crossed the // All objects need to be black after marking. If a white object crossed the
// progress bar, we would fail here. // progress bar, we would fail here.
...@@ -5889,10 +5889,10 @@ TEST(Regress615489) { ...@@ -5889,10 +5889,10 @@ TEST(Regress615489) {
isolate->factory()->NewFixedArray(500, AllocationType::kOld)->Size(); isolate->factory()->NewFixedArray(500, AllocationType::kOld)->Size();
} }
const double kStepSizeInMs = 100; const double kStepSizeInMs = 100;
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
CHECK(marking->IsMarkingComplete()); CHECK(marking->IsMajorMarkingComplete());
intptr_t size_before = heap->SizeOfObjects(); intptr_t size_before = heap->SizeOfObjects();
CcTest::CollectAllGarbage(); CcTest::CollectAllGarbage();
intptr_t size_after = heap->SizeOfObjects(); intptr_t size_after = heap->SizeOfObjects();
...@@ -5947,7 +5947,7 @@ TEST(Regress631969) { ...@@ -5947,7 +5947,7 @@ TEST(Regress631969) {
// Finish incremental marking. // Finish incremental marking.
const double kStepSizeInMs = 100; const double kStepSizeInMs = 100;
IncrementalMarking* marking = heap->incremental_marking(); IncrementalMarking* marking = heap->incremental_marking();
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
......
...@@ -31,7 +31,7 @@ void HeapInternalsBase::SimulateIncrementalMarking(Heap* heap, ...@@ -31,7 +31,7 @@ void HeapInternalsBase::SimulateIncrementalMarking(Heap* heap,
CHECK(marking->IsMarking()); CHECK(marking->IsMarking());
if (!force_completion) return; if (!force_completion) return;
while (!marking->IsMarkingComplete()) { while (!marking->IsMajorMarkingComplete()) {
marking->AdvanceForTesting(kStepSizeInMs); marking->AdvanceForTesting(kStepSizeInMs);
} }
} }
......
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