Skip to content

Improve concurrent performance of exponential histogram measurements#7535

Closed
dashpole wants to merge 1 commit intoopen-telemetry:mainfrom
dashpole:optimize_exphist_ringbuffer
Closed

Improve concurrent performance of exponential histogram measurements#7535
dashpole wants to merge 1 commit intoopen-telemetry:mainfrom
dashpole:optimize_exphist_ringbuffer

Conversation

@dashpole
Copy link
Copy Markdown
Contributor

@dashpole dashpole commented Oct 24, 2025

This builds on #7474 and #7478. It is similar to the histogram implementation, but uses an additional hotColdWaitGroup for downscaling without blocking the hot path, and a "circular" slice for buckets to avoid shifts.

This is not ready for review, and still needs a significant amount of polish. I will return to this after the previous PRs are merged.

                                                                                  │   main.txt   │              hist.txt               │
                                                                                  │    sec/op    │    sec/op     vs base               │
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/0-24      296.4n ± 18%   155.5n ±  4%  -47.54% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/1-24      304.0n ± 16%   152.1n ±  2%  -49.97% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/10-24     332.8n ±  5%   150.4n ±  4%  -54.80% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/0-24    314.7n ±  8%   152.6n ± 16%  -51.51% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/1-24    314.2n ±  8%   158.7n ±  3%  -49.49% (p=0.002 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/10-24   334.3n ±  8%   157.2n ±  3%  -52.96% (p=0.002 n=6)
geomean                                                                             315.8n         154.4n        -51.10%

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 87.86127% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.1%. Comparing base (f57bf14) to head (8871f03).
⚠️ Report is 205 commits behind head on main.

Files with missing lines Patch % Lines
sdk/metric/internal/aggregate/atomic.go 23.3% 23 Missing ⚠️
...metric/internal/aggregate/exponential_histogram.go 93.9% 11 Missing and 8 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7535     +/-   ##
=======================================
- Coverage   86.2%   86.1%   -0.1%     
=======================================
  Files        298     298             
  Lines      21829   22003    +174     
=======================================
+ Hits       18817   18957    +140     
- Misses      2635    2664     +29     
- Partials     377     382      +5     
Files with missing lines Coverage Δ
sdk/metric/internal/aggregate/aggregate.go 100.0% <100.0%> (ø)
...metric/internal/aggregate/exponential_histogram.go 94.8% <93.9%> (-2.6%) ⬇️
sdk/metric/internal/aggregate/atomic.go 75.7% <23.3%> (-12.5%) ⬇️

... and 3 files 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 optimize_exphist_ringbuffer branch 3 times, most recently from a7ee0ea to ee6cf66 Compare December 11, 2025 20:57
@dashpole dashpole force-pushed the optimize_exphist_ringbuffer branch from ee6cf66 to 8871f03 Compare December 11, 2025 21:01
@dashpole
Copy link
Copy Markdown
Contributor Author

I'm going to try to split this into smaller PRs:

  • Migrate to circular buffer to avoid shifts (which we can't do in a lockless manner)
  • Compute count at export time
  • Split scale by positive and negative, and re-join at collect time.

dashpole added a commit that referenced this pull request Dec 16, 2025
Tiny part of
#7535

It doesn't make much of a difference now, since we are bound by lock
contention, but it matters once we move to atomics.

```
                                                                                  │  main.txt   │           defercount.txt           │
                                                                                  │   sec/op    │    sec/op     vs base              │
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/0-24      300.9n ± 7%   282.8n ± 10%       ~ (p=0.394 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/1-24      300.0n ± 8%   307.5n ±  3%       ~ (p=0.394 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialInt64Histogram/Attributes/10-24     301.4n ± 5%   300.7n ±  5%       ~ (p=0.485 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/0-24    286.4n ± 4%   284.8n ±  3%       ~ (p=0.420 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/1-24    303.8n ± 3%   294.3n ±  2%  -3.11% (p=0.006 n=6)
SyncMeasure/NoView/ExemplarsDisabled/ExponentialFloat64Histogram/Attributes/10-24   297.9n ± 4%   291.6n ±  3%  -2.10% (p=0.024 n=6)
geomean                                                                             298.3n        293.5n        -1.62%
```
@MrAlias MrAlias mentioned this pull request Jan 16, 2026
39 tasks
@dashpole dashpole closed this Mar 9, 2026
dashpole added a commit that referenced this pull request Mar 13, 2026
This is the first PR towards a lockless fast-path for the exponential
histogram aggregation. It just replaces use of min, max, sum and counts
with atomic types.

You can see the full set of planned changes in
main...dashpole:opentelemetry-go:lockless_exphist_ai.
The implementation is largely based on
#7535 (which I
implemented by hand), but with help from an AI to break it down into
smaller PRs, and simplify aspects of the design.

Part of #7796
dashpole added a commit that referenced this pull request Mar 17, 2026
Follows #8025

This is the second PR towards a lockless fast-path for the exponential
histogram aggregation. It replaces use of uint64 with atomic.Uint64. It
does not make buckets concurrent-safe. That will come in future PRs.
This is a refactor to make future PRs easier to review since it has a
large diff, but is relatively simple.

The record and measure calls are still guarded by a lock at this point.

You can see the full set of planned changes in
main...dashpole:opentelemetry-go:lockless_exphist_ai.
The implementation is largely based on
#7535 (which I
implemented by hand), but with help from an AI to break it down into
smaller PRs, and simplify aspects of the design.

Part of #7796
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.

1 participant