Skip to content
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

Export rocksdb fuctions in rocksdbjni.dll #12246

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rhubner
Copy link
Contributor

@rhubner rhubner commented Jan 18, 2024

This PR modifies the CMake build process on Windows to export all RocksDB functions in rocksdbjni.dll.

This allows us to reuse Java builds artifacts in other programming languages without need to manually recompile the .dll for each language.

Closes #12226

Copy link
Collaborator

@adamretter adamretter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @rhubner Could you also send me some short text to add to the documentation on this Wiki page please?

@adamretter adamretter marked this pull request as ready for review June 11, 2024 10:12
@rhubner rhubner force-pushed the eb/rocksjni-export-fuctions branch from 33b8098 to b02ff8c Compare June 11, 2024 10:36
@ajkr
Copy link
Contributor

ajkr commented Jun 12, 2024

I see the appeal of exporting all the functions as librocksdbjni is almost usable as a general-purpose (non-Java) RocksDB library. But I am not sure we want to encourage or guarantee that usage as the 'j' in librocksdbjni was supposed to stand for Java. Is there precedent for using it for non-Java?

@adamretter
Copy link
Collaborator

@ajkr expanding on my comment here - #12226 (comment)

If you extract the .dll or .so files from the Jar file, then you really just have a binary build of RocksDB as a library. It does also include the JNI C++ API, but that is fairly light-weight, and you can use it if you wish, or just ignore it.

The Linux and macOS libraries for RocksJava have always already included the C++ and C API. This change just makes the Windows RocksJava library on-par with the Linux and macOs ones.

@rhubner rhubner force-pushed the eb/rocksjni-export-fuctions branch from b02ff8c to c17df06 Compare June 19, 2024 12:06
@adamretter
Copy link
Collaborator

@ajkr @pdillinger can we get this one merged please?

@rhubner rhubner force-pushed the eb/rocksjni-export-fuctions branch from c17df06 to e0b1612 Compare July 2, 2024 12:46
@ajkr
Copy link
Contributor

ajkr commented Jul 8, 2024

Would providing a Makefile target to build non-Java static lib with statically linked compression libraries solve their problem?

@adamretter
Copy link
Collaborator

@ajkr Possibly. However, regardless, I think we would also like the RocksJava binaries to be equivalent on all platforms (including Windows).

@ajkr
Copy link
Contributor

ajkr commented Jul 9, 2024

The inconsistency feels like a difference in linker behavior that only affects off-label uses of the JAR. This kind of inconsistency seems expected to me. So, I am trying to suggest alternatives that may serve the user better and leave behind fewer hacks/semi-officially supported usages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange exported function names in official Maven build for Windows
4 participants