ES|QL deserves a new hash table #98749
Conversation
We've been using `LongHash` and `LongLongHash` which are open addressed,
linear probing hash tables that grow in place. They have served us well,
but we need to add features to them to support all of ES|QL. It turns out
that there've been a lot of advances in the hash space in the ten years
since we wrote these hash tables! And they weren't the most "advanced"
thing back then. This PR creates a new hash table implementation the
borrows significantly from google's Swiss Tables. It's 25% to 49% faster
in microbenchmarks:
```
unique longHash ordinator64
5 7.470 ± 0.033 -> 4.158 ± 0.037 ns/op 45% faster
1000 9.657 ± 0.375 -> 4.907 ± 0.036 ns/op 49% faster
10000 15.505 ± 0.051 -> 11.609 ± 0.062 ns/op 25% faster
100000 20.948 ± 0.112 -> 13.413 ± 0.764 ns/op 35% fsater
1000000 48.507 ± 0.586 -> 36.306 ± 0.296 ns/op 25% faster
```
This also integrates the new table into ES|QL's grouping functions,
though imperfectly at the moment.
| @OutputTimeUnit(TimeUnit.NANOSECONDS) | ||
| @State(Scope.Thread) | ||
| @Fork(1) | ||
| @Fork(value = 1, jvmArgsAppend = { "--enable-preview", "--add-modules", "jdk.incubator.vector" }) |
There was a problem hiding this comment.
I've updated this to work with the new hash, but it doesn't produce the lovely performance numbers - yet. Partly that's because we're not integrating with the hash super well - the vector case needs to consume the array somehow. Or something similar. But that feels like something for another time.
There was a problem hiding this comment.
The other reason this doesn't show the performance bump we expect is because we don't enable all of the other aggregations - and because we don't aggregate much larger groups. Either way, this benchmark is much better at showing the performance of aggs, not the groupings. At least not yet.
| // TODO Discuss moving compileOptions.getCompilerArgs() to use provider api with Gradle team. | ||
| List<String> compilerArgs = compileOptions.getCompilerArgs(); | ||
| compilerArgs.add("-Werror"); | ||
| // compilerArgs.add("-Werror"); NOCOMMIT add me back once we figure out how to not fail compiling with preview features |
There was a problem hiding this comment.
This'd be a huge problem to commit, but I can't figure out a good way around it. If I enable the vector API it'll emit the warning. I think Lucene has some kind of hack for accessing the vector API that I think would fix this. And we'd want to steal that.
| it.properties = doubleProperties | ||
| it.inputFile = blockHashInputFile | ||
| it.outputFile = "org/elasticsearch/compute/aggregation/blockhash/DoubleBlockHash.java" | ||
| } |
There was a problem hiding this comment.
I'm generating these so it's easier to keep them updated. I'll generate some more Ordinators at some point - at 32 and 128 bit one at least. But that's another follow up.
| groups[i] = hashOrdToGroupNullReserved(longHash.add(Double.doubleToLongBits(vector.getDouble(i)))); | ||
| groups[i] = ordinator.add(Double.doubleToLongBits(vector.getDouble(i))); | ||
| } | ||
| return new LongArrayVector(groups, groups.length); |
There was a problem hiding this comment.
We need to use an array style add to make this fast. It should be possible, but I'd like to wait for this in a follow up too.
| * explosion of groups caused by multivalued fields | ||
| */ | ||
| public BlockHash build(List<HashAggregationOperator.GroupSpec> groups, int emitBatchSize) { | ||
| if (groups.size() == 1) { |
There was a problem hiding this comment.
Here's a place where we can plug in detection of the vector API not being available. If we don't have the vector API we could always use PackadVauesBlockHash which we can make sure doesn't use the new classes.
Or, potentially, we could turn off all of ESQL if you don't have the vector API.
|
run elasticsearch-ci/part-2 |
Wow - impressive results. I will have a look at this PR shortly today. |
|
High-level, I love the idea. There is a new-to-Elasticsearch dependency on the Panama Vector API. I think that this is fine, but will clearly need some rework and discussion about how it should be integrated. FWIW, I think that we should adopt a similar strategy as that of what is currently done in Lucene. First, generate and checkin a JDK-version specific api jar containing the Vector API stubs - this can be used to compile against. Second, build the Vector dependent code into the MRJAR versioned section of the jar. Lastly, dynamically load either the SIMD'ized version or a fallback non-SIMD version of the code, at runtime, depending on the JDK runtime version. |
|
So what's the right next step here? I'd love to push this somehow, but am not entirely sure how. |
It's on me. I'll figure out a plan and file a GH issue for it. |
We have optimizations that kick in when aggregating on the following pairs of field types: * `long`, `long` * `keyword`, `long` * `long`, `keyword` These optimizations don't have proper support for `null` valued fields but will grow that after elastic#98749. In the mean time this disables them in a way that prevents them from bit-rotting.
* ESQL: Disable optimizations with bad null handling We have optimizations that kick in when aggregating on the following pairs of field types: * `long`, `long` * `keyword`, `long` * `long`, `keyword` These optimizations don't have proper support for `null` valued fields but will grow that after #98749. In the mean time this disables them in a way that prevents them from bit-rotting. * Update docs/changelog/99434.yaml
|
The GH issue that tracks adding Panama Vector API support, #101314 |
@ChrisHegarty Since this issue was closed, does this mean this issue is unblocked, or whats the way forward? |
|
Going to dump my thoughts here on how we might be able to move this forward. The general idea is to extract out the parts of the implementation - either Additionally, and in the majority of cases, the implementations in simdvec are "optional". There is typically both a scalar and Panama vectorized implementation, retrievable through The code in this particular PR is more complex that than this reference, but by way of straightforward example to help illustrate the above points, one can look at the following PR that extracts out some basic functionality into scalar and vectorized - #135087. Note: simdvec has minimal dependencies and does not depend upon server, so things like Circult breaker, page recycler are not accessible. Though I think that these can abstracted out of the core logic that needs to be vectorized. |
8 participants
We've been using
LongHashandLongLongHashwhich are open addressed, linear probing hash tables that grow in place. They have served us well, but we need to add features to them to support all of ES|QL. It turns out that there've been a lot of advances in the hash space in the ten years since we wrote these hash tables! And they weren't the most "advanced" thing back then. This PR creates a new hash table implementation the borrows significantly from google's Swiss Tables. It's 25% to 49% faster in microbenchmarks:This also integrates the new table into ES|QL's grouping functions, though imperfectly at the moment.