-
Notifications
You must be signed in to change notification settings - Fork 323
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reimplement Column.truncate, .ceil, and .floor as vectorized Java ops #6941
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me, the overall 'framework' of the change is good.
However, I think we should avoid boxing and here it is certainly possible. Please create 2 classes UnaryLongToLongOp
and UnaryDoubleToLongOp
. You can essentially copy-paste the UnaryIntegerOp
and just replace the generic T
with primitive double
/long
. This is a pain in Java that generics don't play with primitives, but it's sometthing we cannot do about and while I hate such copy pasting, it's the only way in Java (apart from writing custom code generators - we may come there but at this moment it would be an overkill IMO) to ensure no boxing occurs.
I appreciate we are finally getting benchmarks for operations like this. Good to see the performance difference - it seems it's quite significant. This may indicate that we may need to rethink #6292 and how I implemented #6860 - since for custom types Enso is more efficient than Java-to-Enso callbacks, but apparently with primitives we are still far behind (but of course we just need separate benchmarks for the MultiValueKey, which I will create when working on #6292 - but it's good to see some first comparative results to get an idea what is roughly the difference between operations in pure Enso vs Java).
std-bits/table/src/main/java/org/enso/table/data/column/storage/DoubleStorage.java
Outdated
Show resolved
Hide resolved
Looks all good 🎉 |
Pull Request Description
Reimplement these in Java.
Benchmarks:
Before:
Column.truncate floats average: 124.4ms
Column.ceil floats average: 121.47ms
Column.floor floats average: 120.18ms
Column.truncate ints average: 124.78ms
Column.ceil ints average: 120.41ms
Column.floor ints average: 102.35ms
After (boxed):
Column.truncate floats average: 3.75ms
Column.ceil floats average: 2.25ms
Column.floor floats average: 1.89ms
Column.truncate ints average: 2ms
Column.ceil ints average: 1.77ms
Column.floor ints average: 1.74ms
After (unboxed):
Column.truncate floats average: 3.32ms
Column.ceil floats average: 2.15ms
Column.floor floats average: 1.69ms
Column.truncate ints average: 1.74ms
Column.ceil ints average: 1.61ms
Column.floor ints average: 1.99ms
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.