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

[pkg/pdatautil] Optimize the hashing function for pcommon.Map #27840

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

mdonkers
Copy link
Contributor

Description:

Improve the performance of the MapHash function, mostly by using the xxhash architecture optimized version.

hash.Sum is a 'Go-code' only implementation
xxhash.Sum64 has optimized versions for different architectures
Both result in the exact same hash though.

For the given benchmarks, the gain is > 10%

From main:

goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	47676003	       236.0 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	22551222	       532.3 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	14098969	       893.1 ns/op	      56 B/op	       3 allocs/op

The PR:

goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	59854737	       203.4 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	25609375	       475.0 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	15950144	       753.8 ns/op	      56 B/op	       3 allocs/op

Testing:
(Re-)using the same tests and benchmarks to prove semantics didn't change.

Documentation:
none

@mdonkers mdonkers requested review from dmitryax and a team as code owners October 18, 2023 12:40
@songy23 songy23 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Oct 18, 2023
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution, @mdonkers!

@dmitryax dmitryax changed the title Optimize the hashing function for pcommon.Map [pkg/pdatautil] Optimize the hashing function for pcommon.Map Oct 19, 2023
@dmitryax dmitryax merged commit 762f843 into open-telemetry:main Oct 19, 2023
76 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 19, 2023
martin-majlis-s1 pushed a commit to scalyr/opentelemetry-collector-contrib that referenced this pull request Oct 20, 2023
…-telemetry#27840)

**Description:**

Improve the performance of the `MapHash` function, mostly by using the
xxhash architecture optimized version.

`hash.Sum` is a 'Go-code' only implementation
`xxhash.Sum64` has optimized versions for different architectures
Both result in the exact same hash though.


For the given benchmarks, the gain is > 10%

From `main`:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	47676003	       236.0 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	22551222	       532.3 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	14098969	       893.1 ns/op	      56 B/op	       3 allocs/op
```

The PR:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	59854737	       203.4 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	25609375	       475.0 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	15950144	       753.8 ns/op	      56 B/op	       3 allocs/op
```

**Testing:**
(Re-)using the same tests and benchmarks to prove semantics didn't
change.
sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this pull request Oct 27, 2023
…-telemetry#27840)

**Description:**

Improve the performance of the `MapHash` function, mostly by using the
xxhash architecture optimized version.

`hash.Sum` is a 'Go-code' only implementation
`xxhash.Sum64` has optimized versions for different architectures
Both result in the exact same hash though.


For the given benchmarks, the gain is > 10%

From `main`:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	47676003	       236.0 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	22551222	       532.3 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	14098969	       893.1 ns/op	      56 B/op	       3 allocs/op
```

The PR:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	59854737	       203.4 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	25609375	       475.0 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	15950144	       753.8 ns/op	      56 B/op	       3 allocs/op
```

**Testing:**
(Re-)using the same tests and benchmarks to prove semantics didn't
change.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…-telemetry#27840)

**Description:**

Improve the performance of the `MapHash` function, mostly by using the
xxhash architecture optimized version.

`hash.Sum` is a 'Go-code' only implementation
`xxhash.Sum64` has optimized versions for different architectures
Both result in the exact same hash though.


For the given benchmarks, the gain is > 10%

From `main`:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	47676003	       236.0 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	22551222	       532.3 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	14098969	       893.1 ns/op	      56 B/op	       3 allocs/op
```

The PR:
```
goos: linux
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
BenchmarkMapHashFourItems-16                  	59854737	       203.4 ns/op	      24 B/op	       1 allocs/op
BenchmarkMapHashEightItems-16                 	25609375	       475.0 ns/op	      32 B/op	       2 allocs/op
BenchmarkMapHashWithEmbeddedSliceAndMap-16    	15950144	       753.8 ns/op	      56 B/op	       3 allocs/op
```

**Testing:**
(Re-)using the same tests and benchmarks to prove semantics didn't
change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/pdatautil Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants