Commit 1d8a669e authored by Nikolaos Papaspyrou's avatar Nikolaos Papaspyrou Committed by V8 LUCI CQ

Reland "cppgc: Minor fix in cppgc efficiency calculation"

This is a reland of commit 543acf34

Original change's description:
> cppgc: Minor fix in cppgc efficiency calculation
>
> 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 leads to NaN values showing in metrics and
> CHECKs failing. This CL fixes the issue.
>
> Bug: chromium:1338256
> Change-Id: I1dbc52072fcde3411aa38fa0c11da25afd107ca8
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3714356
> Reviewed-by: Omer Katz <omerkatz@chromium.org>
> Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#81329}

Bug: chromium:1338256
Bug: chromium:1339180
Change-Id: Ib2b2a6973a6d290adf01568f35a205b606dd99f5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3723499Reviewed-by: 's avatarOmer Katz <omerkatz@chromium.org>
Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81354}
parent 7681b6a9
...@@ -160,6 +160,10 @@ double StatsCollector::GetRecentAllocationSpeedInBytesPerMs() const { ...@@ -160,6 +160,10 @@ double StatsCollector::GetRecentAllocationSpeedInBytesPerMs() const {
namespace { namespace {
int64_t SumPhases(const MetricRecorder::GCCycle::Phases& phases) { int64_t SumPhases(const MetricRecorder::GCCycle::Phases& phases) {
DCHECK_LE(0, phases.mark_duration_us);
DCHECK_LE(0, phases.weak_duration_us);
DCHECK_LE(0, phases.compact_duration_us);
DCHECK_LE(0, phases.sweep_duration_us);
return phases.mark_duration_us + phases.weak_duration_us + return phases.mark_duration_us + phases.weak_duration_us +
phases.compact_duration_us + phases.sweep_duration_us; phases.compact_duration_us + phases.sweep_duration_us;
} }
...@@ -215,11 +219,22 @@ MetricRecorder::GCCycle GetCycleEventForMetricRecorder( ...@@ -215,11 +219,22 @@ MetricRecorder::GCCycle GetCycleEventForMetricRecorder(
static_cast<double>(event.objects.after_bytes) / static_cast<double>(event.objects.after_bytes) /
event.objects.before_bytes; event.objects.before_bytes;
// Efficiency: // Efficiency:
event.efficiency_in_bytes_per_us = if (event.objects.freed_bytes == 0) {
static_cast<double>(event.objects.freed_bytes) / SumPhases(event.total); event.efficiency_in_bytes_per_us = 0;
event.main_thread_efficiency_in_bytes_per_us = event.main_thread_efficiency_in_bytes_per_us = 0;
static_cast<double>(event.objects.freed_bytes) / } else {
SumPhases(event.main_thread); // Here, SumPhases(event.main_thread) or even SumPhases(event.total) 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. 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 =
static_cast<double>(event.objects.freed_bytes) / SumPhases(event.total);
event.main_thread_efficiency_in_bytes_per_us =
static_cast<double>(event.objects.freed_bytes) /
SumPhases(event.main_thread);
}
return event; return event;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment