From 0215fb56f4c75b054116632039edbff0d7f40373 Mon Sep 17 00:00:00 2001 From: adamk Date: Tue, 4 Aug 2015 10:47:27 -0700 Subject: [PATCH] Revert of GC: Refactor public incremental marking interface in heap (patchset #6 id:100001 of https://codereview.chromium.org/1273483002/ ) Reason for revert: Fails on the MSAN builder: http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/3580/steps/Check/logs/RegExpInterruption Likely due to lack of initialization of IncrementalMarking::gc_callback_flags_. Original issue's description: > GC: Refactor incremental marking interface from heap > > BUG= > > Committed: https://crrev.com/c9fcaeb336919ce4b76fded8c8059457e9820250 > Cr-Commit-Position: refs/heads/master@{#30009} TBR=ulan@chromium.org,hpayer@chromium.org,mlippautz@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/1271823006 Cr-Commit-Position: refs/heads/master@{#30011} --- src/heap/heap.cc | 38 ++++++++++---------------------------- src/heap/heap.h | 19 ++++--------------- src/heap/incremental-marking.cc | 17 +++-------------- src/heap/incremental-marking.h | 23 +---------------------- 4 files changed, 18 insertions(+), 79 deletions(-) diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 61dd650..c17a7ac 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -769,8 +769,7 @@ void Heap::PreprocessStackTraces() { void Heap::HandleGCRequest() { if (incremental_marking()->request_type() == IncrementalMarking::COMPLETE_MARKING) { - CollectAllGarbage(Heap::kNoGCFlags, "GC interrupt", - incremental_marking()->CallbackFlags()); + CollectAllGarbage(Heap::kNoGCFlags, "GC interrupt"); return; } DCHECK(FLAG_overapproximate_weak_closure); @@ -980,7 +979,7 @@ bool Heap::CollectGarbage(GarbageCollector collector, const char* gc_reason, if (!mark_compact_collector()->abort_incremental_marking() && incremental_marking()->IsStopped() && incremental_marking()->ShouldActivateEvenWithoutIdleNotification()) { - incremental_marking()->Start(kNoGCFlags, kNoGCCallbackFlags, "GC epilogue"); + incremental_marking()->Start(kNoGCFlags); } return next_gc_likely_to_collect_more; @@ -1007,18 +1006,9 @@ int Heap::NotifyContextDisposed(bool dependant_context) { } -void Heap::StartIncrementalMarking(int gc_flags, - const GCCallbackFlags gc_callback_flags, - const char* reason) { - DCHECK(incremental_marking()->IsStopped()); - incremental_marking()->Start(gc_flags, gc_callback_flags, reason); -} - - void Heap::StartIdleIncrementalMarking() { gc_idle_time_handler_.ResetNoProgressCounter(); - StartIncrementalMarking(kReduceMemoryFootprintMask, kNoGCCallbackFlags, - "idle"); + incremental_marking()->Start(kReduceMemoryFootprintMask); } @@ -4801,21 +4791,13 @@ GCIdleTimeHandler::HeapState Heap::ComputeHeapState() { double Heap::AdvanceIncrementalMarking( intptr_t step_size_in_bytes, double deadline_in_ms, - IncrementalMarking::StepActions step_actions) { + IncrementalMarking::ForceCompletionAction completion) { DCHECK(!incremental_marking()->IsStopped()); - - if (step_size_in_bytes == 0) { - step_size_in_bytes = GCIdleTimeHandler::EstimateMarkingStepSize( - static_cast(GCIdleTimeHandler::kIncrementalMarkingStepTimeInMs), - static_cast( - tracer()->FinalIncrementalMarkCompactSpeedInBytesPerMillisecond())); - } - double remaining_time_in_ms = 0.0; do { - incremental_marking()->Step( - step_size_in_bytes, step_actions.completion_action, - step_actions.force_marking, step_actions.force_completion); + incremental_marking()->Step(step_size_in_bytes, + IncrementalMarking::NO_GC_VIA_STACK_GUARD, + IncrementalMarking::FORCE_MARKING, completion); remaining_time_in_ms = deadline_in_ms - MonotonicallyIncreasingTimeInMs(); } while (remaining_time_in_ms >= 2.0 * GCIdleTimeHandler::kIncrementalMarkingStepTimeInMs && @@ -4834,9 +4816,9 @@ bool Heap::PerformIdleTimeAction(GCIdleTimeAction action, result = true; break; case DO_INCREMENTAL_MARKING: { - const double remaining_idle_time_in_ms = - AdvanceIncrementalMarking(action.parameter, deadline_in_ms, - IncrementalMarking::NoForcedStepActions()); + const double remaining_idle_time_in_ms = AdvanceIncrementalMarking( + action.parameter, deadline_in_ms, + IncrementalMarking::DO_NOT_FORCE_COMPLETION); if (remaining_idle_time_in_ms > 0.0) { action.additional_work = TryFinalizeIdleIncrementalMarking( remaining_idle_time_in_ms, heap_state.size_of_objects, diff --git a/src/heap/heap.h b/src/heap/heap.h index 2e5be2b..48e3de8 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -852,21 +852,6 @@ class Heap { // incremental steps. void StartIdleIncrementalMarking(); - // Starts incremental marking assuming incremental marking is currently - // stopped. - void StartIncrementalMarking(int gc_flags, - const GCCallbackFlags gc_callback_flags, - const char* reason = nullptr); - - // Performs incremental marking steps of step_size_in_bytes as long as - // deadline_ins_ms is not reached. step_size_in_bytes can be 0 to compute - // an estimate increment. Returns the remaining time that cannot be used - // for incremental marking anymore because a single step would exceed the - // deadline. - double AdvanceIncrementalMarking( - intptr_t step_size_in_bytes, double deadline_in_ms, - IncrementalMarking::StepActions step_actions); - inline void increment_scan_on_scavenge_pages() { scan_on_scavenge_pages_++; if (FLAG_gc_verbose) { @@ -2246,6 +2231,10 @@ class Heap { GCIdleTimeHandler::HeapState ComputeHeapState(); + double AdvanceIncrementalMarking( + intptr_t step_size_in_bytes, double deadline_in_ms, + IncrementalMarking::ForceCompletionAction completion); + bool PerformIdleTimeAction(GCIdleTimeAction action, GCIdleTimeHandler::HeapState heap_state, double deadline_in_ms); diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc index 343fd80..6b44771 100644 --- a/src/heap/incremental-marking.cc +++ b/src/heap/incremental-marking.cc @@ -16,13 +16,6 @@ namespace v8 { namespace internal { -IncrementalMarking::StepActions IncrementalMarking::NoForcedStepActions() { - return StepActions(IncrementalMarking::NO_GC_VIA_STACK_GUARD, - IncrementalMarking::DO_NOT_FORCE_MARKING, - IncrementalMarking::DO_NOT_FORCE_COMPLETION); -} - - IncrementalMarking::IncrementalMarking(Heap* heap) : heap_(heap), state_(STOPPED), @@ -465,12 +458,9 @@ static void PatchIncrementalMarkingRecordWriteStubs( } -void IncrementalMarking::Start(int mark_compact_flags, - const GCCallbackFlags gc_callback_flags, - const char* reason) { +void IncrementalMarking::Start(int mark_compact_flags) { if (FLAG_trace_incremental_marking) { - PrintF("[IncrementalMarking] Start (%s)\n", - (reason == nullptr) ? "unknown reason" : reason); + PrintF("[IncrementalMarking] Start\n"); } DCHECK(FLAG_incremental_marking); DCHECK(FLAG_incremental_marking_steps); @@ -480,7 +470,6 @@ void IncrementalMarking::Start(int mark_compact_flags, ResetStepCounters(); - gc_callback_flags_ = gc_callback_flags; was_activated_ = true; if (!heap_->mark_compact_collector()->sweeping_in_progress()) { @@ -823,7 +812,7 @@ void IncrementalMarking::Epilogue() { void IncrementalMarking::OldSpaceStep(intptr_t allocated) { if (IsStopped() && ShouldActivateEvenWithoutIdleNotification()) { - Start(Heap::kNoGCFlags, kNoGCCallbackFlags, "old space step"); + Start(Heap::kNoGCFlags); } else { Step(allocated * kFastMarking / kInitialMarkingSpeed, GC_VIA_STACK_GUARD); } diff --git a/src/heap/incremental-marking.h b/src/heap/incremental-marking.h index 2c63cfc..8fe3411 100644 --- a/src/heap/incremental-marking.h +++ b/src/heap/incremental-marking.h @@ -26,21 +26,6 @@ class IncrementalMarking { enum GCRequestType { COMPLETE_MARKING, OVERAPPROXIMATION }; - struct StepActions { - StepActions(CompletionAction complete_action_, - ForceMarkingAction force_marking_, - ForceCompletionAction force_completion_) - : completion_action(complete_action_), - force_marking(force_marking_), - force_completion(force_completion_) {} - - CompletionAction completion_action; - ForceMarkingAction force_marking; - ForceCompletionAction force_completion; - }; - - static StepActions NoForcedStepActions(); - explicit IncrementalMarking(Heap* heap); static void Initialize(); @@ -82,9 +67,7 @@ class IncrementalMarking { bool WasActivated(); - void Start(int mark_compact_flags, - const GCCallbackFlags gc_callback_flags = kNoGCCallbackFlags, - const char* reason = nullptr); + void Start(int mark_compact_flags); void Stop(); @@ -202,8 +185,6 @@ class IncrementalMarking { Heap* heap() const { return heap_; } - GCCallbackFlags CallbackFlags() const { return gc_callback_flags_; } - private: int64_t SpaceLeftInOldSpace(); @@ -262,8 +243,6 @@ class IncrementalMarking { GCRequestType request_type_; - GCCallbackFlags gc_callback_flags_; - DISALLOW_IMPLICIT_CONSTRUCTORS(IncrementalMarking); }; } -- 2.7.4