cppgc: Fix CppHeap::TraceEpilogue

Reporting marked bytes after atomic sweeping means we might be missing
allocations in case finalizers are allocating during sweeping.
Instead report marked bytes and marking time directly to
LocalEmbedderHeapTracer as soon as marking is done.

Bug: chromium:1056170
Change-Id: Ie770f077d2eec10dea182a503a7cd514d3b66baf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2748579
Commit-Queue: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73318}

src/heap/cppgc-js/cpp-heap.cc

@@ -360,6 +360,13 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
     marker_->LeaveAtomicPause();
   }
   marker_.reset();
+  if (isolate_) {
+    auto* tracer = isolate_->heap()->local_embedder_heap_tracer();
+    DCHECK_NOT_NULL(tracer);
+    tracer->UpdateRemoteStats(
+        stats_collector_->marked_bytes(),
+        stats_collector_->marking_time().InMillisecondsF());
+  }
   ExecutePreFinalizers();
   // TODO(chromium:1056170): replace build flag with dedicated flag.
 #if DEBUG
@@ -385,8 +392,8 @@ void CppHeap::TraceEpilogue(TraceSummary* trace_summary) {
     sweeper().Start(sweeping_config);
   }
   DCHECK_NOT_NULL(trace_summary);
-  trace_summary->allocated_size = stats_collector_->marked_bytes();
-  trace_summary->time = stats_collector_->marking_time().InMillisecondsF();
+  trace_summary->allocated_size = SIZE_MAX;
+  trace_summary->time = 0;
   in_atomic_pause_ = false;
   sweeper().NotifyDoneIfNeeded();
 }
@@ -436,6 +443,7 @@ void CppHeap::CollectGarbageForTesting(
     AdvanceTracing(std::numeric_limits<double>::infinity());
     TraceSummary trace_summary;
     TraceEpilogue(&trace_summary);
+    DCHECK_EQ(SIZE_MAX, trace_summary.allocated_size);
   }
 }
 

src/heap/embedder-tracing.cc

@@ -34,14 +34,19 @@ void LocalEmbedderHeapTracer::TraceEpilogue() {
 
   EmbedderHeapTracer::TraceSummary summary;
   remote_tracer_->TraceEpilogue(&summary);
-  remote_stats_.used_size = summary.allocated_size;
+  if (summary.allocated_size == SIZE_MAX) return;
+  UpdateRemoteStats(summary.allocated_size, summary.time);
+}
+
+void LocalEmbedderHeapTracer::UpdateRemoteStats(size_t allocated_size,
+                                                double time) {
+  remote_stats_.used_size = allocated_size;
   // Force a check next time increased memory is reported. This allows for
   // setting limits close to actual heap sizes.
   remote_stats_.allocated_size_limit_for_check = 0;
   constexpr double kMinReportingTimeMs = 0.5;
-  if (summary.time > kMinReportingTimeMs) {
-    isolate_->heap()->tracer()->RecordEmbedderSpeed(summary.allocated_size,
-                                                    summary.time);
+  if (time > kMinReportingTimeMs) {
+    isolate_->heap()->tracer()->RecordEmbedderSpeed(allocated_size, time);
   }
 }
 

src/heap/embedder-tracing.h

@@ -128,6 +128,8 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
     wrapper_descriptor_ = wrapper_descriptor;
   }
 
+  void UpdateRemoteStats(size_t, double);
+
  private:
   static constexpr size_t kEmbedderAllocatedThreshold = 128 * KB;