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 benchmarks to proto #138

Closed
tigrannajaryan opened this issue Apr 15, 2020 · 6 comments
Closed

Add benchmarks to proto #138

tigrannajaryan opened this issue Apr 15, 2020 · 6 comments
Labels
release:after-ga Not required before GA release, and not going to work on before GA

Comments

@tigrannajaryan
Copy link
Member

I have benchmarks in my personal repo https://github.com/tigrannajaryan/exp-otelproto

This contains both microbenchmarks for proto encoding/decoding and also full protocol implementations (OTLP, OpenCensus and others).

I am happy to donate this provided that I get merging rights on this subtree of the repo. I need to be able quickly iterate on this and waiting for regular approvals for changes in benchmarks is not going to work for the upcoming plans I have for iterating on logs support in OTLP. It will just slow me down too much.

If we can introduce CODEOWNERS and give me merging rights for a subdirectory where the benchmarks reside I will be glad to move it to this repo so that other people who need to work on the proto can also benchmark their changes.

If there are concerns about giving merging rights I am happy to keep the benchmarks in my repo, no problem.

@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers what do you think?

@Oberon00
Copy link
Member

Oberon00 commented Apr 16, 2020

You did not mention "Go" anywhere, but it seems you are actually benchmarking only the Go code for parsing/writing the protocol.

The protocol will have different performance depending on the protobuf implementation and language. I think you can't sensibly benchmark the protocol itself, only writers and readers based on it. And I think these should be benchmarked in their respective language repositories.

Related #79.

@tigrannajaryan
Copy link
Member Author

You did not mention "Go" anywhere, but it seems you are actually benchmarking only the Go code for parsing/writing the protocol.
The protocol will have different performance depending on the protobuf implementation and language. I think you can't sensibly benchmark the protocol itself, only writers and readers based on it. And I think these should be benchmarked in their respective language repositories.

@Oberon00 yes, that's correct and I am also not happy that it has been limited to Go so far. It would be nice to add other languages to benchmarks. It would be good if other contributors added benchmarks for other languages.

@bogdandrutu bogdandrutu added the release:after-ga Not required before GA release, and not going to work on before GA label Aug 11, 2020
@ThomsonTan
Copy link

@tigrannajaryan if we don't want to limit the proto benchmark to Go, could we add this to below benchmark spec which recommends other OTLP SDKs to implement the same set of benchmark measurement?

open-telemetry/opentelemetry-specification#748

@tigrannajaryan
Copy link
Member Author

Yes, the benchmark implementation in https://github.com/tigrannajaryan/exp-otelproto can be extended to other languages. I will not be able to do that myself but if there are other contributors will to do that then that would be great.

@tigrannajaryan
Copy link
Member Author

Closing since there has been no interest in this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:after-ga Not required before GA release, and not going to work on before GA
Projects
None yet
Development

No branches or pull requests

4 participants