feat: add simd add/sub/mul expressions#19512
Conversation
FrankChen021
left a comment
There was a problem hiding this comment.
I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.
Reviewed 30 of 30 changed files.
This is an automated review by Codex GPT-5.5
|
This is looking nice, I would like to know any discussion around using JDK based SIMD or native Rust/Cpp based SIMD to understand the improvements, benefits we will get and tradeoff. Ref #19456 |
I think starting from Vector API is a natural first step. |
I guess I view this change and my planned follow-ups as sort of orthogonal to whatever experiments are done re #19456; that said these changes do make a baseline of what our current stuff can do which we can measure any experiments against. |
|
I suspect benchmarks will look better on the JDK 25 compiler (next release). |
| { | ||
| for (int iterations = 0; iterations < numIterations; iterations++) { | ||
| assertEvalsMatch(expr, parsed, makeRandomizedBindings(VECTOR_SIZE, types)); | ||
| for (int vectorSize : VECTOR_SIZES) { |
There was a problem hiding this comment.
Is there a VectorExprResultConsistencyTest for one side or the other being 100% null? Would be a good case to check for.
Description
This PR adds a new config
druid.expressions.useVectorApito allow using the incubating JDK Vector API ((also requires running Druid with flags--add-modules jdk.incubator.vector).The measurements look pretty nice, though I think perhaps some of the simpler expressions I measured might have already been eligible for auto-vectorization given that the numbers look approximately the same for the first few:
I just did add/subtract/multiply implmenetations so far, but there are a few other easy wins that I'll probably add over a few follow-up PRs.