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

Check usage of int64 vs sfixed64 in metrics proto and uint64 vs fixed64 #211

Closed
bogdandrutu opened this issue Aug 21, 2020 · 3 comments · Fixed by #214
Closed

Check usage of int64 vs sfixed64 in metrics proto and uint64 vs fixed64 #211

bogdandrutu opened this issue Aug 21, 2020 · 3 comments · Fixed by #214
Labels
priority:p1 release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics

Comments

@bogdandrutu
Copy link
Member

No description provided.

@bogdandrutu bogdandrutu added spec:metrics release:required-for-ga Must be resolved before GA release, or nice to have before GA priority:p1 labels Aug 21, 2020
@tigrannajaryan
Copy link
Member

Do you want to make sure we use them consistently or you want to check to see if there are any benefits and drawbacks?

They are all the same in-memory size, int versions may be more compact on the wire (for small values) and may be a tiny bit slower than fixed versions when marshaling/unmarshaling (likely negligible).

@bogdandrutu
Copy link
Member Author

bogdandrutu commented Aug 25, 2020

/private/var/folders/5c/5p_3jmvd6qx0rsvmb9j7c_s00000gn/T/___BenchmarkEncodeDecode_in_github_com_bogdandrutu_metrics_proto_benchmark__3_ -test.v -test.bench ^\QBenchmarkEncodeDecode\E$ -test.run ^$
goos: darwin
goarch: amd64
pkg: github.com/bogdandrutu/metrics-proto/benchmark
BenchmarkEncodeDecode
BenchmarkEncodeDecode/merged
BenchmarkEncodeDecode/merged-16         	   35745	     33669 ns/op	   30288 B/op	     523 allocs/op
BenchmarkEncodeDecode/mergedfixed
BenchmarkEncodeDecode/mergedfixed-16    	   36036	     33310 ns/op	   30416 B/op	     523 allocs/op
BenchmarkEncodeDecode/unmerged
BenchmarkEncodeDecode/unmerged-16       	   36104	     32878 ns/op	   29168 B/op	     523 allocs/op
BenchmarkEncodeDecode/unmergedfixed
BenchmarkEncodeDecode/unmergedfixed-16  	   36522	     32474 ns/op	   29296 B/op	     523 allocs/op
PASS

Process finished with exit code 0

@jmacd
Copy link
Contributor

jmacd commented Aug 28, 2020

This was discussed and we did not find a lot of support.

@jmacd jmacd closed this as completed Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p1 release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:metrics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants