Add benchmark for metrics recording by skonto · Pull Request #2052 · knative/pkg

skonto · 2021-03-09T15:22:14Z

Adds a benchmark to be used as a baseline for future changes or migrations.
This is helpful especially because metrics reporting happens in many places and after some user request eg. eventing.
/cc @evankanderson
I used this benchmark to compare with branch release-0.20. Results are bellow.
Possibly due to our change in Fix potential deadlock when k8s client is used #2031 moving to a channel implementation we add some overhead ~1ms but for overloaded
pods with many goroutines things get better (I will compare in the future with using atomic values) probably due to this.
It would be nice numbers to be validated independently (or percentage diffs).

main

$go test -run=^$ -test.v -test.benchmem=true -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-12         	  716590	      2110 ns/op	     368 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-12           	  356906	      3212 ns/op	     368 B/op	       9 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-12   	  568777	      2252 ns/op	     400 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-12     	  295448	      3850 ns/op	     400 B/op	       9 allocs/op
PASS
ok  	knative.dev/pkg/metrics	6.185s


$ go test -run=^$ -test.v -test.benchmem=true -cpu=3000 -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-3000         	  540606	      1984 ns/op	     368 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-3000           	  379110	      3114 ns/op	     369 B/op	       9 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-3000   	  519412	      2420 ns/op	     400 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-3000     	  350462	      3914 ns/op	     401 B/op	       9 allocs/op
PASS
ok  	knative.dev/pkg/metrics	5.516s


$ go test -run=^$ -test.v -test.benchmem=true -cpu=15000 -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-15000         	  826749	      2125 ns/op	     368 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-15000           	  267745	      3974 ns/op	     374 B/op	       9 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-15000   	  433729	      2634 ns/op	     400 B/op	       9 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-15000     	  279043	      4349 ns/op	     405 B/op	       9 allocs/op
PASS
ok  	knative.dev/pkg/metrics	13.112s

release-0.20

$ go test -run=^$ -test.v -test.benchmem=true -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-12         	  891790	      1182 ns/op	     264 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-12           	 1000000	      1176 ns/op	     264 B/op	       7 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-12   	  891654	      1240 ns/op	     296 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-12     	 1000000	      1360 ns/op	     296 B/op	       7 allocs/op
PASS
ok  	knative.dev/pkg/metrics	4.809s

$ go test -run=^$ -test.v -test.benchmem=true -cpu=3000 -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-3000         	 1000000	      1268 ns/op	     264 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-3000           	  556707	      2501 ns/op	     265 B/op	       7 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-3000   	  955573	      1591 ns/op	     296 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-3000     	  425730	      2701 ns/op	     296 B/op	       7 allocs/op
PASS
ok  	knative.dev/pkg/metrics	8.039s


$ go test -run=^$ -test.v -test.benchmem=true -cpu=15000 -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-15000         	 1000000	      1501 ns/op	     264 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-15000           	   87499	     12933 ns/op	     298 B/op	       7 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-15000   	  956638	      1524 ns/op	     296 B/op	       7 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-15000     	   26812	     55237 ns/op	     424 B/op	       8 allocs/op
PASS
ok  	knative.dev/pkg/metrics	13.983s

markusthoemmes

Great, thanks!

markusthoemmes · 2021-03-09T16:16:27Z

+			}
+		})
+	})
+	UnregisterResourceView(v...)


nit: defer this after registering the views?

markusthoemmes · 2021-03-09T16:17:32Z

 }
+
+func BenchmarkMetricsRecording(b *testing.B) {
+	ctx := context.Background()


Can we add some tags etc. to this? AFAIK that's causing quite some allocations as well.

codecov · 2021-03-10T11:38:41Z

Codecov Report

Merging #2052 (def79b1) into main (0f8d8de) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2052   +/-   ##
=======================================
  Coverage   67.39%   67.39%           
=======================================
  Files         215      215           
  Lines        9095     9095           
=======================================
  Hits         6130     6130           
  Misses       2690     2690           
  Partials      275      275

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f8d8de...def79b1. Read the comment docs.

skonto · 2021-03-10T11:42:02Z

I added a few tags and also getting the ctx is part of the benchmark evaluation since this is what we do in eventing a) get new ctx with tags b) record.
Here is the interesting thing:
tag.New is really heavy (first is run with a pre-allocated ctx):

$ go test -run=^$ -test.v  -test.benchmem=true  -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-12         	  864126	      1358 ns/op	     499 B/op	      13 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-12           	  574707	      1977 ns/op	     499 B/op	      13 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-12   	  884908	      1535 ns/op	     662 B/op	      17 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-12     	  576783	      2305 ns/op	     662 B/op	      17 allocs/op
PASS
ok  	knative.dev/pkg/metrics	5.095s
$ go test -run=^$ -test.v  -test.benchmem=true  -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-12         	  614230	      2148 ns/op	    1243 B/op	      29 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-12           	  381198	      2827 ns/op	    1247 B/op	      29 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-12   	  506767	      2376 ns/op	    1406 B/op	      33 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-12     	  358122	      3142 ns/op	    1408 B/op	      33 allocs/op
PASS
ok  	knative.dev/pkg/metrics	4.864s

A mem optimized version that skips tags validation and mutator creation:

getTagCtx := func () (context.Context, error) {
		ctx := context.Background()
		ctx, err := tag.New(
			ctx,
			m,
		)
		return ctx, err
}


func New(ctx context.Context, t map[Key]string) (context.Context, error) {
	m := newMap()
	for k, v := range t {
		m.insert(k, v, metadatas{})
	}
	return NewContext(ctx, m), nil
}

$ go test -run=^$ -test.v  -test.benchmem=true  -test.bench=^BenchmarkMetricsRecording$ ./metrics/ 
goos: linux
goarch: amd64
pkg: knative.dev/pkg/metrics
BenchmarkMetricsRecording
BenchmarkMetricsRecording/sequential
BenchmarkMetricsRecording/sequential-12         	  717141	      1737 ns/op	     955 B/op	      17 allocs/op
BenchmarkMetricsRecording/parallel
BenchmarkMetricsRecording/parallel-12           	  476246	      2704 ns/op	     955 B/op	      17 allocs/op
BenchmarkMetricsRecording/sequential-batch
BenchmarkMetricsRecording/sequential-batch-12   	  534591	      2545 ns/op	    1118 B/op	      21 allocs/op
BenchmarkMetricsRecording/parallel-batch
BenchmarkMetricsRecording/parallel-batch-12     	  311800	      3628 ns/op	    1118 B/op	      21 allocs/op
PASS
ok  	knative.dev/pkg/metrics	5.157s

I am wondering if there is a way to bypass all the tags creation stuff, probably not.

skonto · 2021-03-10T12:02:53Z

=== FAIL: metrics TestMemStatsMetrics (1.69s)
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_bucket_hash_sys Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_gc_sys Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_other_sys Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_next_gc Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_last_gc Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_total_gc_pause_ns Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_num_gc Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_num_forced_gc Reporter len(d) 1
    memstats_test.go:54: For metric, unexpected data reported when no data was expected. metric go_gc_cpu_fraction Reporter len(d) 1

Seems unrelated.

skonto · 2021-03-10T12:03:14Z

/retest

skonto · 2021-03-10T12:45:53Z

Btw otel-go seems really promising at least for the labels part, almost no heap allocations. It seems straightforward from an api pov as it should be.

skonto · 2021-03-10T16:30:28Z

@markusthoemmes gentle ping for approval.

markusthoemmes

/lgtm
/approve

Thanks

knative-prow-robot · 2021-03-10T16:48:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markusthoemmes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~metrics/OWNERS~~ [markusthoemmes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

add bench for metrics recording

c346cc5

google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Mar 9, 2021

skonto changed the title ~~add bench for metrics recording~~ Add benchmark for metrics recording Mar 9, 2021

knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 9, 2021

markusthoemmes reviewed Mar 9, 2021

View reviewed changes

fixes

45084b9

fix import

def79b1

markusthoemmes approved these changes Mar 10, 2021

View reviewed changes

knative-prow-robot assigned markusthoemmes Mar 10, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2021

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 10, 2021

knative-prow-robot merged commit 85bc723 into knative:main Mar 10, 2021

skonto mentioned this pull request Apr 1, 2021

Add benchmarking for the http tracing handler #2080

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add benchmark for metrics recording#2052

Add benchmark for metrics recording#2052
knative-prow-robot merged 3 commits into
knative:mainfrom
skonto:add_bench_record

skonto commented Mar 9, 2021 •

edited

Loading

Uh oh!

markusthoemmes left a comment

Uh oh!

markusthoemmes Mar 9, 2021

Uh oh!

markusthoemmes Mar 9, 2021

Uh oh!

skonto Mar 10, 2021

Uh oh!

codecov Bot commented Mar 10, 2021 •

edited

Loading

Uh oh!

skonto commented Mar 10, 2021 •

edited

Loading

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

skonto commented Mar 10, 2021 •

edited

Loading

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

markusthoemmes left a comment

Uh oh!

knative-prow-robot commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

skonto commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markusthoemmes left a comment

Choose a reason for hiding this comment

Uh oh!

markusthoemmes Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

markusthoemmes Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

skonto Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skonto commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

skonto commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skonto commented Mar 10, 2021

Uh oh!

markusthoemmes left a comment

Choose a reason for hiding this comment

Uh oh!

knative-prow-robot commented Mar 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skonto commented Mar 9, 2021 •

edited

Loading

codecov Bot commented Mar 10, 2021 •

edited

Loading

skonto commented Mar 10, 2021 •

edited

Loading

skonto commented Mar 10, 2021 •

edited

Loading