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

Benchmarks for context conversion #131

Merged
merged 2 commits into from
Dec 16, 2016
Merged

Benchmarks for context conversion #131

merged 2 commits into from
Dec 16, 2016

Conversation

anuptalwalkar
Copy link
Contributor

@anuptalwalkar anuptalwalkar commented Dec 16, 2016

Type conversion benchmarks for wrapping context.Context with fx.Context struct vs using Upgrade/Convert function with type assertion.

Looks like with type assertion when no WithValue is called, saves us 1 alloc and ~7 ns/op. When WithValue is called, type assertion is 8 ns/op expensive.

bash-3.2$ go test -bench=. -benchmem
BenchmarkContext_StructWrapper-8                	30000000	        55.3 ns/op	      32 B/op	       3 allocs/op
BenchmarkContext_StructWrapperWithValue-8       	30000000	        43.5 ns/op	      32 B/op	       3 allocs/op
BenchmarkContext_WithTypeAssertion-8            	30000000	        47.8 ns/op	      16 B/op	       2 allocs/op
BenchmarkContext_WithTypeAssertionWithValue-8   	20000000	        52.8 ns/op	      32 B/op	       3 allocs/op

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.717% when pulling 425ef67 on context-benchmark into aef180e on master.

@anuptalwalkar anuptalwalkar merged commit 8744287 into master Dec 16, 2016
@anuptalwalkar anuptalwalkar deleted the context-benchmark branch December 16, 2016 22:37
Copy link
Contributor

@madhuravi madhuravi left a comment

Choose a reason for hiding this comment

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

What is the conclusion for what we want to do based on this?

f(New(context.Background(), service.NullHost()))
}

func BenchmarkContext_StructWrapper(b *testing.B) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's confusing that the test StructWrapper is calling the function typeConversion on line 41 whereas the test WithTypeAssertion is calling the function typeUpgrade. Fix and clarify naming.

@ascandella
Copy link
Contributor

Numbers look good. Nanoseconds are totally fine. Allocs seem small. I don't think we need to worry about optimizing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants