Commit 0f8fe4e5 authored by Dominik Inführ's avatar Dominik Inführ Committed by Commit Bot

Revert "[heap] Remove SWEEPING phase in incremental marking"

This reverts commit 2afb00c0.

Reason for revert: Some tests started to timeout.

Original change's description:
> [heap] Remove SWEEPING phase in incremental marking
>
> The SWEEPING phase in incremental marking was used to finish sweeping
> of the last GC cycle concurrently before starting incremental marking.
> This avoids potentially long pauses when starting incremental marking.
> However this shouldn't be necessary in most cases where sweeping is
> already finished when starting the next cycle. The implementation also
> didn't cleanly separate the GC cycles.
>
> In case the sweeping phase is necessary for pause times, we can
> introduce a "CompleteSweep" phase which runs right before starting
> incremental marking.
>
> Change-Id: Iaff8c06d5691e584894f57941f181d0424051eec
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2567707
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#71555}

TBR=ulan@chromium.org,dinfuehr@chromium.org

Change-Id: I9adea60c21ff7cdfa7bbac3e6a4a240640fa5ea9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2569766Reviewed-by: 's avatarDominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71570}
parent 78e9a3a7
...@@ -198,12 +198,15 @@ void IncrementalMarking::Start(GarbageCollectionReason gc_reason) { ...@@ -198,12 +198,15 @@ void IncrementalMarking::Start(GarbageCollectionReason gc_reason) {
heap_->array_buffer_sweeper()->EnsureFinished(); heap_->array_buffer_sweeper()->EnsureFinished();
} }
collector_->EnsureSweepingCompleted(); if (!collector_->sweeping_in_progress()) {
DCHECK(!collector_->sweeping_in_progress()); StartMarking();
#ifdef DEBUG } else {
heap_->VerifyCountersAfterSweeping(); if (FLAG_trace_incremental_marking) {
#endif heap()->isolate()->PrintWithTimestamp(
StartMarking(); "[IncrementalMarking] Start sweeping.\n");
}
SetState(SWEEPING);
}
heap_->AddAllocationObserversToAllSpaces(&old_generation_observer_, heap_->AddAllocationObserversToAllSpaces(&old_generation_observer_,
&new_generation_observer_); &new_generation_observer_);
...@@ -788,6 +791,36 @@ StepResult IncrementalMarking::AdvanceWithDeadline( ...@@ -788,6 +791,36 @@ StepResult IncrementalMarking::AdvanceWithDeadline(
return Step(kStepSizeInMs, completion_action, step_origin); return Step(kStepSizeInMs, completion_action, step_origin);
} }
void IncrementalMarking::FinalizeSweeping() {
DCHECK(state_ == SWEEPING);
if (ContinueConcurrentSweeping()) {
if (FLAG_stress_incremental_marking) {
// To start concurrent marking a bit earlier, support concurrent sweepers
// from main thread by sweeping some pages.
SupportConcurrentSweeping();
}
return;
}
SafepointScope scope(heap());
collector_->EnsureSweepingCompleted();
DCHECK(!collector_->sweeping_in_progress());
#ifdef DEBUG
heap_->VerifyCountersAfterSweeping();
#endif
StartMarking();
}
bool IncrementalMarking::ContinueConcurrentSweeping() {
if (!collector_->sweeping_in_progress()) return false;
return FLAG_concurrent_sweeping &&
collector_->sweeper()->AreSweeperTasksRunning();
}
void IncrementalMarking::SupportConcurrentSweeping() {
collector_->sweeper()->SupportConcurrentSweeping();
}
size_t IncrementalMarking::StepSizeToKeepUpWithAllocations() { size_t IncrementalMarking::StepSizeToKeepUpWithAllocations() {
// Update bytes_allocated_ based on the allocation counter. // Update bytes_allocated_ based on the allocation counter.
size_t current_counter = heap_->OldGenerationAllocationCounter(); size_t current_counter = heap_->OldGenerationAllocationCounter();
...@@ -878,7 +911,7 @@ void IncrementalMarking::AdvanceOnAllocation() { ...@@ -878,7 +911,7 @@ void IncrementalMarking::AdvanceOnAllocation() {
// Code using an AlwaysAllocateScope assumes that the GC state does not // Code using an AlwaysAllocateScope assumes that the GC state does not
// change; that implies that no marking steps must be performed. // change; that implies that no marking steps must be performed.
if (heap_->gc_state() != Heap::NOT_IN_GC || !FLAG_incremental_marking || if (heap_->gc_state() != Heap::NOT_IN_GC || !FLAG_incremental_marking ||
state_ != MARKING || heap_->always_allocate()) { (state_ != SWEEPING && state_ != MARKING) || heap_->always_allocate()) {
return; return;
} }
HistogramTimerScope incremental_marking_scope( HistogramTimerScope incremental_marking_scope(
...@@ -894,6 +927,11 @@ StepResult IncrementalMarking::Step(double max_step_size_in_ms, ...@@ -894,6 +927,11 @@ StepResult IncrementalMarking::Step(double max_step_size_in_ms,
StepOrigin step_origin) { StepOrigin step_origin) {
double start = heap_->MonotonicallyIncreasingTimeInMs(); double start = heap_->MonotonicallyIncreasingTimeInMs();
if (state_ == SWEEPING) {
TRACE_GC(heap_->tracer(), GCTracer::Scope::MC_INCREMENTAL_SWEEPING);
FinalizeSweeping();
}
StepResult combined_result = StepResult::kMoreWorkRemaining; StepResult combined_result = StepResult::kMoreWorkRemaining;
size_t bytes_to_process = 0; size_t bytes_to_process = 0;
size_t v8_bytes_processed = 0; size_t v8_bytes_processed = 0;
......
...@@ -29,7 +29,7 @@ enum class StepResult { ...@@ -29,7 +29,7 @@ enum class StepResult {
class V8_EXPORT_PRIVATE IncrementalMarking final { class V8_EXPORT_PRIVATE IncrementalMarking final {
public: public:
enum State : uint8_t { STOPPED, MARKING, COMPLETE }; enum State : uint8_t { STOPPED, SWEEPING, MARKING, COMPLETE };
enum CompletionAction { GC_VIA_STACK_GUARD, NO_GC_VIA_STACK_GUARD }; enum CompletionAction { GC_VIA_STACK_GUARD, NO_GC_VIA_STACK_GUARD };
...@@ -116,6 +116,8 @@ class V8_EXPORT_PRIVATE IncrementalMarking final { ...@@ -116,6 +116,8 @@ class V8_EXPORT_PRIVATE IncrementalMarking final {
inline bool IsStopped() const { return state() == STOPPED; } inline bool IsStopped() const { return state() == STOPPED; }
inline bool IsSweeping() const { return state() == SWEEPING; }
inline bool IsMarking() const { return state() >= MARKING; } inline bool IsMarking() const { return state() >= MARKING; }
inline bool IsMarkingIncomplete() const { return state() == MARKING; } inline bool IsMarkingIncomplete() const { return state() == MARKING; }
......
...@@ -848,6 +848,10 @@ void MarkCompactCollector::Prepare() { ...@@ -848,6 +848,10 @@ void MarkCompactCollector::Prepare() {
heap_->array_buffer_sweeper()->EnsureFinished(); heap_->array_buffer_sweeper()->EnsureFinished();
} }
if (heap()->incremental_marking()->IsSweeping()) {
heap()->incremental_marking()->Stop();
}
if (!was_marked_incrementally_) { if (!was_marked_incrementally_) {
{ {
TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_EMBEDDER_PROLOGUE); TRACE_GC(heap()->tracer(), GCTracer::Scope::MC_MARK_EMBEDDER_PROLOGUE);
......
...@@ -173,6 +173,9 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) { ...@@ -173,6 +173,9 @@ void SimulateIncrementalMarking(i::Heap* heap, bool force_completion) {
SafepointScope scope(heap); SafepointScope scope(heap);
collector->EnsureSweepingCompleted(); collector->EnsureSweepingCompleted();
} }
if (marking->IsSweeping()) {
marking->FinalizeSweeping();
}
CHECK(marking->IsMarking() || marking->IsStopped() || marking->IsComplete()); CHECK(marking->IsMarking() || marking->IsStopped() || marking->IsComplete());
if (marking->IsStopped()) { if (marking->IsStopped()) {
heap->StartIncrementalMarking(i::Heap::kNoGCFlags, heap->StartIncrementalMarking(i::Heap::kNoGCFlags,
......
...@@ -1467,9 +1467,4 @@ ...@@ -1467,9 +1467,4 @@
'regress/regress-set-flags-stress-compact': [SKIP], 'regress/regress-set-flags-stress-compact': [SKIP],
}], # variant == stress_sampling }], # variant == stress_sampling
##############################################################################
['variant == stress_incremental_marking', {
'wasm/shared-memory-worker-stress': [PASS, SLOW],
}], # variant == stress_incremental_marking
] ]
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