-
Notifications
You must be signed in to change notification settings - Fork 87
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
cuvs-java: Rework the api to be Java 21 friendly #628
cuvs-java: Rework the api to be Java 21 friendly #628
Conversation
/ok to test |
/ok to test |
/ok to test |
/ok to test |
/ok to test |
@ChrisHegarty @chatman and I tried running this branch locally and came across the following. Please help. Thanks.
|
From the gist:
The JVM is crashing. Please take a look at the
That issue arises with running with the classes on the class path rather than the jar file. Lemme see if I can fix it. |
The examples now compiles and runs for me. There was an issue with the multi-release attribute missing from the jar. Please pull the latest branch and confirm that all is good. Both the crash and examples should be fixed. |
/ok to test |
I tried the example project:
Any ideas, please? Edit: before doing this, I did |
@chatman Please confirm that you are on the latest commit in the branch, since that error should be fixed.
|
@chatman Ok, I see that ur running the example in a different way. Lemme try what ur doing. |
Yes, that I tried that commit only. Didn't work.
…On Wed, 5 Feb, 2025, 4:01 pm Chris Hegarty, ***@***.***> wrote:
@chatman <https://github.com/chatman> Ok, I see that ur running the
example in a different way. Lemme try what ur doing.
—
Reply to this email directly, view it on GitHub
<#628 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDCR5FH3HOCSW4LUXABLCD2OHR6TAVCNFSM6AAAAABWGDWKJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMZWGM2DCNJWGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Please let me know how you were able to run it. |
@chatman Please clean and rebuild ur local maven repository, to ensure that there are no stale artifacts. |
I can confirm the same problem after cleaning maven before building and trying to run again. |
The examples work perfectly now, thanks @ChrisHegarty. |
I also see a crash in my local testing. The crash/issue appears to the same as the crash that I see in the main |
Quick summary of the test crash situation in this branch:
|
Hypothesis on this.
I believe that this PR branch does not see the crash, since it checks the error code from native calls. |
/ok to test |
/merge |
This change reworks the api to allow it to be used with Java 21. The implementation is moved to an internal package, compiled with JDK 22, and packaged as an mrjar. The benefit of this structure is that the api can be used in environments that compile to a minimum of Java 21, but run on more recent JDKs like 22 and 23 - which is exactly what Elasticsearch and Lucene do. In fact, a minimum compilation target of Java 21 is common, since 21, at the time of writing, is the most recent LTS Java release.
The most significant change is that the non-trivial api types are now, for the most part, interfaces. Instance can be created by one of the factory methods, which lookup an spi to find the implementation. If on a release greater than Java 21, then a functioning implementation is returned. Otherwise, a no-op implementation is returned. This is a reasonably standard way for a Java api to behave, and allows the developer to handle the case where the platform does not have a functioning implementation.
This change also refactors the native downcall method handles so that they are static final constants - which optimise better by the JVM. It's also the generally accepted pattern, where the handles are tied to the lifetime of class which effectively mediates access - by virtue of reachability.
Another thing that I added is the ability to programmatically set the temporary directory used for intermediate operations - this is important to how both Lucene and Elasticsearch work - since they commonly only have permission to write to certain parts of the disk.
Additionally,
cuvsGetLastErrorText
.mvn verify
, ormvn integration-test
, ormvn -Dit.test="*Hnsw*" verify
spi
layer. You can see the minimal impact on the tests.