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 <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>

Author: mknyszek

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)

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) {