Extract the trivial cleanup bits from https://chromium-review.googlesource.com/c/v8/v8/+/924073/10

This is an attempt to isolate what's causing the hard-to-diagnose bots only
failures with that CL.

Bug: chromium:812178
Change-Id: I50ffe8953bebbbc6b5a5e2f689718662a537acb4
Reviewed-on: https://chromium-review.googlesource.com/924864Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51348}

src/heap/concurrent-marking.cc

@@ -424,18 +424,11 @@ ConcurrentMarking::ConcurrentMarking(Heap* heap, MarkingWorklist* shared,
       shared_(shared),
       bailout_(bailout),
       on_hold_(on_hold),
-      weak_objects_(weak_objects),
-      total_marked_bytes_(0),
-      pending_task_count_(0),
-      task_count_(0) {
+      weak_objects_(weak_objects) {
 // The runtime flag should be set only if the compile time flag was set.
 #ifndef V8_CONCURRENT_MARKING
   CHECK(!FLAG_concurrent_marking);
 #endif
-  for (int i = 0; i <= kMaxTasks; i++) {
-    is_pending_[i] = false;
-    task_state_[i].marked_bytes = 0;
-  }
 }
 
 void ConcurrentMarking::Run(int task_id, TaskState* task_state) {
@@ -458,40 +451,39 @@ void ConcurrentMarking::Run(int task_id, TaskState* task_state) {
   }
   {
     TimedScope scope(&time_ms);
-    bool done = false;
-    while (!done) {
+    {
       base::LockGuard<base::Mutex> guard(&task_state->lock);
-      size_t current_marked_bytes = 0;
-      int objects_processed = 0;
-      while (current_marked_bytes < kBytesUntilInterruptCheck &&
-             objects_processed < kObjectsUntilInterrupCheck) {
-        HeapObject* object;
-        if (!shared_->Pop(task_id, &object)) {
-          done = true;
-          break;
+      bool done = false;
+      while (!done) {
+        size_t current_marked_bytes = 0;
+        int objects_processed = 0;
+        while (current_marked_bytes < kBytesUntilInterruptCheck &&
+               objects_processed < kObjectsUntilInterrupCheck) {
+          HeapObject* object;
+          if (!shared_->Pop(task_id, &object)) {
+            done = true;
+            break;
+          }
+          objects_processed++;
+          Address new_space_top = heap_->new_space()->original_top();
+          Address new_space_limit = heap_->new_space()->original_limit();
+          Address addr = object->address();
+          if (new_space_top <= addr && addr < new_space_limit) {
+            on_hold_->Push(task_id, object);
+          } else {
+            Map* map = object->synchronized_map();
+            current_marked_bytes += visitor.Visit(map, object);
+          }
         }
-        objects_processed++;
-        Address new_space_top = heap_->new_space()->original_top();
-        Address new_space_limit = heap_->new_space()->original_limit();
-        Address addr = object->address();
-        if (new_space_top <= addr && addr < new_space_limit) {
-          on_hold_->Push(task_id, object);
-        } else {
-          Map* map = object->synchronized_map();
-          current_marked_bytes += visitor.Visit(map, object);
+        marked_bytes += current_marked_bytes;
+        base::AsAtomicWord::Relaxed_Store<size_t>(&task_state->marked_bytes,
+                                                  marked_bytes);
+        if (task_state->interrupt_request.Value()) {
+          task_state->interrupt_condition.Wait(&task_state->lock);
         }
       }
-      marked_bytes += current_marked_bytes;
-      base::AsAtomicWord::Relaxed_Store<size_t>(&task_state->marked_bytes,
-                                                marked_bytes);
-      if (task_state->interrupt_request.Value()) {
-        task_state->interrupt_condition.Wait(&task_state->lock);
-      }
-    }
-    {
-      // Take the lock to synchronize with worklist update after
+      // The lock is also required to synchronize with worklist update after
       // young generation GC.
-      base::LockGuard<base::Mutex> guard(&task_state->lock);
       bailout_->FlushToGlobal(task_id);
       on_hold_->FlushToGlobal(task_id);
     }

src/heap/concurrent-marking.h

@@ -5,7 +5,11 @@
 #ifndef V8_HEAP_CONCURRENT_MARKING_H_
 #define V8_HEAP_CONCURRENT_MARKING_H_
 
+#include "include/v8-platform.h"
 #include "src/allocation.h"
+#include "src/base/atomic-utils.h"
+#include "src/base/platform/condition-variable.h"
+#include "src/base/platform/mutex.h"
 #include "src/cancelable-task.h"
 #include "src/heap/spaces.h"
 #include "src/heap/worklist.h"
@@ -36,7 +40,7 @@ class ConcurrentMarking {
     ConcurrentMarking* concurrent_marking_;
   };
 
-  static const int kMaxTasks = 4;
+  static constexpr int kMaxTasks = 4;
   using MarkingWorklist = Worklist<HeapObject*, 64 /* segment size */>;
 
   ConcurrentMarking(Heap* heap, MarkingWorklist* shared,
@@ -68,25 +72,26 @@ class ConcurrentMarking {
     // The concurrent marker waits on this condition until the request
     // flag is cleared by the main thread.
     base::ConditionVariable interrupt_condition;
+
     LiveBytesMap live_bytes;
-    size_t marked_bytes;
+    size_t marked_bytes = 0;
     char cache_line_padding[64];
   };
   class Task;
   void Run(int task_id, TaskState* task_state);
-  Heap* heap_;
-  MarkingWorklist* shared_;
-  MarkingWorklist* bailout_;
-  MarkingWorklist* on_hold_;
-  WeakObjects* weak_objects_;
+  Heap* const heap_;
+  MarkingWorklist* const shared_;
+  MarkingWorklist* const bailout_;
+  MarkingWorklist* const on_hold_;
+  WeakObjects* const weak_objects_;
   TaskState task_state_[kMaxTasks + 1];
-  base::AtomicNumber<size_t> total_marked_bytes_;
+  base::AtomicNumber<size_t> total_marked_bytes_{0};
   base::Mutex pending_lock_;
   base::ConditionVariable pending_condition_;
-  int pending_task_count_;
-  bool is_pending_[kMaxTasks + 1];
-  CancelableTaskManager::Id cancelable_id_[kMaxTasks + 1];
-  int task_count_;
+  int pending_task_count_ = 0;
+  bool is_pending_[kMaxTasks + 1] = {};
+  CancelableTaskManager::Id cancelable_id_[kMaxTasks + 1] = {};
+  int task_count_ = 0;
 };
 
 }  // namespace internal