[heap] Use non-nestable tasks for finalizing garbage collection

Pass on information about the embedder state using the fact that tasks
are run from top level

Bug: chromium:893944
Change-Id: I01441778770c5acc784540e496eec5c3fdb87796
Reviewed-on: https://chromium-review.googlesource.com/c/1273048
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56540}

src/heap/embedder-tracing.h

@@ -81,6 +81,24 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
   friend class EmbedderStackStateScope;
 };
 
+class V8_EXPORT_PRIVATE EmbedderStackStateScope final {
+ public:
+  EmbedderStackStateScope(LocalEmbedderHeapTracer* local_tracer,
+                          EmbedderHeapTracer::EmbedderStackState stack_state)
+      : local_tracer_(local_tracer),
+        old_stack_state_(local_tracer_->embedder_stack_state_) {
+    local_tracer_->embedder_stack_state_ = stack_state;
+  }
+
+  ~EmbedderStackStateScope() {
+    local_tracer_->embedder_stack_state_ = old_stack_state_;
+  }
+
+ private:
+  LocalEmbedderHeapTracer* const local_tracer_;
+  const EmbedderHeapTracer::EmbedderStackState old_stack_state_;
+};
+
 }  // namespace internal
 }  // namespace v8
 

src/heap/heap.cc

@@ -1606,6 +1606,11 @@ bool Heap::PerformGarbageCollection(
 
   {
     GCCallbacksScope scope(this);
+    // Temporary override any embedder stack state as callbacks may create their
+    // own state on the stack and recursively trigger GC.
+    EmbedderStackStateScope embedder_scope(
+        local_embedder_heap_tracer(),
+        EmbedderHeapTracer::EmbedderStackState::kUnknown);
     if (scope.CheckReenter()) {
       AllowHeapAllocation allow_allocation;
       TRACE_GC(tracer(), GCTracer::Scope::HEAP_EXTERNAL_PROLOGUE);

src/heap/incremental-marking-job.cc

@@ -5,6 +5,7 @@
 #include "src/heap/incremental-marking-job.h"
 
 #include "src/base/platform/time.h"
+#include "src/heap/embedder-tracing.h"
 #include "src/heap/heap-inl.h"
 #include "src/heap/heap.h"
 #include "src/heap/incremental-marking.h"
@@ -15,6 +16,29 @@
 namespace v8 {
 namespace internal {
 
+class IncrementalMarkingJob::Task : public CancelableTask {
+ public:
+  static void Step(Heap* heap,
+                   EmbedderHeapTracer::EmbedderStackState stack_state);
+
+  Task(Isolate* isolate, IncrementalMarkingJob* job,
+       EmbedderHeapTracer::EmbedderStackState stack_state)
+      : CancelableTask(isolate),
+        isolate_(isolate),
+        job_(job),
+        stack_state_(stack_state) {}
+
+  // CancelableTask overrides.
+  void RunInternal() override;
+
+  Isolate* isolate() const { return isolate_; }
+
+ private:
+  Isolate* const isolate_;
+  IncrementalMarkingJob* const job_;
+  const EmbedderHeapTracer::EmbedderStackState stack_state_;
+};
+
 void IncrementalMarkingJob::Start(Heap* heap) {
   DCHECK(!heap->incremental_marking()->IsStopped());
   ScheduleTask(heap);
@@ -26,19 +50,32 @@ void IncrementalMarkingJob::ScheduleTask(Heap* heap) {
     task_pending_ = true;
     auto taskrunner =
         V8::GetCurrentPlatform()->GetForegroundTaskRunner(isolate);
-    taskrunner->PostTask(base::make_unique<Task>(heap->isolate(), this));
+    if (taskrunner->NonNestableTasksEnabled()) {
+      taskrunner->PostNonNestableTask(base::make_unique<Task>(
+          heap->isolate(), this,
+          EmbedderHeapTracer::EmbedderStackState::kEmpty));
+    } else {
+      taskrunner->PostTask(base::make_unique<Task>(
+          heap->isolate(), this,
+          EmbedderHeapTracer::EmbedderStackState::kUnknown));
+    }
   }
 }
 
-void IncrementalMarkingJob::Task::Step(Heap* heap) {
+void IncrementalMarkingJob::Task::Step(
+    Heap* heap, EmbedderHeapTracer::EmbedderStackState stack_state) {
   const int kIncrementalMarkingDelayMs = 1;
   double deadline =
       heap->MonotonicallyIncreasingTimeInMs() + kIncrementalMarkingDelayMs;
   heap->incremental_marking()->AdvanceIncrementalMarking(
       deadline, i::IncrementalMarking::NO_GC_VIA_STACK_GUARD,
       i::StepOrigin::kTask);
-  heap->FinalizeIncrementalMarkingIfComplete(
-      GarbageCollectionReason::kFinalizeMarkingViaTask);
+  {
+    EmbedderStackStateScope scope(heap->local_embedder_heap_tracer(),
+                                  stack_state);
+    heap->FinalizeIncrementalMarkingIfComplete(
+        GarbageCollectionReason::kFinalizeMarkingViaTask);
+  }
 }
 
 void IncrementalMarkingJob::Task::RunInternal() {
@@ -61,7 +98,7 @@ void IncrementalMarkingJob::Task::RunInternal() {
   job_->task_pending_ = false;
 
   if (!incremental_marking->IsStopped()) {
-    Step(heap);
+    Step(heap, stack_state_);
     if (!incremental_marking->IsStopped()) {
       job_->ScheduleTask(heap);
     }

src/heap/incremental-marking-job.h

@@ -18,31 +18,18 @@ class Isolate;
 // step and posts another task until the marking is completed.
 class IncrementalMarkingJob {
  public:
-  class Task : public CancelableTask {
-   public:
-    explicit Task(Isolate* isolate, IncrementalMarkingJob* job)
-        : CancelableTask(isolate), isolate_(isolate), job_(job) {}
-    static void Step(Heap* heap);
-    // CancelableTask overrides.
-    void RunInternal() override;
+  IncrementalMarkingJob() = default;
 
-    Isolate* isolate() { return isolate_; }
-
-   private:
-    Isolate* isolate_;
-    IncrementalMarkingJob* job_;
-  };
-
-  IncrementalMarkingJob() : task_pending_(false) {}
-
-  bool TaskPending() { return task_pending_; }
+  bool TaskPending() const { return task_pending_; }
 
   void Start(Heap* heap);
 
   void ScheduleTask(Heap* heap);
 
  private:
-  bool task_pending_;
+  class Task;
+
+  bool task_pending_ = false;
 };
 }  // namespace internal
 }  // namespace v8

test/unittests/heap/embedder-tracing-unittest.cc

@@ -102,6 +102,36 @@ TEST(LocalEmbedderHeapTracer, EnterFinalPauseStackStateIsForwarded) {
   local_tracer.EnterFinalPause();
 }
 
+TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackState) {
+  StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
+  local_tracer.SetRemoteTracer(&remote_tracer);
+  // Default is unknown, see above.
+  {
+    EmbedderStackStateScope scope(&local_tracer, EmbedderHeapTracer::kEmpty);
+    EXPECT_CALL(remote_tracer, EnterFinalPause(EmbedderHeapTracer::kEmpty));
+    local_tracer.EnterFinalPause();
+  }
+}
+
+TEST(LocalEmbedderHeapTracer, TemporaryEmbedderStackStateRestores) {
+  StrictMock<MockEmbedderHeapTracer> remote_tracer;
+  LocalEmbedderHeapTracer local_tracer(nullptr);
+  local_tracer.SetRemoteTracer(&remote_tracer);
+  // Default is unknown, see above.
+  {
+    EmbedderStackStateScope scope(&local_tracer, EmbedderHeapTracer::kEmpty);
+    {
+      EmbedderStackStateScope scope(&local_tracer,
+                                    EmbedderHeapTracer::kUnknown);
+      EXPECT_CALL(remote_tracer, EnterFinalPause(EmbedderHeapTracer::kUnknown));
+      local_tracer.EnterFinalPause();
+    }
+    EXPECT_CALL(remote_tracer, EnterFinalPause(EmbedderHeapTracer::kEmpty));
+    local_tracer.EnterFinalPause();
+  }
+}
+
 TEST(LocalEmbedderHeapTracer, EnterFinalPauseStackStateResets) {
   StrictMock<MockEmbedderHeapTracer> remote_tracer;
   LocalEmbedderHeapTracer local_tracer(nullptr);