Speed up bit compared with floats or bytes script operations#117199
Merged
elasticsearchmachine merged 6 commits intoelastic:mainfrom Dec 2, 2024
Merged
Speed up bit compared with floats or bytes script operations#117199elasticsearchmachine merged 6 commits intoelastic:mainfrom
elasticsearchmachine merged 6 commits intoelastic:mainfrom
Conversation
benwtrent
added
>enhancement
auto-backport
elasticsearchmachine
added
the
Team:Search Relevance
Collaborator
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
Collaborator
|
Hi @benwtrent, I've created a changelog YAML for you. |
pmpailis
reviewed
Dec 2, 2024
| // now combine the two vectors, summing the byte dimensions where the bit in d is `1` | ||
| for (int i = 0; i < d.length; i++) { | ||
| byte mask = d[i]; | ||
| acc0 += fma(q[i * Byte.SIZE + 0], (mask >> 7) & 1, acc0); |
Contributor
There was a problem hiding this comment.
Overlooked this one initially; but shouldn't the additive component to fma be either 0 or just reset the value of acc0 (i.e. without +=) ? I think that we're making the addition twice in for the accumulators for last bits in lines 80-83.
Member
Author
There was a problem hiding this comment.
@pmpailis you are correct ;) I did a bad copy paste here. Tests have found it.
benwtrent
added
the
auto-merge-without-approval
benwtrent
added a commit
to benwtrent/elasticsearch
that referenced
this pull request
Dec 2, 2024
…#117199) Instead of doing an "if" statement, which doesn't lend itself to vectorization, I switched to expand to the bits and multiply the 1s and 0s. This led to a marginal speed improvement on ARM. I expect that Panama vector could be used here to be even faster, but I didn't want to spend anymore time on this for the time being. ``` Benchmark (dims) Mode Cnt Score Error Units IpBitVectorScorerBenchmark.dotProductByteIfStatement 768 thrpt 5 2.952 ± 0.026 ops/us IpBitVectorScorerBenchmark.dotProductByteUnwrap 768 thrpt 5 4.017 ± 0.068 ops/us IpBitVectorScorerBenchmark.dotProductFloatIfStatement 768 thrpt 5 2.987 ± 0.124 ops/us IpBitVectorScorerBenchmark.dotProductFloatUnwrap 768 thrpt 5 4.726 ± 0.136 ops/us ``` Benchmark I used. https://gist.github.com/benwtrent/b0edb3975d2f03356c1a5ea84c72abc9
elasticsearchmachine
pushed a commit
that referenced
this pull request
Dec 2, 2024
#117841) Instead of doing an "if" statement, which doesn't lend itself to vectorization, I switched to expand to the bits and multiply the 1s and 0s. This led to a marginal speed improvement on ARM. I expect that Panama vector could be used here to be even faster, but I didn't want to spend anymore time on this for the time being. ``` Benchmark (dims) Mode Cnt Score Error Units IpBitVectorScorerBenchmark.dotProductByteIfStatement 768 thrpt 5 2.952 ± 0.026 ops/us IpBitVectorScorerBenchmark.dotProductByteUnwrap 768 thrpt 5 4.017 ± 0.068 ops/us IpBitVectorScorerBenchmark.dotProductFloatIfStatement 768 thrpt 5 2.987 ± 0.124 ops/us IpBitVectorScorerBenchmark.dotProductFloatUnwrap 768 thrpt 5 4.726 ± 0.136 ops/us ``` Benchmark I used. https://gist.github.com/benwtrent/b0edb3975d2f03356c1a5ea84c72abc9
| int acc2 = 0; | ||
| int acc3 = 0; | ||
| // now combine the two vectors, summing the byte dimensions where the bit in d is `1` | ||
| for (int i = 0; i < d.length; i++) { |
There was a problem hiding this comment.
Just a drive-by question here (free to disregard): is this intended to allow vectorization?
Member
Author
There was a problem hiding this comment.
@svilen-mihaylov-db it allows some vectorization via the unrolling, but it definitely isn't as fast as a custom vectorized version that we could provide with the Panama API. This solution isn't as fast as it could be, for sure.
Mainly, I discovered its much faster than the previous if block and so its a step in the right direction :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of doing an "if" statement, which doesn't lend itself to vectorization, I switched to expand to the bits and multiply the 1s and 0s.
This led to a marginal speed improvement on ARM.
I expect that Panama vector could be used here to be even faster, but I didn't want to spend anymore time on this for the time being.
Benchmark I used.
https://gist.github.com/benwtrent/b0edb3975d2f03356c1a5ea84c72abc9