open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.08k stars 509 forks source link

otelzap: Use pool for encoder #5719

Open khushijain21 opened 4 weeks ago

khushijain21 commented 4 weeks ago

Part of https://github.com/open-telemetry/opentelemetry-go-contrib/issues/5191

Pre-work https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5279

Results:

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/contrib/bridges/otelzap
cpu: 12th Gen Intel(R) Core(TM) i5-1245U
                           │ old_bench.txt │            new_bench.txt             │
                           │    sec/op     │    sec/op     vs base                │
CoreWrite/10_fields-12        872.7n ± 73%   473.1n ± 52%  -45.78% (p=0.000 n=10)
CoreWrite/20_fields-12        2.033µ ± 13%   1.660µ ±  6%  -18.35% (p=0.000 n=10)
CoreWrite/Namespace-12        1.647µ ± 31%   1.033µ ±  3%  -37.28% (p=0.000 n=10)
CoreWrite/With10_fields-12    219.9n ± 19%   218.8n ±  7%        ~ (p=0.912 n=10)
CoreWrite/With20_fields-12    468.6n ± 21%   497.0n ± 23%        ~ (p=0.393 n=10)
CoreWrite/WithNamespace-12    14.64n ± 13%   17.23n ± 21%  +17.69% (p=0.014 n=10)
geomean                       404.9n         339.1n        -16.26%

                           │ old_bench.txt │            new_bench.txt             │
                           │     B/op      │    B/op     vs base                  │
CoreWrite/10_fields-12        978.0 ± 0%     307.0 ± 0%  -68.61% (p=0.000 n=10)
CoreWrite/20_fields-12       2180.0 ± 0%     817.0 ± 0%  -62.52% (p=0.000 n=10)
CoreWrite/Namespace-12       1760.0 ± 0%     609.0 ± 0%  -65.40% (p=0.000 n=10)
CoreWrite/With10_fields-12    208.0 ± 0%     208.0 ± 0%        ~ (p=1.000 n=10) ¹
CoreWrite/With20_fields-12    640.0 ± 0%     640.0 ± 0%        ~ (p=1.000 n=10) ¹
CoreWrite/WithNamespace-12    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                                  ²               -41.35%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                           │ old_bench.txt │            new_bench.txt             │
                           │   allocs/op   │ allocs/op   vs base                  │
CoreWrite/10_fields-12        13.00 ± 0%     10.00 ± 0%  -23.08% (p=0.000 n=10)
CoreWrite/20_fields-12        22.00 ± 0%     17.00 ± 0%  -22.73% (p=0.000 n=10)
CoreWrite/Namespace-12        16.00 ± 0%     12.00 ± 0%  -25.00% (p=0.000 n=10)
CoreWrite/With10_fields-12    1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹
CoreWrite/With20_fields-12    1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹
CoreWrite/WithNamespace-12    0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                                  ²               -12.60%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean
codecov[bot] commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.6%. Comparing base (ff65757) to head (8db2a5e). Report is 20 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719/graphs/tree.svg?width=650&height=150&src=pr&token=P6F3W9WA7Q&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) ```diff @@ Coverage Diff @@ ## main #5719 +/- ## ======================================= + Coverage 64.5% 64.6% +0.1% ======================================= Files 200 200 Lines 12558 12585 +27 ======================================= + Hits 8101 8134 +33 + Misses 4220 4215 -5 + Partials 237 236 -1 ``` | [Files](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | | |---|---|---| | [bridges/otelzap/core.go](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719?src=pr&el=tree&filepath=bridges%2Fotelzap%2Fcore.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-YnJpZGdlcy9vdGVsemFwL2NvcmUuZ28=) | `90.0% <100.0%> (+<0.1%)` | :arrow_up: | | [bridges/otelzap/encoder.go](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719?src=pr&el=tree&filepath=bridges%2Fotelzap%2Fencoder.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-YnJpZGdlcy9vdGVsemFwL2VuY29kZXIuZ28=) | `87.3% <100.0%> (+1.4%)` | :arrow_up: | ... and [13 files with indirect coverage changes](https://app.codecov.io/gh/open-telemetry/opentelemetry-go-contrib/pull/5719/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)
pellared commented 4 weeks ago

Please make sure to add benchmarks to the PRs which are meant to improve performance. You can also consider creating separate PRs that would contain just benchmarks.

pellared commented 6 days ago

Please use https://pkg.go.dev/golang.org/x/perf/cmd/benchstat to show the performance improvement (between main and this branch) in the PR description.