-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat: initial FFI crate + Java bindings #2516
Conversation
CodSpeed Performance ReportMerging #2516 will degrade performances by 26.52%Comparing Summary
Benchmarks breakdown
|
@@ -0,0 +1,251 @@ | |||
#!/bin/sh |
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.
Hello darkness my old friend
I tried
|
It sounds like you need to regenerate the TPC-H files? So maybe there was a break sometime in the past day or so from when I branched |
77d5f2a
to
e6a5d1c
Compare
I've rebased to incorporate any format breaks from the past 24 hours |
import com.sun.jna.Pointer; | ||
import com.sun.jna.PointerType; | ||
|
||
/** |
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.
You'll note that every type generally has an associated XYZ_free()
function. My follow-on goal is to add nicer Java builders/wrappers that are AutoCloseable with @MustClose
to make it harder to accidentally leak these things.
My error is gone with latest push! |
f209ae7
to
055da4f
Compare
Iceberg actually builds to Java 11 so had to bring target language level here down from 17 -> 11. Honestly didn't change much, only thing I was using was the switch expressions |
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.
We can use AutoCloseable already to avoid further breaks, @MustClose and wrappers can come later imho. AutoCloseable is the correct interace to implement in java after java 7
This PR adds a new
vortex-ffi
crate, which builds a shared library that is then consumed by thevortex-jni
java project. This PR previously included some initial Spark bindings but to prevent this getting any larger I've decided to offload that to another PR.Some things to note
./gradlew test
from the java directory to run the basic scan test.FLUPs: