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

Strange exported function names in official Maven build for Windows #12226

Closed
theolivenbaum opened this issue Jan 11, 2024 · 16 comments · May be fixed by #12246
Closed

Strange exported function names in official Maven build for Windows #12226

theolivenbaum opened this issue Jan 11, 2024 · 16 comments · May be fixed by #12246

Comments

@theolivenbaum
Copy link
Contributor

I'm trying to switch our C# RockDB binding to use the official JNI builds, and that's fine for macOS and Linux, but weirdly, the function names exported for the Windows .dll files have some weird prefixes on them. When we try to use this library instead of the custom built .dll, it fails because it can't find any of the functions exported.

Expected behavior

Exported function names don't have added JAVA-related prefixes:

image

Actual behavior

Method names have strange prefixes, like Java_org_rocksdb_AbstractEventListener_...
image

Steps to reproduce the behavior

Download the latest Windows build from Maven, open it as a zip and use DLL Export or other tools to view exported function names.

@theolivenbaum theolivenbaum changed the title Strange exported function names in official jni for Windows Strange exported function names in official Maven build for Windows Jan 11, 2024
@adamretter
Copy link
Collaborator

adamretter commented Jan 11, 2024

@theolivenbaum You are looking at the names of the JNI interface functions. These exist on all platforms in RocksJava binaries alongside the non-JNI functions. You should also be able to find the non-JNI (i.e. pure C++) API functions too.

@rubo
Copy link

rubo commented Jan 11, 2024

Looks like the non-JNI functions are not exported for Windows.

@adamretter
Copy link
Collaborator

@rubo if that is the case, then we may need to tweak the CMake build that is used for Windows. @rhubner any ideas on this one?

@rhubner
Copy link
Contributor

rhubner commented Jan 11, 2024

@adamretter Nothing in my sleeve at the moment, but at least I can replicate what @theolivenbaum mentioned.

@rubo
Copy link

rubo commented Jan 29, 2024

Any chance this will be addressed in the next release?

@adamretter
Copy link
Collaborator

@rubo It depends on timing. We have a draft PR that @rhubner has prepared but it is still far from ideal (as it currently means having to list all files to export symbols from). We need to find a better approach in that PR - if you have ideas?

@theolivenbaum
Copy link
Contributor Author

theolivenbaum commented Jan 30, 2024

@adamretter any ideas why this is different than on the Linux/macOS builds?

@rhubner
Copy link
Contributor

rhubner commented Jan 30, 2024

@theolivenbaum

It's a limitation of linker on windows. By default it only exports from the library you are compiling, in our case rocksdbjni. To export symbols from librocksdb you need to provide def file where all symbols are explicitly mentioned. Unfortunately there is no simple solution to generate these symbols automatically.

I have two ideas how to fix this, neither is best:

  1. First compile rocksdb.dll and then export all symbols and with some PowerShell magic prepare def file.
  2. Change build that rocksdb and rocksjni will be compiled together. At the moment we first compile rocksdb to .lib and then we compile rocksdbjni.dll and link together.

Radek

@theolivenbaum
Copy link
Contributor Author

Wouldn't it be possible to compile just the rocksdb library without the Java methods instead and provide it as part of the package?

@rhubner
Copy link
Contributor

rhubner commented Jan 30, 2024

@theolivenbaum
Yes, but then it will double the size of the distribution package(.jar) and code will be there twice. Once as librocksdb.dll and second as librocksdbjni.dll

@theolivenbaum
Copy link
Contributor Author

theolivenbaum commented Jan 30, 2024

Not sure if relevant in this case, but just found this: https://cmake.org/cmake/help/latest/prop_tgt/WINDOWS_EXPORT_ALL_SYMBOLS.html

This can be used to export symbols from a .dll that are not in any of its object files 
but are added by the linker from dependencies (e.g. msvcrt.lib).

More info here too.

@theolivenbaum
Copy link
Contributor Author

Another question about the JNI builds: are they built with or without the jemalloc / tcmalloc flags?

@rhubner
Copy link
Contributor

rhubner commented Feb 6, 2024

@theolivenbaum

Another question about the JNI builds: are they built with or without the jemalloc / tcmalloc flags?

Without jemalloc/tcmalloc. We are not at this time planning to support jemalloc/tcmalloc in the Windows builds of RocksJava that are released. If you need that, you will have to build your own binaries. The binaries we release aim to be as portable as possible and have minimal optimisations enabled.

@theolivenbaum
Copy link
Contributor Author

theolivenbaum commented Feb 6, 2024

Perfect, just wanted to confirm that! I've added on our end a separate build for Linux (on top of using the JNI r)eleases) so we can have a librocksd-jemalloc.so, because we very often hit this in production. But that is only affecting Linux so far (and it's on an old RHEL release, so maybe not an issue in newer malloc implementations).

@rhubner
Copy link
Contributor

rhubner commented Mar 26, 2024

Hello @theolivenbaum (cc @rubo)

I just updated #12246 Now we should be able to compile and release .dll on windows with rocksdb functions. The solution is not perfect, but it works. Unfortunately WINDOWS_EXPORT_ALL_SYMBOLS didn't work.

It will be great if you can check the PR and test if the output .dll works for you.

Radek

@rubo
Copy link

rubo commented May 15, 2024

I think we can consider this issue resolved. Thank you for the fix.

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

Successfully merging a pull request may close this issue.

4 participants