Skip to content

Add exemplar reservoir parallel benchmarks#7441

Merged
pellared merged 8 commits intoopen-telemetry:mainfrom
dashpole:exemplar_parallel_benchmarks
Oct 3, 2025
Merged

Add exemplar reservoir parallel benchmarks#7441
pellared merged 8 commits intoopen-telemetry:mainfrom
dashpole:exemplar_parallel_benchmarks

Conversation

@dashpole
Copy link
Copy Markdown
Contributor

@dashpole dashpole commented Oct 1, 2025

This also fixes a bug introduced in #7423, where we were only locking around storage and not around other shared fields (e.g. count). Fixing the bug is required for benchmarks to run properly, but wasn't caught by concurrent safe tests because the SDK does not currently call exemplar methods concurrently.

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric/exemplar
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkFixedSizeReservoirOffer-24    	  498955	       248.4 ns/op	       0 B/op	       0 allocs/op
BenchmarkHistogramReservoirOffer-24    	  478068	       250.1 ns/op	       0 B/op	       0 allocs/op

@dashpole dashpole added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 1, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.7%. Comparing base (dc906d6) to head (df14404).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7441     +/-   ##
=======================================
- Coverage   85.7%   85.7%   -0.1%     
=======================================
  Files        284     284             
  Lines      25001   25006      +5     
=======================================
+ Hits       21437   21441      +4     
- Misses      3185    3186      +1     
  Partials     379     379             
Files with missing lines Coverage Δ
sdk/metric/exemplar/fixed_size_reservoir.go 97.6% <100.0%> (+<0.1%) ⬆️
sdk/metric/exemplar/histogram_reservoir.go 92.3% <100.0%> (+1.8%) ⬆️
sdk/metric/exemplar/storage.go 100.0% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dashpole dashpole force-pushed the exemplar_parallel_benchmarks branch from 89ff225 to 6b0b66e Compare October 1, 2025 19:17
@dashpole dashpole force-pushed the exemplar_parallel_benchmarks branch from 6b0b66e to 1631ed7 Compare October 2, 2025 13:31
@dashpole dashpole force-pushed the exemplar_parallel_benchmarks branch 3 times, most recently from 572360f to 6990fa0 Compare October 2, 2025 13:51
dashpole added a commit that referenced this pull request Oct 2, 2025
See #7441

```
### Errors in sdk/metric/exemplar/fixed_size_reservoir.go

* [403] <https://dl.acm.org/doi/10.1145/198429.198435> | Rejected status code (this depends on your "accept" configuration): Forbidden
```
Comment thread sdk/metric/exemplar/benchmark_test.go Outdated
@pellared pellared merged commit bd1b3da into open-telemetry:main Oct 3, 2025
30 checks passed
@dashpole dashpole deleted the exemplar_parallel_benchmarks branch October 3, 2025 13:17
@MrAlias MrAlias added this to the v1.39.0 milestone Oct 14, 2025
@MrAlias MrAlias mentioned this pull request Jan 16, 2026
39 tasks
dashpole added a commit that referenced this pull request Jan 23, 2026
~Depends on #7441, #7443~

This improves the concurrent performance of the fixed size reservoir's
Offer function by 4x (i.e. 75% reduction). This improves the performance
of Measure() for fixed-size reservoirs by 60% overall.

Accomplish this by:

* using a single atomic for count and next. This assumes that both can
fit in a uint32.
* only use a lock to guard changing `w` and `next` together.

Offer benchmarks:
```
                           │   main.txt   │           fixedsize.txt            │
                           │    sec/op    │   sec/op     vs base               │
FixedSizeReservoirOffer-24   185.25n ± 4%   45.58n ± 1%  -75.40% (p=0.002 n=6)
```

Measure benchmarks:
```
                                                                          │   main.txt   │            fixedsize.txt            │
                                                                          │    sec/op    │    sec/op     vs base               │
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/0-24            175.45n ± 6%   67.01n ±  9%  -61.81% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/1-24            170.25n ± 1%   69.82n ±  6%  -58.99% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64Counter/Attributes/10-24           167.40n ± 2%   64.52n ± 10%  -61.46% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/0-24          173.55n ± 0%   69.17n ± 12%  -60.14% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/1-24          169.50n ± 1%   68.55n ±  5%  -59.56% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64Counter/Attributes/10-24         166.95n ± 1%   65.82n ±  6%  -60.58% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/0-24      168.85n ± 1%   67.99n ± 11%  -59.73% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/1-24      173.50n ± 1%   66.69n ±  2%  -61.56% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Int64UpDownCounter/Attributes/10-24     171.30n ± 5%   67.73n ±  8%  -60.46% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/0-24    168.90n ± 2%   67.69n ±  9%  -59.92% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/1-24    173.35n ± 2%   68.25n ±  9%  -60.63% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsEnabled/Float64UpDownCounter/Attributes/10-24   172.95n ± 2%   70.90n ±  7%  -59.01% (p=0.002 n=6)
geomean                                                                      171.0n        67.83n        -60.33%
```

---------

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants