[heap] Fix bug in efficiency and collection rate metrics

When calculating the GC collection rate, we assume that the start object
size (before GC) is non zero. It appears that this is not always the
case, not only because of tests that explicitly trigger GC, but also in
Chrome, when the --gc-interval flag is used with a small interval value.

Furthermore, efficiency calculation (freed bytes over GC duration)
assumes that the duration of the GC is non zero. However, if the clock
resolution is not small enough and the entire GC is very short, the
timed value appears to be zero. This again leads to NaN values showing
in metrics and CHECKs failing and has already been fixed for Oilpan
(crrev.com/c/3723499).

This CL fixes these two issues.

Change-Id: I902b2e9740d9750a2b6463a00289625500c4c0d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3810393Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82205}

src/heap/cppgc/stats-collector.cc

@@ -222,9 +222,13 @@ MetricRecorder::GCCycle GetCycleEventForMetricRecorder(
   event.memory.after_bytes = memory_after_bytes;
   event.memory.freed_bytes = memory_freed_bytes;
   // Collection Rate:
-  event.collection_rate_in_percent =
-      static_cast<double>(event.objects.after_bytes) /
-      event.objects.before_bytes;
+  if (event.objects.before_bytes == 0) {
+    event.collection_rate_in_percent = 0;
+  } else {
+    event.collection_rate_in_percent =
+        static_cast<double>(event.objects.after_bytes) /
+        event.objects.before_bytes;
+  }
   // Efficiency:
   if (event.objects.freed_bytes == 0) {
     event.efficiency_in_bytes_per_us = 0;

src/heap/gc-tracer.cc

@@ -1715,16 +1715,32 @@ void GCTracer::ReportYoungCycleToRecorder() {
   event.main_thread_wall_clock_duration_in_us =
       static_cast<int64_t>(main_thread_wall_clock_duration_in_us);
   // Collection Rate:
-  event.collection_rate_in_percent =
-      static_cast<double>(current_.survived_young_object_size) /
-      current_.young_object_size;
+  if (current_.young_object_size == 0) {
+    event.collection_rate_in_percent = 0;
+  } else {
+    event.collection_rate_in_percent =
+        static_cast<double>(current_.survived_young_object_size) /
+        current_.young_object_size;
+  }
   // Efficiency:
   auto freed_bytes =
       current_.young_object_size - current_.survived_young_object_size;
-  event.efficiency_in_bytes_per_us =
-      freed_bytes / total_wall_clock_duration_in_us;
-  event.main_thread_efficiency_in_bytes_per_us =
-      freed_bytes / main_thread_wall_clock_duration_in_us;
+  if (freed_bytes == 0) {
+    event.efficiency_in_bytes_per_us = 0;
+    event.main_thread_efficiency_in_bytes_per_us = 0;
+  } else {
+    // Here, main_thread_wall_clock_duration_in_us or even
+    // total_wall_clock_duration_in_us can be zero if the clock resolution is
+    // not small enough and the entire GC was very short, so the timed value
+    // was zero. This appears to happen on Windows, see crbug.com/1338256 and
+    // crbug.com/1339180, related to the same issue in cppgc. In this case, we
+    // are only here if the number of freed bytes is nonzero and the division
+    // below produces an infinite value.
+    event.efficiency_in_bytes_per_us =
+        freed_bytes / total_wall_clock_duration_in_us;
+    event.main_thread_efficiency_in_bytes_per_us =
+        freed_bytes / main_thread_wall_clock_duration_in_us;
+  }
 
   recorder->AddMainThreadEvent(event, GetContextId(heap_->isolate()));
 }