From: Michael Anthony Knyszek <mknyszek@google.com>
Date: Tue, 22 Dec 2020 16:23:29 +0000 (+0000)
Subject: runtime: fix allocs-by-size and frees-by-size buckets
X-Git-Tag: go1.16rc1~141
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=8db7e2fecdcd04af31c82d075c60ab6fdf6b7a48;p=gostls13.git

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 d3c0341aee..af86a0f03a 100644
--- 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 167edd57fd..0ee469ae29 100644
--- 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) {