runtime: fix allocs-by-size and frees-by-size buckets

Currently these two metrics are reported incorrectly, going by the
documentation in the runtime/metrics package. We just copy in the
size-class-based values from the runtime wholesale, but those implicitly
have an inclusive upper-bound and exclusive lower-bound (e.g. 48-byte
size class contains objects in the size range (32, 48]) but the API
declares inclusive lower-bounds and exclusive upper-bounds.

Also, the bottom bucket representing (-inf, 1) should always be empty.
Extend the consistency check to verify this.

Updates #43329.

Change-Id: I11b5b062a34e13405ab662d15334bda91f779775
Reviewed-on: https://go-review.googlesource.com/c/go/+/279467
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: 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 d3c0341aee81fb17667d1d34ff38c37ca93330fc..af86a0f03aa00a99adc78f6faa662e54f7f67da4 100644 (file)
--- a/src/runtime/metrics.go
+++ b/src/runtime/metrics.go
@@ -43,7 +43,18 @@ func initMetrics() {
        }
        sizeClassBuckets = make([]float64, _NumSizeClasses)
        for i := range sizeClassBuckets {
-               sizeClassBuckets[i] = float64(class_to_size[i])
+               // Size classes have an inclusive upper-bound
+               // and exclusive lower bound (e.g. 48-byte size class is
+               // (32, 48]) whereas we want and inclusive lower-bound
+               // and exclusive upper-bound (e.g. 48-byte size class is
+               // [33, 49). We can achieve this by shifting all bucket
+               // boundaries up by 1.
+               //
+               // Also, a float64 can precisely represent integers with
+               // value up to 2^53 and size classes are relatively small
+               // (nowhere near 2^48 even) so this will give us exact
+               // boundaries.
+               sizeClassBuckets[i] = float64(class_to_size[i] + 1)
        }
        timeHistBuckets = timeHistogramMetricsBuckets()
        metrics = map[string]metricData{

diff --git a/src/runtime/metrics_test.go b/src/runtime/metrics_test.go
index 167edd57fd882151a3eab3f2954280b613feda46..0ee469ae29406c38b32aaf6a371c8f2fd9e436d8 100644 (file)
--- a/src/runtime/metrics_test.go
+++ b/src/runtime/metrics_test.go
@@ -154,6 +154,12 @@ func TestReadMetricsConsistency(t *testing.T) {
        if totalVirtual.got != totalVirtual.want {
                t.Errorf(`"/memory/classes/total:bytes" does not match sum of /memory/classes/**: got %d, want %d`, totalVirtual.got, totalVirtual.want)
        }
+       if objects.alloc.Counts[0] > 0 {
+               t.Error("found counts for objects of non-positive size in allocs-by-size")
+       }
+       if objects.free.Counts[0] > 0 {
+               t.Error("found counts for objects of non-positive size in frees-by-size")
+       }
        if len(objects.alloc.Buckets) != len(objects.free.Buckets) {
                t.Error("allocs-by-size and frees-by-size buckets don't match in length")
        } else if len(objects.alloc.Counts) != len(objects.free.Counts) {