Revert of Reland "[heap] Ensure progress when incrementally marking wrappers"...

Revert of Reland "[heap] Ensure progress when incrementally marking wrappers" (patchset #8 id:140001 of https://codereview.chromium.org/2591383004/ )

Reason for revert:
Breaks webkit-unit-tests. Investigating..

Original issue's description:
> Reland "[heap] Ensure progress when incrementally marking wrappers"
>
> 1) Alternate between processing v8 and wrappers
> 2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and wrappers
> 3) After that, finalize once v8 marking deque is empty again
>
> BUG=
>
> Review-Url: https://codereview.chromium.org/2591383004
> Cr-Commit-Position: refs/heads/master@{#41932}
> Committed: https://chromium.googlesource.com/v8/v8/+/61a55548c50e01d84ed4aefa396324cbb4039b51

TBR=hpayer@chromium.org,ulan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

Review-Url: https://codereview.chromium.org/2592393003
Cr-Commit-Position: refs/heads/master@{#41936}

src/heap/embedder-tracing.cc

@@ -13,7 +13,6 @@ void LocalEmbedderHeapTracer::TracePrologue() {
   if (!InUse()) return;
 
   CHECK(cached_wrappers_to_trace_.empty());
-  num_v8_marking_deque_was_empty_ = 0;
   remote_tracer_->TracePrologue();
 }
 

src/heap/embedder-tracing.h

@@ -6,7 +6,6 @@
 #define V8_HEAP_EMBEDDER_TRACING_H_
 
 #include "include/v8.h"
-#include "src/flags.h"
 #include "src/globals.h"
 
 namespace v8 {
@@ -18,8 +17,7 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
  public:
   typedef std::pair<void*, void*> WrapperInfo;
 
-  LocalEmbedderHeapTracer()
-      : remote_tracer_(nullptr), num_v8_marking_deque_was_empty_(0) {}
+  LocalEmbedderHeapTracer() : remote_tracer_(nullptr) {}
 
   void SetRemoteTracer(EmbedderHeapTracer* tracer) { remote_tracer_ = tracer; }
   bool InUse() { return remote_tracer_ != nullptr; }
@@ -45,20 +43,11 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
   // are too many of them.
   bool RequiresImmediateWrapperProcessing();
 
-  void NotifyV8MarkingDequeWasEmpty() { num_v8_marking_deque_was_empty_++; }
-  bool ShouldFinalizeIncrementalMarking() {
-    static const size_t kMaxIncrementalFixpointRounds = 3;
-    return !FLAG_incremental_marking_wrappers || !InUse() ||
-           NumberOfWrappersToTrace() == 0 ||
-           num_v8_marking_deque_was_empty_ > kMaxIncrementalFixpointRounds;
-  }
-
  private:
   typedef std::vector<WrapperInfo> WrapperCache;
 
   EmbedderHeapTracer* remote_tracer_;
   WrapperCache cached_wrappers_to_trace_;
-  size_t num_v8_marking_deque_was_empty_;
 };
 
 }  // namespace internal

src/heap/heap.cc

@@ -4176,18 +4176,19 @@ void Heap::ReduceNewSpaceSize() {
   }
 }
 
+bool Heap::MarkingDequesAreEmpty() {
+  return mark_compact_collector()->marking_deque()->IsEmpty() &&
+         local_embedder_heap_tracer()->NumberOfWrappersToTrace() == 0;
+}
+
 void Heap::FinalizeIncrementalMarkingIfComplete(
     GarbageCollectionReason gc_reason) {
   if (incremental_marking()->IsMarking() &&
       (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
        (!incremental_marking()->finalize_marking_completed() &&
-        mark_compact_collector()->marking_deque()->IsEmpty() &&
-        local_embedder_heap_tracer()->ShouldFinalizeIncrementalMarking()))) {
+        MarkingDequesAreEmpty()))) {
     FinalizeIncrementalMarking(gc_reason);
-  } else if (incremental_marking()->IsComplete() ||
-             (mark_compact_collector()->marking_deque()->IsEmpty() &&
-              local_embedder_heap_tracer()
-                  ->ShouldFinalizeIncrementalMarking())) {
+  } else if (incremental_marking()->IsComplete() || MarkingDequesAreEmpty()) {
     CollectAllGarbage(current_gc_flags_, gc_reason);
   }
 }
@@ -4199,16 +4200,13 @@ bool Heap::TryFinalizeIdleIncrementalMarking(
       tracer()->FinalIncrementalMarkCompactSpeedInBytesPerMillisecond();
   if (incremental_marking()->IsReadyToOverApproximateWeakClosure() ||
       (!incremental_marking()->finalize_marking_completed() &&
-       mark_compact_collector()->marking_deque()->IsEmpty() &&
-       local_embedder_heap_tracer()->ShouldFinalizeIncrementalMarking() &&
+       MarkingDequesAreEmpty() &&
        gc_idle_time_handler_->ShouldDoOverApproximateWeakClosure(
            idle_time_in_ms))) {
     FinalizeIncrementalMarking(gc_reason);
     return true;
   } else if (incremental_marking()->IsComplete() ||
-             (mark_compact_collector()->marking_deque()->IsEmpty() &&
-              local_embedder_heap_tracer()
-                  ->ShouldFinalizeIncrementalMarking() &&
+             (MarkingDequesAreEmpty() &&
               gc_idle_time_handler_->ShouldDoFinalIncrementalMarkCompact(
                   idle_time_in_ms, size_of_objects,
                   final_incremental_mark_compact_speed_in_bytes_per_ms))) {

src/heap/heap.h

@@ -1627,6 +1627,10 @@ class Heap {
     return current_gc_flags_ & kFinalizeIncrementalMarkingMask;
   }
 
+  // Checks whether both, the internal marking deque, and the embedder provided
+  // one are empty. Avoid in fast path as it potentially calls through the API.
+  bool MarkingDequesAreEmpty();
+
   void PreprocessStackTraces();
 
   // Checks whether a global GC is necessary

src/heap/incremental-marking.cc

@@ -1038,28 +1038,8 @@ double IncrementalMarking::AdvanceIncrementalMarking(
       kStepSizeInMs,
       heap()->tracer()->IncrementalMarkingSpeedInBytesPerMillisecond());
 
-  const bool incremental_wrapper_tracing =
-      state_ == MARKING && FLAG_incremental_marking_wrappers &&
-      heap_->local_embedder_heap_tracer()->InUse();
-  bool trace_wrappers_toggle = true;
   do {
-    if (incremental_wrapper_tracing && trace_wrappers_toggle) {
-      TRACE_GC(heap()->tracer(),
-               GCTracer::Scope::MC_INCREMENTAL_WRAPPER_TRACING);
-      const double wrapper_deadline =
-          heap_->MonotonicallyIncreasingTimeInMs() + kStepSizeInMs;
-      if (!heap_->local_embedder_heap_tracer()
-               ->ShouldFinalizeIncrementalMarking()) {
-        heap_->local_embedder_heap_tracer()->Trace(
-            wrapper_deadline, EmbedderHeapTracer::AdvanceTracingActions(
-                                  EmbedderHeapTracer::ForceCompletionAction::
-                                      DO_NOT_FORCE_COMPLETION));
-      }
-    } else {
-      Step(step_size_in_bytes, completion_action, force_completion,
-           step_origin);
-    }
-    trace_wrappers_toggle = !trace_wrappers_toggle;
+    Step(step_size_in_bytes, completion_action, force_completion, step_origin);
     remaining_time_in_ms =
         deadline_in_ms - heap()->MonotonicallyIncreasingTimeInMs();
   } while (remaining_time_in_ms >= kStepSizeInMs && !IsComplete() &&
@@ -1158,26 +1138,42 @@ size_t IncrementalMarking::Step(size_t bytes_to_process,
 
   size_t bytes_processed = 0;
   if (state_ == MARKING) {
-    bytes_processed = ProcessMarkingDeque(bytes_to_process);
-    if (step_origin == StepOrigin::kTask) {
-      bytes_marked_ahead_of_schedule_ += bytes_processed;
+    const bool incremental_wrapper_tracing =
+        FLAG_incremental_marking_wrappers &&
+        heap_->local_embedder_heap_tracer()->InUse();
+    const bool process_wrappers =
+        incremental_wrapper_tracing &&
+        (heap_->local_embedder_heap_tracer()
+             ->RequiresImmediateWrapperProcessing() ||
+         heap_->mark_compact_collector()->marking_deque()->IsEmpty());
+    bool wrapper_work_left = incremental_wrapper_tracing;
+    if (!process_wrappers) {
+      bytes_processed = ProcessMarkingDeque(bytes_to_process);
+      if (step_origin == StepOrigin::kTask) {
+        bytes_marked_ahead_of_schedule_ += bytes_processed;
+      }
+    } else {
+      const double wrapper_deadline =
+          heap_->MonotonicallyIncreasingTimeInMs() + kStepSizeInMs;
+      TRACE_GC(heap()->tracer(),
+               GCTracer::Scope::MC_INCREMENTAL_WRAPPER_TRACING);
+      wrapper_work_left = heap_->local_embedder_heap_tracer()->Trace(
+          wrapper_deadline, EmbedderHeapTracer::AdvanceTracingActions(
+                                EmbedderHeapTracer::ForceCompletionAction::
+                                    DO_NOT_FORCE_COMPLETION));
     }
 
-    if (heap_->mark_compact_collector()->marking_deque()->IsEmpty()) {
-      if (heap_->local_embedder_heap_tracer()
-              ->ShouldFinalizeIncrementalMarking()) {
-        if (completion == FORCE_COMPLETION ||
-            IsIdleMarkingDelayCounterLimitReached()) {
-          if (!finalize_marking_completed_) {
-            FinalizeMarking(action);
-          } else {
-            MarkingComplete(action);
-          }
+    if (heap_->mark_compact_collector()->marking_deque()->IsEmpty() &&
+        !wrapper_work_left) {
+      if (completion == FORCE_COMPLETION ||
+          IsIdleMarkingDelayCounterLimitReached()) {
+        if (!finalize_marking_completed_) {
+          FinalizeMarking(action);
         } else {
-          IncrementIdleMarkingDelayCounter();
+          MarkingComplete(action);
         }
       } else {
-        heap_->local_embedder_heap_tracer()->NotifyV8MarkingDequeWasEmpty();
+        IncrementIdleMarkingDelayCounter();
       }
     }
   }

src/heap/mark-compact.cc

@@ -2157,8 +2157,7 @@ void MarkCompactCollector::ProcessEphemeralMarking(
     work_to_do = !marking_deque()->IsEmpty();
     ProcessMarkingDeque<MarkCompactMode::FULL>();
   }
-  CHECK(marking_deque()->IsEmpty());
-  CHECK_EQ(0, heap()->local_embedder_heap_tracer()->NumberOfWrappersToTrace());
+  CHECK(heap_->MarkingDequesAreEmpty());
 }
 
 void MarkCompactCollector::ProcessTopOptimizedFrame(ObjectVisitor* visitor) {