Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Benchmark AddConstraint function #168

Merged
merged 2 commits into from
Jan 12, 2022

Conversation

willbeason
Copy link
Member

We didn't expect this to be problematic, but it's nice to have a
benchmark for this as we do intend to make some changes to what happens
when Constraints are added.

Fix "ModuleSimple" so it actually compiles. I was originally against
having the error check in the benchmark since it's slightly slower, but
(1) the performance difference is negligible and (2) we want to be sure
the CT is actually being compiled, so it's worth the minuscule
performance cost.

Signed-off-by: Will Beason willbeason@google.com

@willbeason
Copy link
Member Author

Current benchmark:

constraint$ go test ./pkg/client -bench=BenchmarkClient_AddConstraint --benchmem
goos: linux
goarch: amd64
pkg: github.com/open-policy-agent/frameworks/constraint/pkg/client
cpu: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
BenchmarkClient_AddConstraint-12    	   10458	     98396 ns/op	   33817 B/op	     522 allocs/op
PASS
ok  	github.com/open-policy-agent/frameworks/constraint/pkg/client	3.867s

@willbeason
Copy link
Member Author

Corrected benchmarks for AddTemplate/Simple:

constraint$ go test ./pkg/client -bench=BenchmarkClient_AddTemplate/Simple --benchmem
goos: linux
goarch: amd64
pkg: github.com/open-policy-agent/frameworks/constraint/pkg/client
cpu: Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz
BenchmarkClient_AddTemplate/Simple/1_Templates-12         	     127	   9277974 ns/op	 1264455 B/op	   25215 allocs/op
BenchmarkClient_AddTemplate/Simple/2_Templates-12         	      61	  19972337 ns/op	 2552709 B/op	   51443 allocs/op
BenchmarkClient_AddTemplate/Simple/5_Templates-12         	      20	  50305978 ns/op	 6609700 B/op	  136224 allocs/op
BenchmarkClient_AddTemplate/Simple/10_Templates-12        	      10	 108790631 ns/op	14119363 B/op	  297911 allocs/op
BenchmarkClient_AddTemplate/Simple/20_Templates-12        	       5	 241780155 ns/op	31798065 B/op	  697543 allocs/op
BenchmarkClient_AddTemplate/Simple/50_Templates-12        	       2	 728062077 ns/op	106720528 B/op	 2508794 allocs/op
BenchmarkClient_AddTemplate/Simple/100_Templates-12       	       1	1910071286 ns/op	303966424 B/op	 7568745 allocs/op
BenchmarkClient_AddTemplate/Simple/200_Templates-12       	       1	5347743779 ns/op	968341776 B/op	25343019 allocs/op

@willbeason
Copy link
Member Author

Fixes #152

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2021

Codecov Report

Merging #168 (61ca0d8) into master (4e828a7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #168   +/-   ##
=======================================
  Coverage   44.92%   44.92%           
=======================================
  Files          59       59           
  Lines        2776     2776           
=======================================
  Hits         1247     1247           
  Misses       1286     1286           
  Partials      243      243           
Flag Coverage Δ
unittests 44.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


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 4e828a7...61ca0d8. Read the comment docs.

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@willbeason willbeason force-pushed the bench-add-constraint branch 2 times, most recently from f43254d to f050413 Compare December 10, 2021 16:58
Will Beason added 2 commits January 12, 2022 08:15
We didn't expect this to be problematic, but it's nice to have a
benchmark for this as we do intend to make some changes to what happens
when Constraints are added.

Fix "ModuleSimple" so it actually compiles. I was originally against
having the error check in the benchmark since it's slightly slower, but
(1) the performance difference is negligible and (2) we want to be sure
the CT is actually being compiled, so it's worth the minuscule
performance cost.

Signed-off-by: Will Beason <willbeason@google.com>
Signed-off-by: Will Beason <willbeason@google.com>
@willbeason willbeason merged commit 8306a8d into open-policy-agent:master Jan 12, 2022
@willbeason willbeason deleted the bench-add-constraint branch January 12, 2022 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants