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

Remove wrapping of int and double value from SummaryValue message #51

Conversation

tigrannajaryan
Copy link
Member

This micro-optimization reduces CPU and memory usage.

goos: darwin
goarch: amd64
pkg: github.com/tigrannajaryan/exp-otelproto/encodings
BenchmarkEncode/Baseline/Metric/Summary-8      	     120	  50042473 ns/op
BenchmarkEncode/Proposed/Metric/Summary-8      	     136	  43423088 ns/op

BenchmarkEncode/Baseline/Metric/Mix-8                     	      28	 206595979 ns/op
BenchmarkEncode/Proposed/Metric/Mix-8                     	      30	 199870715 ns/op

BenchmarkEncode/Baseline/Metric/MixSeries-8               	       9	 592705353 ns/op
BenchmarkEncode/Proposed/Metric/MixSeries-8               	      10	 539199628 ns/op

BenchmarkDecode/Baseline/Metric/Summary-8                 	      51	 119929790 ns/op	78696041 B/op	 2024000 allocs/op
BenchmarkDecode/Proposed/Metric/Summary-8                 	      56	 104306363 ns/op	69096032 B/op	 1824000 allocs/op

BenchmarkDecode/Baseline/Metric/Mix-8                     	      10	 545856843 ns/op	328040062 B/op	 9126000 allocs/op
BenchmarkDecode/Proposed/Metric/Mix-8                     	      10	 526862286 ns/op	318440044 B/op	 8926000 allocs/op

BenchmarkDecode/Baseline/Metric/MixSeries-8               	       5	1161078696 ns/op	752040067 B/op	17626000 allocs/op
BenchmarkDecode/Proposed/Metric/MixSeries-8               	       5	1075300086 ns/op	704040064 B/op	16626000 allocs/op

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/remove_summaryvalue_wrapping branch from c743be3 to 743debb Compare November 13, 2019 13:58
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

I don't have an opinion. I'd let people more familiar with the proto comment.

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/remove_summaryvalue_wrapping branch 2 times, most recently from 9fe1588 to db158f2 Compare November 14, 2019 16:36
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/remove_summaryvalue_wrapping branch from db158f2 to 09cfc16 Compare November 14, 2019 22:56
opentelemetry/proto/metrics/v1/metrics.proto Outdated Show resolved Hide resolved
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/remove_summaryvalue_wrapping branch 2 times, most recently from e1dbc26 to 5ae7025 Compare November 15, 2019 17:54
This micro-optimization reduces CPU and memory usage.

```
goos: darwin
goarch: amd64
pkg: github.com/tigrannajaryan/exp-otelproto/encodings
BenchmarkEncode/Baseline/Metric/Summary-8      	     120	  50042473 ns/op
BenchmarkEncode/Proposed/Metric/Summary-8      	     136	  43423088 ns/op

BenchmarkEncode/Baseline/Metric/Mix-8                     	      28	 206595979 ns/op
BenchmarkEncode/Proposed/Metric/Mix-8                     	      30	 199870715 ns/op

BenchmarkEncode/Baseline/Metric/MixSeries-8               	       9	 592705353 ns/op
BenchmarkEncode/Proposed/Metric/MixSeries-8               	      10	 539199628 ns/op

BenchmarkDecode/Baseline/Metric/Summary-8                 	      51	 119929790 ns/op	78696041 B/op	 2024000 allocs/op
BenchmarkDecode/Proposed/Metric/Summary-8                 	      56	 104306363 ns/op	69096032 B/op	 1824000 allocs/op

BenchmarkDecode/Baseline/Metric/Mix-8                     	      10	 545856843 ns/op	328040062 B/op	 9126000 allocs/op
BenchmarkDecode/Proposed/Metric/Mix-8                     	      10	 526862286 ns/op	318440044 B/op	 8926000 allocs/op

BenchmarkDecode/Baseline/Metric/MixSeries-8               	       5	1161078696 ns/op	752040067 B/op	17626000 allocs/op
BenchmarkDecode/Proposed/Metric/MixSeries-8               	       5	1075300086 ns/op	704040064 B/op	16626000 allocs/op
```
@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/remove_summaryvalue_wrapping branch from 5ae7025 to e5a9d23 Compare November 15, 2019 17:56
@bogdandrutu bogdandrutu merged commit 7292b90 into open-telemetry:master Nov 15, 2019
@tigrannajaryan tigrannajaryan deleted the feature/tigran/remove_summaryvalue_wrapping branch November 15, 2019 21:09
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

6 participants