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

[chore] [deltatocumulative]: linear histograms #36486

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sh0rez
Copy link
Member

@sh0rez sh0rez commented Nov 21, 2024

Description

Finishes work started in #35048

That PR only partially introduced a less complex processor architecture by only using it for Sums.
Back then I was not sure of the best way to do it for multiple datatypes, as generics seemed to introduce a lot of complexity regardless of usage.

I since then did of a lot of perf analysis and due to the way Go works (see gcshapes), we do not really gain anything at runtime from using generics, given method calls are still dynamic.

This implementation uses regular Go interfaces and a good old type switch in the hot path (ConsumeMetrics), which lowers mental complexity quite a lot imo.

The value of the new architecture is backed up by the following benchmark:

goos: linux
goarch: arm64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/processor/deltatocumulativeprocessor
                 │ sums.nested │             sums.linear             │
                 │   sec/op    │   sec/op     vs base                │
Processor/sums-8   56.35µ ± 1%   39.99µ ± 1%  -29.04% (p=0.000 n=10)

                 │  sums.nested  │             sums.linear              │
                 │     B/op      │     B/op      vs base                │
Processor/sums-8   11.520Ki ± 0%   3.683Ki ± 0%  -68.03% (p=0.000 n=10)

                 │ sums.nested │            sums.linear             │
                 │  allocs/op  │ allocs/op   vs base                │
Processor/sums-8    365.0 ± 0%   260.0 ± 0%  -28.77% (p=0.000 n=10)

Testing

This is a refactor, existing tests pass unaltered.

Documentation

not needed

expands the linear architecture to do exponential and fixed-width
histograms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants