runtime: ensure consistency of /gc/scan/*

Currently /gc/scan/total:bytes is computed as a separate sum. Compute it
using the same inputs so it's always consistent with the sum of
everything else in /gc/scan/*.

For #56857.

Change-Id: I43d9148a23b1d2eb948ae990193dca1da85df8a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/497880
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/src/runtime/metrics.go b/src/runtime/metrics.go
index 0317cc078abd7db6699a414fef78cfcef6ca334f..442fbf64cd8da16a287bcf2892ab9abc0e57de59 100644 (file)
--- a/src/runtime/metrics.go
+++ b/src/runtime/metrics.go
@@ -192,19 +192,25 @@ func initMetrics() {
                "/gc/scan/globals:bytes": {
                        compute: func(in *statAggregate, out *metricValue) {
                                out.kind = metricKindUint64
-                               out.scalar = gcController.globalsScan.Load()
+                               out.scalar = in.gcStats.globalsScan
                        },
                },
                "/gc/scan/heap:bytes": {
                        compute: func(in *statAggregate, out *metricValue) {
                                out.kind = metricKindUint64
-                               out.scalar = gcController.heapScan.Load()
+                               out.scalar = in.gcStats.heapScan
+                       },
+               },
+               "/gc/scan/stack:bytes": {
+                       compute: func(in *statAggregate, out *metricValue) {
+                               out.kind = metricKindUint64
+                               out.scalar = in.gcStats.stackScan
                        },
                },
                "/gc/scan/total:bytes": {
                        compute: func(in *statAggregate, out *metricValue) {
                                out.kind = metricKindUint64
-                               out.scalar = gcController.globalsScan.Load() + gcController.heapScan.Load() + gcController.lastStackScan.Load()
+                               out.scalar = in.gcStats.totalScan
                        },
                },
                "/gc/heap/allocs-by-size:bytes": {
@@ -318,12 +324,6 @@ func initMetrics() {
                                hist.counts[len(hist.counts)-1] = memstats.gcPauseDist.overflow.Load()
                        },
                },
-               "/gc/scan/stack:bytes": {
-                       compute: func(in *statAggregate, out *metricValue) {
-                               out.kind = metricKindUint64
-                               out.scalar = uint64(gcController.lastStackScan.Load())
-                       },
-               },
                "/gc/stack/starting-size:bytes": {
                        compute: func(in *statAggregate, out *metricValue) {
                                out.kind = metricKindUint64
@@ -505,6 +505,7 @@ const (
        heapStatsDep statDep = iota // corresponds to heapStatsAggregate
        sysStatsDep                 // corresponds to sysStatsAggregate
        cpuStatsDep                 // corresponds to cpuStatsAggregate
+       gcStatsDep                  // corresponds to gcStatsAggregate
        numStatsDeps
 )
 
@@ -666,6 +667,23 @@ func (a *cpuStatsAggregate) compute() {
        // a.cpuStats.accumulate(nanotime(), gcphase == _GCmark)
 }
 
+// cpuStatsAggregate represents various GC stats obtained from the runtime
+// acquired together to avoid skew and inconsistencies.
+type gcStatsAggregate struct {
+       heapScan    uint64
+       stackScan   uint64
+       globalsScan uint64
+       totalScan   uint64
+}
+
+// compute populates the gcStatsAggregate with values from the runtime.
+func (a *gcStatsAggregate) compute() {
+       a.heapScan = gcController.heapScan.Load()
+       a.stackScan = uint64(gcController.lastStackScan.Load())
+       a.globalsScan = gcController.globalsScan.Load()
+       a.totalScan = a.heapScan + a.stackScan + a.globalsScan
+}
+
 // nsToSec takes a duration in nanoseconds and converts it to seconds as
 // a float64.
 func nsToSec(ns int64) float64 {
@@ -682,6 +700,7 @@ type statAggregate struct {
        heapStats heapStatsAggregate
        sysStats  sysStatsAggregate
        cpuStats  cpuStatsAggregate
+       gcStats   gcStatsAggregate
 }
 
 // ensure populates statistics aggregates determined by deps if they
@@ -702,6 +721,8 @@ func (a *statAggregate) ensure(deps *statDepSet) {
                        a.sysStats.compute()
                case cpuStatsDep:
                        a.cpuStats.compute()
+               case gcStatsDep:
+                       a.gcStats.compute()
                }
        }
        a.ensured = a.ensured.union(missing)

diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go
index 45e920673e4c16d2b16f4368ed6f4e3d6a4ad83d..83f6ecddf3638b2004cdf4745cf698a9128335f0 100644 (file)
--- a/src/runtime/metrics_test.go
+++ b/src/runtime/metrics_test.go
@@ -214,6 +214,9 @@ func TestReadMetricsConsistency(t *testing.T) {
                numGC  uint64
                pauses uint64
        }
+       var totalScan struct {
+               got, want uint64
+       }
        var cpu struct {
                gcAssist    float64
                gcDedicated float64
@@ -296,6 +299,14 @@ func TestReadMetricsConsistency(t *testing.T) {
                        for i := range h.Counts {
                                gc.pauses += h.Counts[i]
                        }
+               case "/gc/scan/heap:bytes":
+                       totalScan.want += samples[i].Value.Uint64()
+               case "/gc/scan/globals:bytes":
+                       totalScan.want += samples[i].Value.Uint64()
+               case "/gc/scan/stack:bytes":
+                       totalScan.want += samples[i].Value.Uint64()
+               case "/gc/scan/total:bytes":
+                       totalScan.got = samples[i].Value.Uint64()
                case "/sched/gomaxprocs:threads":
                        if got, want := samples[i].Value.Uint64(), uint64(runtime.GOMAXPROCS(-1)); got != want {
                                t.Errorf("gomaxprocs doesn't match runtime.GOMAXPROCS: got %d, want %d", got, want)
@@ -387,6 +398,9 @@ func TestReadMetricsConsistency(t *testing.T) {
        if gc.pauses < gc.numGC*2 {
                t.Errorf("fewer pauses than expected: got %d, want at least %d", gc.pauses, gc.numGC*2)
        }
+       if totalScan.got != totalScan.want {
+               t.Errorf("/gc/scan/total:bytes doesn't line up with sum of /gc/scan*: total %d vs. sum %d", totalScan.got, totalScan.want)
+       }
 }
 
 func BenchmarkReadMetricsLatency(b *testing.B) {