damccorm commented on code in PR #24881:
URL: https://github.com/apache/beam/pull/24881#discussion_r1062615527
##########
sdks/go/pkg/beam/core/metrics/metrics_test.go:
##########
@@ -617,21 +617,23 @@ func TestPcolQueryResult(t *testing.T) {
}
}
-// Run on @lostluck's desktop (2020/01/21) go1.13.4
+// Run on @shanemhansen's desktop (2022/01/03) go1.20 RC1 after changing
hashName to use a pool of hashers
+// sync.Pool can return thread-local results eliminating the need for a lock
and increasing throughput.
+// There are users in the wild who create an excessive number of Counters so a
4x improvement in throughput at the expense of
+// creating GOMAXPROCS hasher values seems reasonable.
//
-// Allocs & bytes should be consistent within go versions, but ns/op is
relative to the running machine.
-//
-// BenchmarkMetrics/counter_inplace-12 6054129 208
ns/op 48 B/op 1 allocs/op
-// BenchmarkMetrics/distribution_inplace-12 5707147 228
ns/op 48 B/op 1 allocs/op
-// BenchmarkMetrics/gauge_inplace-12 4742331 259
ns/op 48 B/op 1 allocs/op
-// BenchmarkMetrics/counter_predeclared-12 90147133
12.7 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/distribution_predeclared-12 55396678
21.6 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/gauge_predeclared-12 18535839
60.5 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/counter_raw-12 159581343
7.18 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/distribution_raw-12 82724314
14.7 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/gauge_raw-12 23292386
55.2 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/getStore-12 309361303
3.78 ns/op 0 B/op 0 allocs/op
-// BenchmarkMetrics/getCounterSet-12 287720998
3.98 ns/op 0 B/op 0 allocs/op
+// name old time/op new time/op delta
+// Metrics/counter_inplace-12 376ns ±17% 88ns ± 7% -76.66%
(p=0.008 n=5+5)
+// Metrics/distribution_inplace-12 394ns ± 3% 153ns ± 8% -61.17%
(p=0.008 n=5+5)
+// Metrics/gauge_inplace-12 371ns ± 4% 258ns ± 1% -30.37%
(p=0.008 n=5+5)
Review Comment:
Nice
##########
sdks/go/pkg/beam/core/metrics/metrics_test.go:
##########
@@ -617,21 +617,23 @@ func TestPcolQueryResult(t *testing.T) {
}
}
-// Run on @lostluck's desktop (2020/01/21) go1.13.4
+// Run on @shanemhansen's desktop (2022/01/03) go1.20 RC1 after changing
hashName to use a pool of hashers
+// sync.Pool can return thread-local results eliminating the need for a lock
and increasing throughput.
+// There are users in the wild who create an excessive number of Counters so a
4x improvement in throughput at the expense of
+// creating GOMAXPROCS hasher values seems reasonable.
Review Comment:
We should probably cut some of this before merging.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
