Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

improve performance of hashing seriesbytag with fnv #1436

Merged
merged 9 commits into from
Aug 21, 2019

Conversation

robert-milan
Copy link
Contributor

@robert-milan robert-milan commented Aug 16, 2019

This PR addresses the poor performance of BySeriesWithTagsFnv. It moves the code from fnv into the method itself which reduces our allocations to 1 and speeds things up quite a bit.

It also adds the tests back into schema, and creates a new test specifically for the old and new methods used to obtain the partition with fnv in this PR.

These benchmarks were not performed in a suitable environment for benchmarking, but I think the difference is large enough to not warrant it.

Benchcmp

benchmark                                    old ns/op     new ns/op     delta
BenchmarkPartitionByOrg-12                   64.7          60.7          -6.18%
BenchmarkPartitionBySeries-12                60.0          56.9          -5.17%
BenchmarkPartitionBySeriesWithTags-12        629           630           +0.16%
BenchmarkPartitionBySeriesWithTagsFnv-12     1508          396           -73.74%

benchmark                                    old allocs     new allocs     delta
BenchmarkPartitionByOrg-12                   3              3              +0.00%
BenchmarkPartitionBySeries-12                2              2              +0.00%
BenchmarkPartitionBySeriesWithTags-12        2              2              +0.00%
BenchmarkPartitionBySeriesWithTagsFnv-12     23             1              -95.65%

benchmark                                    old bytes     new bytes     delta
BenchmarkPartitionByOrg-12                   16            16            +0.00%
BenchmarkPartitionBySeries-12                20            20            +0.00%
BenchmarkPartitionBySeriesWithTags-12        112           112           +0.00%
BenchmarkPartitionBySeriesWithTagsFnv-12     488           32            -93.44%

Original

BenchmarkPartitionByOrg-12                      30000000                64.7 ns/op            16 B/op          3 allocs/op
BenchmarkPartitionBySeries-12                   20000000                60.0 ns/op            20 B/op          2 allocs/op
BenchmarkPartitionBySeriesWithTags-12            2000000               629 ns/op             112 B/op          2 allocs/op
BenchmarkPartitionBySeriesWithTagsFnv-12         1000000              1508 ns/op             488 B/op         23 allocs/op

New

BenchmarkPartitionByOrg-12                      20000000                60.7 ns/op            16 B/op          3 allocs/op
BenchmarkPartitionBySeries-12                   30000000                56.9 ns/op            20 B/op          2 allocs/op
BenchmarkPartitionBySeriesWithTags-12            2000000               630 ns/op             112 B/op          2 allocs/op
BenchmarkPartitionBySeriesWithTagsFnv-12         5000000               396 ns/op              32 B/op          1 allocs/op

schema/partition.go Outdated Show resolved Hide resolved
@robert-milan robert-milan force-pushed the improve-serieswithtags-fnv-hashing branch from 1e88217 to 9203c66 Compare August 19, 2019 09:30
schema/fnv32a.go Outdated Show resolved Hide resolved
Copy link
Contributor

@replay replay left a comment

Choose a reason for hiding this comment

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

LGTM, please check my commits before merging

@robert-milan robert-milan merged commit b6ffeb1 into master Aug 21, 2019
@robert-milan robert-milan deleted the improve-serieswithtags-fnv-hashing branch August 21, 2019 13:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants