Skip to content

[attraction] reduce allocations in HASH action#45211

Merged
atoulme merged 4 commits intoopen-telemetry:mainfrom
constanca-m:atraction-hash
Jan 24, 2026
Merged

[attraction] reduce allocations in HASH action#45211
atoulme merged 4 commits intoopen-telemetry:mainfrom
constanca-m:atraction-hash

Conversation

@constanca-m
Copy link
Contributor

@constanca-m constanca-m commented Jan 2, 2026

Description

Improves performance on hashing. See:

goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
                                │ current code │               this PR                │
                                │    sec/op    │    sec/op     vs base                │
AttrProc_HashRegex/N20_R10-16      2.578µ ± 1%   2.354µ ± 14%   -8.69% (p=0.000 n=10)
AttrProc_HashRegex/N20_R50-16      5.763µ ± 2%   4.497µ ±  6%  -21.97% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R10-16    191.7µ ± 5%   145.8µ ±  2%  -23.98% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R50-16    530.6µ ± 6%   409.3µ ± 12%  -22.87% (p=0.000 n=10)
geomean                            35.06µ        28.19µ        -19.60%

                                │ current code  │               this PR                │
                                │     B/op      │     B/op      vs base                │
AttrProc_HashRegex/N20_R10-16        353.0 ± 0%     161.0 ± 0%  -54.39% (p=0.000 n=10)
AttrProc_HashRegex/N20_R50-16       1762.0 ± 0%     801.0 ± 0%  -54.54% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R10-16    34.47Ki ± 0%   15.70Ki ± 0%  -54.45% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R50-16   172.08Ki ± 0%   78.25Ki ± 0%  -54.53% (p=0.000 n=10)
geomean                            7.702Ki        3.506Ki       -54.48%

                                │ current code │               this PR               │
                                │  allocs/op   │  allocs/op   vs base                │
AttrProc_HashRegex/N20_R10-16       8.000 ± 0%    4.000 ± 0%  -50.00% (p=0.000 n=10)
AttrProc_HashRegex/N20_R50-16       40.00 ± 0%    20.00 ± 0%  -50.00% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R10-16     800.0 ± 0%    400.0 ± 0%  -50.00% (p=0.000 n=10)
AttrProc_HashRegex/N2000_R50-16    4.000k ± 0%   2.000k ± 0%  -50.00% (p=0.000 n=10)
geomean                             178.9         89.44       -50.00%

Testing

I added a unit test to make sure there were no breaking changes.

@constanca-m constanca-m requested a review from a team as a code owner January 2, 2026 10:38
@constanca-m constanca-m requested a review from mwear January 2, 2026 10:38
Comment on lines +79 to +83
attrs := newMap(b, tc.totalAttrs, tc.secretRatio)

b.ReportAllocs()
for b.Loop() {
ap.Process(b.Context(), nil, attrs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be something like this?

  template := newMap(b, tc.totalAttrs, tc.secretRatio)

  b.ReportAllocs()
  for b.Loop() {
      attrs := pcommon.NewMap()
      template.CopyTo(attrs)
      ap.Process(b.Context(), nil, attrs)
  }

I guess the loop is iterating over the same attrs, so we are just doing real work on the first iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes 🤦 I have updated the issue description with the new results. All good now?

@constanca-m
Copy link
Contributor Author

Hey @iblancasa , can you take a look at this again? Also @codeboten since you are assigned

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@atoulme atoulme merged commit e83f399 into open-telemetry:main Jan 24, 2026
191 checks passed
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.

4 participants