Revert "[heap] Introduce new state in CollectionBarrier"

This reverts commit 8358ab49.

Reason for revert: TSan issues: https://ci.chromium.org/p/v8/builders/ci/V8%20Linux64%20TSAN/33730

Original change's description:
> [heap] Introduce new state in CollectionBarrier
>
> Introduce new state kCollectionStarted in CollectionBarrier. This state
> is used during Heap::PerformGarbageCollection. It stops threads from
> requesting GC when the GC was already started. This happens because a
> background thread only requests the GC after it parked itself - the GC
> could be started in-between those two events.
>
> Bug: v8:10315
> Change-Id: I59cf3d4ea41c7a2c37ffce89c5b057221a2499e0
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2474858
> Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70572}

TBR=ulan@chromium.org,dinfuehr@chromium.org

Change-Id: Ia67b1cbb931ce1b965876c7a1bbb09f48b8c7b43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10315
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2480563Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70576}

src/heap/collection-barrier.cc

@@ -38,20 +38,7 @@ class BackgroundCollectionInterruptTask : public CancelableTask {
 };
 
 void CollectionBarrier::AwaitCollectionBackground() {
-  bool first;
-
-  {
-    base::MutexGuard guard(&mutex_);
-    first = FirstCollectionRequest();
-    if (first) {
-      // Initialize scope while holding the lock - prevents GC from starting
-      // before setting up this counter
-      time_to_collection_scope_.emplace(
-          heap_->isolate()->counters()->time_to_collection());
-    }
-  }
-
-  if (first) {
+  if (FirstCollectionRequest()) {
     // This is the first background thread requesting collection, ask the main
     // thread for GC.
     ActivateStackGuardAndPostTask();
@@ -62,11 +49,6 @@ void CollectionBarrier::AwaitCollectionBackground() {
 
 void CollectionBarrier::StopTimeToCollectionTimer() {
   base::MutexGuard guard(&mutex_);
-  RequestState old_state = state_.exchange(RequestState::kCollectionStarted,
-                                           std::memory_order_relaxed);
-  USE(old_state);
-  DCHECK(old_state == RequestState::kDefault ||
-         old_state == RequestState::kCollectionRequested);
   time_to_collection_scope_.reset();
 }
 
@@ -78,6 +60,7 @@ void CollectionBarrier::ActivateStackGuardAndPostTask() {
       reinterpret_cast<v8::Isolate*>(isolate));
   taskrunner->PostTask(
       std::make_unique<BackgroundCollectionInterruptTask>(heap_));
+  base::MutexGuard guard(&mutex_);
   time_to_collection_scope_.emplace(isolate->counters()->time_to_collection());
 }
 

src/heap/collection-barrier.h

@@ -29,10 +29,7 @@ class CollectionBarrier {
     kDefault,
 
     // Collection was already requested
-    kCollectionRequested,
-
-    // Collection was already started
-    kCollectionStarted,
+    kCollection,
 
     // This state is reached after isolate starts to shut down. The main
     // thread can't perform any GCs anymore, so all allocations need to be
@@ -43,6 +40,8 @@ class CollectionBarrier {
   // The current state.
   std::atomic<RequestState> state_;
 
+  void BlockUntilCollected();
+
   // Request GC by activating stack guards and posting a task to perform the
   // GC.
   void ActivateStackGuardAndPostTask();
@@ -51,16 +50,14 @@ class CollectionBarrier {
   // kCollection.
   bool FirstCollectionRequest() {
     RequestState expected = RequestState::kDefault;
-    return state_.compare_exchange_strong(expected,
-                                          RequestState::kCollectionRequested);
+    return state_.compare_exchange_strong(expected, RequestState::kCollection);
   }
 
   // Sets state back to kDefault - invoked at end of GC.
   void ClearCollectionRequested() {
     RequestState old_state =
         state_.exchange(RequestState::kDefault, std::memory_order_relaxed);
-    USE(old_state);
-    DCHECK_EQ(old_state, RequestState::kCollectionStarted);
+    CHECK_NE(old_state, RequestState::kShutdown);
   }
 
  public:
@@ -69,12 +66,10 @@ class CollectionBarrier {
 
   // Checks whether any background thread requested GC.
   bool CollectionRequested() {
-    return state_.load(std::memory_order_relaxed) ==
-           RequestState::kCollectionRequested;
+    return state_.load(std::memory_order_relaxed) == RequestState::kCollection;
   }
 
   void StopTimeToCollectionTimer();
-  void BlockUntilCollected();
 
   // Resumes threads waiting for collection.
   void ResumeThreadsAwaitingCollection();

src/heap/heap.cc

@@ -1515,6 +1515,8 @@ bool Heap::CollectGarbage(AllocationSpace space,
       this, IsYoungGenerationCollector(collector) ? "MinorGC" : "MajorGC",
       GarbageCollectionReasonToString(gc_reason));
 
+  collection_barrier_->StopTimeToCollectionTimer();
+
   if (!CanPromoteYoungAndExpandOldGeneration(0)) {
     InvokeNearHeapLimitCallback();
   }
@@ -1933,11 +1935,6 @@ size_t Heap::PerformGarbageCollection(
     GarbageCollector collector, const v8::GCCallbackFlags gc_callback_flags) {
   DisallowJavascriptExecution no_js(isolate());
   base::Optional<SafepointScope> optional_safepoint_scope;
-
-  // Stop time-to-collection timer before safepoint - we do not want to measure
-  // time for safepointing.
-  collection_barrier_->StopTimeToCollectionTimer();
-
   if (FLAG_local_heaps) {
     optional_safepoint_scope.emplace(this);
   }