heap: Re-land trivial refactorings from change 1713626

- Minor ThreadHeapStatsCollector fixes
- Refactor sweeping finalization to reflect the two-step approach (Notify ->
  PostSweep) that already happened in practice.

Initially landed see https://crrev.com/c/1713626

Bug: 982754
Change-Id: I358e0cb43129fd631c31b626e64157e3ba57ce07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724675
Auto-Submit: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682274}

third_party/blink/renderer/platform/heap/heap_stats_collector.cc

@@ -134,7 +134,7 @@ void ThreadHeapStatsCollector::NotifySweepingCompleted() {
           ? static_cast<double>(current().marked_bytes) /
                 current_.object_size_in_bytes_before_sweeping
           : 0.0;
-  current_.gc_nested_in_v8_ = gc_nested_in_v8_;
+  current_.gc_nested_in_v8 = gc_nested_in_v8_;
   gc_nested_in_v8_ = base::TimeDelta();
   // Reset the current state.
   static_assert(std::is_trivially_copyable<Event>::value,

third_party/blink/renderer/platform/heap/heap_stats_collector.h

@@ -278,7 +278,7 @@ class PLATFORM_EXPORT ThreadHeapStatsCollector {
     size_t partition_alloc_bytes_before_sweeping = 0;
     double live_object_rate = 0;
     size_t wrapper_count_before_sweeping = 0;
-    base::TimeDelta gc_nested_in_v8_;
+    base::TimeDelta gc_nested_in_v8;
   };
 
   // Indicates a new garbage collection cycle.
@@ -348,7 +348,6 @@ class PLATFORM_EXPORT ThreadHeapStatsCollector {
   // Statistics for the previously running garbage collection.
   const Event& previous() const { return previous_; }
 
-
   void RegisterObserver(ThreadHeapStatsObserver* observer);
   void UnregisterObserver(ThreadHeapStatsObserver* observer);
 

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -637,7 +637,7 @@ void ThreadState::PerformIdleLazySweep(base::TimeTicks deadline) {
   }
 
   if (sweep_completed) {
-    PostSweep();
+    NotifySweepDone();
   }
 }
 
@@ -884,7 +884,7 @@ void ThreadState::AtomicPauseEpilogue() {
   if (!IsSweepingInProgress()) {
     // Sweeping was finished during the atomic pause. Update statistics needs to
     // run outside of the top-most stats scope.
-    UpdateStatisticsAfterSweeping();
+    PostSweep();
   }
 }
 
@@ -918,7 +918,7 @@ void ThreadState::CompleteSweep() {
     if (!was_in_atomic_pause)
       LeaveAtomicPause();
   }
-  PostSweep();
+  NotifySweepDone();
 }
 
 void ThreadState::SynchronizeAndFinishConcurrentSweeping() {
@@ -1011,7 +1011,7 @@ void UpdateHistograms(const ThreadHeapStatsCollector::Event& event) {
   UMA_HISTOGRAM_TIMES("BlinkGC.TimeForIncrementalMarking",
                       event.incremental_marking_time());
   UMA_HISTOGRAM_TIMES("BlinkGC.TimeForMarking", event.marking_time());
-  UMA_HISTOGRAM_TIMES("BlinkGC.TimeForNestedInV8", event.gc_nested_in_v8_);
+  UMA_HISTOGRAM_TIMES("BlinkGC.TimeForNestedInV8", event.gc_nested_in_v8);
   UMA_HISTOGRAM_TIMES("BlinkGC.TimeForSweepingForeground",
                       event.foreground_sweeping_time());
   UMA_HISTOGRAM_TIMES("BlinkGC.TimeForSweepingBackground",
@@ -1110,30 +1110,29 @@ void UpdateHistograms(const ThreadHeapStatsCollector::Event& event) {
 
 }  // namespace
 
-void ThreadState::UpdateStatisticsAfterSweeping() {
-  DCHECK(!IsSweepingInProgress());
-  DCHECK(Heap().stats_collector()->is_started());
-  Heap().stats_collector()->NotifySweepingCompleted();
-  if (IsMainThread())
-    UpdateHistograms(Heap().stats_collector()->previous());
-  // Emit trace counters for all threads.
-  UpdateTraceCounters(*Heap().stats_collector());
+void ThreadState::NotifySweepDone() {
+  DCHECK(CheckThread());
+  SetGCPhase(GCPhase::kNone);
+  if (!in_atomic_pause()) {
+    PostSweep();
+  }
 }
 
 void ThreadState::PostSweep() {
-  DCHECK(CheckThread());
-
-  SetGCPhase(GCPhase::kNone);
+  DCHECK(!in_atomic_pause());
+  DCHECK(!IsSweepingInProgress());
 
   gc_age_++;
 
   for (auto* const observer : observers_)
     observer->OnCompleteSweepDone();
 
-  if (!in_atomic_pause()) {
-    // Immediately update the statistics if running outside of the atomic pause.
-    UpdateStatisticsAfterSweeping();
-  }
+  Heap().stats_collector()->NotifySweepingCompleted();
+
+  if (IsMainThread())
+    UpdateHistograms(Heap().stats_collector()->previous());
+  // Emit trace counters for all threads.
+  UpdateTraceCounters(*Heap().stats_collector());
 }
 
 void ThreadState::SafePoint(BlinkGC::StackState stack_state) {

third_party/blink/renderer/platform/heap/thread_state.h

@@ -293,6 +293,7 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
 
   void CompleteSweep();
   void FinishSnapshot();
+  void NotifySweepDone();
   void PostSweep();
 
   // Returns whether it is currently allowed to allocate an object. Mainly used
@@ -500,8 +501,6 @@ class PLATFORM_EXPORT ThreadState final : private RAILModeObserver {
                       BlinkGC::SweepingType,
                       BlinkGC::GCReason);
 
-  void UpdateStatisticsAfterSweeping();
-
   // The version is needed to be able to start incremental marking.
   void MarkPhasePrologue(BlinkGC::StackState,
                          BlinkGC::MarkingType,