Speedup sum aggregator #120241
Speedup sum aggregator #120241
Conversation
Using an object to track the two double fields is extremely inefficient here since the object can't be removed by escape analysis.
elasticsearchmachine
added
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
In ESQL we tend to generate code for this sort of thing. Though we might still be using these Kahan objects out of habit. Server doesn't have any such precedent so I guess we're out of luck there. |
|
@nik9000 you would probably get the same code out of the JIT if you instantiated that object inline and used its code. The problem here is that the object comes from an outside scope so escape analysis can't do away with it. |
original-brownbear
added
auto-backport
|
Thanks Nik! |
| } | ||
|
|
||
| var compensations = SumAggregator.this.compensations; | ||
| double delta = compensations.get(bucket); |
There was a problem hiding this comment.
Why don't we put this inside the if statement?
There was a problem hiding this comment.
ouch you're right we have another optimization there, was so proud I did away with the redundant store and missed this sec, lets fix it right away.
💚 Backport successful
|
Using an object to track the two double fields is extremely inefficient here since the object can't be removed by escape analysis.
Trivial follow up to elastic#120241, no need for a redundant load though the not-load path is probably very cold anyway, but still better to give the compiler all the inputs it can get.
Trivial follow up to #120241, no need for a redundant load though the not-load path is probably very cold anyway, but still better to give the compiler all the inputs it can get.
Trivial follow up to elastic#120241, no need for a redundant load though the not-load path is probably very cold anyway, but still better to give the compiler all the inputs it can get.
|
Here we go for a bit of drier logic #120436 and speedup the average calculation as well |
Random profiling find :)
Using an object to track the two double fields is extremely inefficient here since the object can't be removed by escape analysis.
We have a couple more like this that could be fixed in a similar way. There is no state to retain in that summation object but since it's injected from the outside the compiler has no way to optimize away the two double stores at the end of the method call (and might not even be able to do away with the intermediary steps).
It's mildly unfortunate admittedly that there isn't really a neat Java way of drying this calculation up as far as I can tell because we need two double returns.