[heap] Do not emit background GC trace events on the main thread

When the main thread contributes to an item parallel job and runs
an item parallel task, it currently emits a background GC trace event.

That is confusing and may lead to incorrect accounting of main thread
GC time. This patch fixes it by introducing a 'Runner' parameter
to ItemParalllelJob::Task::RunInParallel and emitting a foreground
GC event if the runner is the main thread.

Bug: v8:9508
Change-Id: I755751bfe9eef427666d5f16fb50aa6093059e80
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1706485Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62798}

src/heap/item-parallel-job.cc

@@ -26,8 +26,12 @@ void ItemParallelJob::Task::SetupInternal(base::Semaphore* on_finish,
   }
 }
 
+void ItemParallelJob::Task::WillRunOnForeground() {
+  runner_ = Runner::kForeground;
+}
+
 void ItemParallelJob::Task::RunInternal() {
-  RunInParallel();
+  RunInParallel(runner_);
   on_finish_->Signal();
 }
 
@@ -95,6 +99,7 @@ void ItemParallelJob::Run() {
 
   // Contribute on main thread.
   DCHECK(main_task);
+  main_task->WillRunOnForeground();
   main_task->Run();
 
   // Wait for background tasks.

src/heap/item-parallel-job.h

@@ -65,10 +65,11 @@ class V8_EXPORT_PRIVATE ItemParallelJob {
 
   class V8_EXPORT_PRIVATE Task : public CancelableTask {
    public:
+    enum class Runner { kForeground, kBackground };
     explicit Task(Isolate* isolate);
     ~Task() override = default;
 
-    virtual void RunInParallel() = 0;
+    virtual void RunInParallel(Runner runner) = 0;
 
    protected:
     // Retrieves a new item that needs to be processed. Returns |nullptr| if
@@ -99,13 +100,14 @@ class V8_EXPORT_PRIVATE ItemParallelJob {
     // processing, e.g. scavenging).
     void SetupInternal(base::Semaphore* on_finish, std::vector<Item*>* items,
                        size_t start_index);
-
+    void WillRunOnForeground();
     // We don't allow overriding this method any further.
     void RunInternal() final;
 
     std::vector<Item*>* items_ = nullptr;
     size_t cur_index_ = 0;
     size_t items_considered_ = 0;
+    Runner runner_ = Runner::kBackground;
     base::Semaphore* on_finish_ = nullptr;
 
     DISALLOW_COPY_AND_ASSIGN(Task);

src/heap/mark-compact.cc

@@ -2750,6 +2750,7 @@ class Evacuator : public Malloced {
   inline void Finalize();
 
   virtual GCTracer::BackgroundScope::ScopeId GetBackgroundTracingScope() = 0;
+  virtual GCTracer::Scope::ScopeId GetTracingScope() = 0;
 
  protected:
   static const int kInitialLocalPretenuringFeedbackCapacity = 256;
@@ -2838,6 +2839,10 @@ class FullEvacuator : public Evacuator {
     return GCTracer::BackgroundScope::MC_BACKGROUND_EVACUATE_COPY;
   }
 
+  GCTracer::Scope::ScopeId GetTracingScope() override {
+    return GCTracer::Scope::MC_EVACUATE_COPY_PARALLEL;
+  }
+
   inline void Finalize() {
     Evacuator::Finalize();
 
@@ -2928,16 +2933,24 @@ class PageEvacuationTask : public ItemParallelJob::Task {
         evacuator_(evacuator),
         tracer_(isolate->heap()->tracer()) {}
 
-  void RunInParallel() override {
-    TRACE_BACKGROUND_GC(tracer_, evacuator_->GetBackgroundTracingScope());
+  void RunInParallel(Runner runner) override {
+    if (runner == Runner::kForeground) {
+      TRACE_GC(tracer_, evacuator_->GetTracingScope());
+      ProcessItems();
+    } else {
+      TRACE_BACKGROUND_GC(tracer_, evacuator_->GetBackgroundTracingScope());
+      ProcessItems();
+    }
+  }
+
+ private:
+  void ProcessItems() {
     EvacuationItem* item = nullptr;
     while ((item = GetItem<EvacuationItem>()) != nullptr) {
       evacuator_->EvacuatePage(item->chunk());
       item->MarkFinished();
     }
   }
-
- private:
   Evacuator* evacuator_;
   GCTracer* tracer_;
 };
@@ -3237,24 +3250,35 @@ class UpdatingItem : public ItemParallelJob::Item {
 
 class PointersUpdatingTask : public ItemParallelJob::Task {
  public:
-  explicit PointersUpdatingTask(Isolate* isolate,
-                                GCTracer::BackgroundScope::ScopeId scope)
+  explicit PointersUpdatingTask(
+      Isolate* isolate, GCTracer::Scope::ScopeId scope,
+      GCTracer::BackgroundScope::ScopeId background_scope)
       : ItemParallelJob::Task(isolate),
         tracer_(isolate->heap()->tracer()),
-        scope_(scope) {}
+        scope_(scope),
+        background_scope_(background_scope) {}
+
+  void RunInParallel(Runner runner) override {
+    if (runner == Runner::kForeground) {
+      TRACE_GC(tracer_, scope_);
+      UpdatePointers();
+    } else {
+      TRACE_BACKGROUND_GC(tracer_, background_scope_);
+      UpdatePointers();
+    }
+  }
 
-  void RunInParallel() override {
-    TRACE_BACKGROUND_GC(tracer_, scope_);
+ private:
+  void UpdatePointers() {
     UpdatingItem* item = nullptr;
     while ((item = GetItem<UpdatingItem>()) != nullptr) {
       item->Process();
       item->MarkFinished();
     }
   }
-
- private:
   GCTracer* tracer_;
-  GCTracer::BackgroundScope::ScopeId scope_;
+  GCTracer::Scope::ScopeId scope_;
+  GCTracer::BackgroundScope::ScopeId background_scope_;
 };
 
 template <typename MarkingState>
@@ -3670,7 +3694,7 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
             remembered_set_tasks + num_ephemeron_table_updating_tasks);
     for (int i = 0; i < num_tasks; i++) {
       updating_job.AddTask(new PointersUpdatingTask(
-          isolate(),
+          isolate(), GCTracer::Scope::MC_EVACUATE_UPDATE_POINTERS_PARALLEL,
           GCTracer::BackgroundScope::MC_BACKGROUND_EVACUATE_UPDATE_POINTERS));
     }
     updating_job.AddItem(new EphemeronTableUpdatingItem(heap()));
@@ -3703,7 +3727,7 @@ void MarkCompactCollector::UpdatePointersAfterEvacuation() {
     if (num_tasks > 0) {
       for (int i = 0; i < num_tasks; i++) {
         updating_job.AddTask(new PointersUpdatingTask(
-            isolate(),
+            isolate(), GCTracer::Scope::MC_EVACUATE_UPDATE_POINTERS_PARALLEL,
             GCTracer::BackgroundScope::MC_BACKGROUND_EVACUATE_UPDATE_POINTERS));
       }
       updating_job.Run();
@@ -4213,8 +4237,9 @@ void MinorMarkCompactCollector::UpdatePointersAfterEvacuation() {
   const int num_tasks = Max(to_space_tasks, remembered_set_tasks);
   for (int i = 0; i < num_tasks; i++) {
     updating_job.AddTask(new PointersUpdatingTask(
-        isolate(), GCTracer::BackgroundScope::
-                       MINOR_MC_BACKGROUND_EVACUATE_UPDATE_POINTERS));
+        isolate(), GCTracer::Scope::MINOR_MC_EVACUATE_UPDATE_POINTERS_PARALLEL,
+        GCTracer::BackgroundScope::
+            MINOR_MC_BACKGROUND_EVACUATE_UPDATE_POINTERS));
   }
 
   {
@@ -4517,9 +4542,30 @@ class YoungGenerationMarkingTask : public ItemParallelJob::Task {
                               Page::kPageSize);
   }
 
-  void RunInParallel() override {
-    TRACE_BACKGROUND_GC(collector_->heap()->tracer(),
-                        GCTracer::BackgroundScope::MINOR_MC_BACKGROUND_MARKING);
+  void RunInParallel(Runner runner) override {
+    if (runner == Runner::kForeground) {
+      TRACE_GC(collector_->heap()->tracer(),
+               GCTracer::Scope::MINOR_MC_MARK_PARALLEL);
+      ProcessItems();
+    } else {
+      TRACE_BACKGROUND_GC(
+          collector_->heap()->tracer(),
+          GCTracer::BackgroundScope::MINOR_MC_BACKGROUND_MARKING);
+      ProcessItems();
+    }
+  }
+
+  void MarkObject(Object object) {
+    if (!Heap::InYoungGeneration(object)) return;
+    HeapObject heap_object = HeapObject::cast(object);
+    if (marking_state_->WhiteToGrey(heap_object)) {
+      const int size = visitor_.Visit(heap_object);
+      IncrementLiveBytes(heap_object, size);
+    }
+  }
+
+ private:
+  void ProcessItems() {
     double marking_time = 0.0;
     {
       TimedScope scope(&marking_time);
@@ -4538,17 +4584,6 @@ class YoungGenerationMarkingTask : public ItemParallelJob::Task {
                    static_cast<void*>(this), marking_time);
     }
   }
-
-  void MarkObject(Object object) {
-    if (!Heap::InYoungGeneration(object)) return;
-    HeapObject heap_object = HeapObject::cast(object);
-    if (marking_state_->WhiteToGrey(heap_object)) {
-      const int size = visitor_.Visit(heap_object);
-      IncrementLiveBytes(heap_object, size);
-    }
-  }
-
- private:
   void EmptyLocalMarkingWorklist() {
     HeapObject object;
     while (marking_worklist_.Pop(&object)) {
@@ -4780,6 +4815,10 @@ class YoungGenerationEvacuator : public Evacuator {
     return GCTracer::BackgroundScope::MINOR_MC_BACKGROUND_EVACUATE_COPY;
   }
 
+  GCTracer::Scope::ScopeId GetTracingScope() override {
+    return GCTracer::Scope::MINOR_MC_EVACUATE_COPY_PARALLEL;
+  }
+
  protected:
   void RawEvacuatePage(MemoryChunk* chunk, intptr_t* live_bytes) override;
 

src/heap/scavenger.cc

@@ -41,10 +41,20 @@ class ScavengingTask final : public ItemParallelJob::Task {
         scavenger_(scavenger),
         barrier_(barrier) {}
 
-  void RunInParallel() final {
-    TRACE_BACKGROUND_GC(
-        heap_->tracer(),
-        GCTracer::BackgroundScope::SCAVENGER_BACKGROUND_SCAVENGE_PARALLEL);
+  void RunInParallel(Runner runner) final {
+    if (runner == Runner::kForeground) {
+      TRACE_GC(heap_->tracer(), GCTracer::Scope::SCAVENGER_SCAVENGE_PARALLEL);
+      ProcessItems();
+    } else {
+      TRACE_BACKGROUND_GC(
+          heap_->tracer(),
+          GCTracer::BackgroundScope::SCAVENGER_BACKGROUND_SCAVENGE_PARALLEL);
+      ProcessItems();
+    }
+  }
+
+ private:
+  void ProcessItems() {
     double scavenging_time = 0.0;
     {
       barrier_->Start();
@@ -66,8 +76,6 @@ class ScavengingTask final : public ItemParallelJob::Task {
                    scavenger_->bytes_copied(), scavenger_->bytes_promoted());
     }
   }
-
- private:
   Heap* const heap_;
   Scavenger* const scavenger_;
   OneshotBarrier* const barrier_;

src/init/heap-symbols.h

@@ -416,10 +416,12 @@
   F(MC_EVACUATE_CANDIDATES)                          \
   F(MC_EVACUATE_CLEAN_UP)                            \
   F(MC_EVACUATE_COPY)                                \
+  F(MC_EVACUATE_COPY_PARALLEL)                       \
   F(MC_EVACUATE_EPILOGUE)                            \
   F(MC_EVACUATE_PROLOGUE)                            \
   F(MC_EVACUATE_REBALANCE)                           \
   F(MC_EVACUATE_UPDATE_POINTERS)                     \
+  F(MC_EVACUATE_UPDATE_POINTERS_PARALLEL)            \
   F(MC_EVACUATE_UPDATE_POINTERS_SLOTS_MAIN)          \
   F(MC_EVACUATE_UPDATE_POINTERS_SLOTS_MAP_SPACE)     \
   F(MC_EVACUATE_UPDATE_POINTERS_TO_NEW_ROOTS)        \
@@ -447,15 +449,18 @@
   F(MINOR_MC_EVACUATE)                               \
   F(MINOR_MC_EVACUATE_CLEAN_UP)                      \
   F(MINOR_MC_EVACUATE_COPY)                          \
+  F(MINOR_MC_EVACUATE_COPY_PARALLEL)                 \
   F(MINOR_MC_EVACUATE_EPILOGUE)                      \
   F(MINOR_MC_EVACUATE_PROLOGUE)                      \
   F(MINOR_MC_EVACUATE_REBALANCE)                     \
   F(MINOR_MC_EVACUATE_UPDATE_POINTERS)               \
+  F(MINOR_MC_EVACUATE_UPDATE_POINTERS_PARALLEL)      \
   F(MINOR_MC_EVACUATE_UPDATE_POINTERS_SLOTS)         \
   F(MINOR_MC_EVACUATE_UPDATE_POINTERS_TO_NEW_ROOTS)  \
   F(MINOR_MC_EVACUATE_UPDATE_POINTERS_WEAK)          \
   F(MINOR_MC_MARK)                                   \
   F(MINOR_MC_MARK_GLOBAL_HANDLES)                    \
+  F(MINOR_MC_MARK_PARALLEL)                          \
   F(MINOR_MC_MARK_SEED)                              \
   F(MINOR_MC_MARK_ROOTS)                             \
   F(MINOR_MC_MARK_WEAK)                              \

test/unittests/heap/item-parallel-job-unittest.cc

@@ -28,7 +28,7 @@ class SimpleTask : public ItemParallelJob::Task {
   SimpleTask(Isolate* isolate, bool* did_run)
       : ItemParallelJob::Task(isolate), did_run_(did_run) {}
 
-  void RunInParallel() override {
+  void RunInParallel(Runner runner) override {
     ItemParallelJob::Item* item = nullptr;
     while ((item = GetItem<ItemParallelJob::Item>()) != nullptr) {
       item->MarkFinished();
@@ -58,7 +58,7 @@ class EagerTask : public ItemParallelJob::Task {
  public:
   explicit EagerTask(Isolate* isolate) : ItemParallelJob::Task(isolate) {}
 
-  void RunInParallel() override {
+  void RunInParallel(Runner runner) override {
     SimpleItem* item = nullptr;
     while ((item = GetItem<SimpleItem>()) != nullptr) {
       item->Process();
@@ -120,7 +120,7 @@ class TaskProcessingOneItem : public ItemParallelJob::Task {
         wait_when_done_(wait_when_done),
         did_process_an_item_(did_process_an_item) {}
 
-  void RunInParallel() override {
+  void RunInParallel(Runner runner) override {
     SimpleItem* item = GetItem<SimpleItem>();
 
     if (did_process_an_item_) {
@@ -164,7 +164,7 @@ class TaskForDifferentItems : public ItemParallelJob::Task {
         processed_b_(processed_b) {}
   ~TaskForDifferentItems() override = default;
 
-  void RunInParallel() override {
+  void RunInParallel(Runner runner) override {
     BaseItem* item = nullptr;
     while ((item = GetItem<BaseItem>()) != nullptr) {
       item->ProcessItem(this);