Revert of GC: Refactor public incremental marking interface in heap (patchset #6...

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}

diff --git a/src/heap/heap.cc b/src/heap/heap.cc
index 61dd650..c17a7ac 100644 (file)
--- 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<size_t>(GCIdleTimeHandler::kIncrementalMarkingStepTimeInMs),
-        static_cast<size_t>(
-            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 (file)
--- 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 (file)
--- 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 (file)
--- 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);
 };
 }