Skip to content

Commit

Permalink
Use encoding/binary to convert hashing seed for better performance (#…
Browse files Browse the repository at this point in the history
…31660)

**Description:**
I was exploring this processor to better understand how it works and
when reading this function it took me a minute to make sure I understood
that it was converting the seed to LittleEndian ordering. I decided to
see if using the provided `encoding/binary` function would show a
performance improvement and it does. I believe this change improves the
readability of this function as well as improves the performance (albeit
likely in a marginal way for overall performance of this processor)

It also doesn't seem like a changelog entry is needed here given this is
a minor refactor.

**Testing:**
I added a benchmark to show improved performance which I ran locally
with:
`go test -bench="Benchmark*" -run=XXX -benchmem -benchtime=5s -count=10`
Then I compared the previous version to mine using `benchstat`
```
goos: darwin
goarch: arm64
pkg: github.com/open-telemetry/opentelemetry-collector-contrib/processor/probabilisticsamplerprocessor
         │ old_enc.out  │             new_enc.out              │
         │    sec/op    │    sec/op     vs base                │
32tob-10   2.5105n ± 1%   0.3138n ± 0%  -87.50% (p=0.000 n=10)

         │ old_enc.out │          new_enc.out           │
         │    B/op     │    B/op     vs base            │
32tob-10    0.000 ± 0%   0.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal

         │ old_enc.out │          new_enc.out           │
         │  allocs/op  │ allocs/op   vs base            │
32tob-10    0.000 ± 0%   0.000 ± 0%  ~ (p=1.000 n=10) ¹
¹ all samples are equal
```
  • Loading branch information
ajgajg1134 authored Mar 12, 2024
1 parent 660c59a commit 628bc9a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
5 changes: 2 additions & 3 deletions processor/probabilisticsamplerprocessor/fnvhasher.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package probabilisticsamplerprocessor // import "github.com/open-telemetry/opentelemetry-collector-contrib/processor/probabilisticsamplerprocessor"

import (
"encoding/binary"
"hash/fnv"
)

Expand All @@ -19,8 +20,6 @@ func computeHash(b []byte, seed uint32) uint32 {
// i32tob converts a seed to a byte array to be used as part of fnv.Write()
func i32tob(val uint32) []byte {
r := make([]byte, 4)
for i := uint32(0); i < 4; i++ {
r[i] = byte((val >> (8 * i)) & 0xff)
}
binary.LittleEndian.PutUint32(r, val)
return r
}
16 changes: 16 additions & 0 deletions processor/probabilisticsamplerprocessor/fnvhasher_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package probabilisticsamplerprocessor

import (
"testing"
)

func BenchmarkSeedConversion(b *testing.B) {
val := uint32(0x3024001) // Just a random 32 bit int
b.ResetTimer()
for i := 0; i < b.N; i++ {
i32tob(val)
}
}

0 comments on commit 628bc9a

Please sign in to comment.