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

add Sign1Message benchmarks #54

Merged
merged 3 commits into from
Apr 22, 2022
Merged

add Sign1Message benchmarks #54

merged 3 commits into from
Apr 22, 2022

Conversation

qmuntal
Copy link
Contributor

@qmuntal qmuntal commented Apr 21, 2022

This PR adds benchmarks for Sign1Message methods. These benchmarks will be useful later one to do memory and cpu profiling.

These are the results on my Windows laptop (use only for reference):

goos: windows
goarch: amd64
pkg: github.com/veraison/go-cose
cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
BenchmarkSign1Message_MarshalCBOR-12              666874              1791 ns/op             692 B/op         17 allocs/op
BenchmarkSign1Message_UnmarshalCBOR-12            368671              3303 ns/op            1592 B/op         28 allocs/op
BenchmarkSign1Message_Sign-12                     631834              1739 ns/op             668 B/op         16 allocs/op
BenchmarkSign1Message_Verify-12                   585108              1910 ns/op             668 B/op         16 allocs/op

For #52

Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

Looks good (and useful) to me, thanks!

I have inlined a small editorial comment.

bench_test.go Outdated
Comment on lines 25 to 32
type zeroReader struct{}

func (zeroReader) Read(b []byte) (int, error) {
for i := range b {
b[i] = 0
}
return len(b), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this a dup of zeroSource in conformance_test.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
@yogeshbdeshpande
Copy link
Contributor

yogeshbdeshpande commented Apr 22, 2022

Few General comments:
Are you planning to have bench test for sign in other file, then please rename this file to bench_test_sign1.go
Perhaps for future, makes more sense to move the bench test files to a new folder as benchmark and add all the tests there?

@qmuntal
Copy link
Contributor Author

qmuntal commented Apr 22, 2022

Few General comments: Are you planning to have bench test for sign in other file, then please rename this file to bench_test_sign1.go Perhaps for future, makes more sense to move the bench test files to a new folder as benchmark and add all the tests there?

I plan to stick with bench_test.go for new benchmarks, as I don't expect that those grow too much.
Also, it is common and idiomatic to keep benchmark tests in the same directory as the code being benchmarked.

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

bench_test.go Outdated Show resolved Hide resolved
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit 08be6c0 into veraison:main Apr 22, 2022
@qmuntal qmuntal deleted the bench-sign1 branch April 22, 2022 11:05
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.

4 participants