Windows build

[chris-allan]

NOTE: Currently a draft as I have been opinionated on some refactoring decisions; at @glencoesoftware we have some TileDB experience but we are not experts. There are also I believe some bugs I found which I am happy to split out into one or more separate PRs.

This PR contains a series of changes made in the pursuit of having a working TileDB native build and Java runtime environment available for Windows. Detailed below are a few of the specific commits that I think either bear further discussion or are IMHO just important details that I came across will working through this refactor. Fundamentally the changes can be split into four categories:

1. Making native build process to be cross platform
2. Dealing with differences in paths, path separators and delimiters between Windows and UNIX platforms
3. Fixing test cases that were not idempotent and thus extremely problematic to run on a platform like Windows where file-locking is very different than UNIX
4. Adjusting the Java runtime environment to be cross platform

On the native build side, dependency on make has been removed entirely in favour of cmake along with the install phase. Pickup of native artifacts is now done based on output build location.

I would recommend looking at the following commits for decisions I made getting the native side building:

• 27dbcf4
• 58d4a9d

While getting that building I noted that currently the Intel Thread Building Blocks library is not being built statically on Windows with TileDB. This appears a deliberate choice:

...
  CMake Warning at CMakeLists.txt:113 (message):
    Static TBB build not supported on Windows.  Continuing with
    TILEDB_TBB_SHARED=ON.
...

(https://ci.appveyor.com/project/gs-jenkins/tiledb-java/builds/33269697#L65)

This fact has a dramatic influence on some of the Java runtime environment changes I have made. If TBB could be built statically with TileDB some of the changes would be unnecessary.

There appears to be a double free in the JNI bindings when dealing with native array retrieval. The JVM would segfault while running the test cases without the following change:

• 11eb8e9

I'm honestly not sure why the above code works when run on Linux but it certainly appears to. At best there is a memory leak with the current implementation.

I have tried very hard to make no changes to the Java API but some might be warranted. The changes to the test cases are the bulk of the diff and I tried to keep the spirit of each as much as possible while refactoring. The only ones that were problematic were ArrayTest.testArrayOpenAt() and ArrayTest.testArrayOpenAtEncrypted() which would fail repeatedly on CI. I could not reproduce the failures locally but someone will want to look at the following changes to make sure the test still tests what it was originally intended to test:

• 4a676a4
• 063a1fb
• 1cafcf4

I can only guess that there are some timing/system clock differences that were exposed by the AppVeyor running environment.

The changes made to NativeLibLoader are close to a complete rewrite but I really didn't have another option. On Windows it is also likely that temporary files will get left around due to the dynamic linking interdependence between tbb.dll, tiledb.dll and tiledbjni.dll. There is likely significant discussion to be had. Notably the lack of any log factory in the library makes debugging of native library loading issues extremely difficult currently. Also something to consider.

Through the course of trying to get various things to work the following was also done:

1. Upgrade to Gradle 6.4.1
2. Upgrade to junit 4.12
3. Allow the cmake generator and architecture to be configured through Gradle

We have been using "Visual Studio 2019" on AppVeyor for the CI environment and I have been running locally with "Visual Studio 2017". The most recent, as of initial writing, passing build can be examined here:

• https://ci.appveyor.com/project/gs-jenkins/tiledb-java/builds/33269697

Artifacts are also available from that build for testing by anyone who is interested:

• https://ci.appveyor.com/project/gs-jenkins/tiledb-java/builds/33269697/artifacts

With JDK8+ just grabbing the class JAR from the above link and running java -cp path\totiledb-java-0.2.7-SNAPSHOT.jar examples.io.tiledb.java.api.TileDBVersion should be a good smoke test. The output should be something similar to the following:

PS F:\TileDB-Java> java -cp build\libs\tiledb-java-0.2.7-SNAPSHOT.jar examples.io.tiledb.java.api.TileDBVersion
TileDB v2.0.0

[Shelnutt2]

@chris-allan Thank you for the PR, Windows support will be a great addition to the Java API.

@gsvic and I will take look through the changes and post comments.

A couple general notes to start with:

On logging for the native lib loader, we should definitely add something here. We added logging in our TileDB-VCF project, so we can do something similar in this repo. That can be a followup PR, that @gsvic can take care of.

For CI we've been transitioning all of our repositories to Azure pipelines, we'll go ahead and do this repo next. That will give us windows CI support. No need to do anything in this PR, we'll confirm the unit tests locally, and add the CI for windows in a followup.

Also in a follow up we can look at adding windows libraries to the maven central jar. Ideally it'd have the linux, osx and windows libraries for a cross platform jar from maven. This will be easier with azure pipelines for the artifacts from each platform.

[chris-allan]

Thanks, @Shelnutt2.

Again with the weird test failures, this time on Travis, but only on ArrayTest.testArrayOpenAt():

• https://travis-ci.org/github/TileDB-Inc/TileDB-Java/jobs/693939457

Maybe I've broken something or disrupted some fundamental timing.

👍 from me on going to Azure Pipelines, GitHub Actions or similar. We have several native libraries that we are involved in packaging, including https://github.com/glencoesoftware/jxrlib (Travis for macOS and Linux; AppVeyor for Windows), with a variety of different build strategies. The native library loader is factored out here and used for most of them:

• https://github.com/SciJava/native-lib-loader

For logging JUL is certainly an option. I would personally suggest slf4j but that is possibly dragging in more dependencies than you want. Much of the SciJava community will have it on their CLASSPATH already though.

[Shelnutt2]

Did you find that we needed to set the maxHeapSize in your environment?

[chris-allan]

I did, but that was before I refactored a lot of the resource handling into try-with-resources blocks and when I had a lot of tests failing. I'll push a commit without and we'll see if that passes.

[chris-allan]

Without maxHeapSize = '1G', fails with an OOM:

• https://ci.appveyor.com/project/gs-jenkins/tiledb-java#L751

I can roll a40e3ef back or we can try something else if you'd like.

[Shelnutt2]

On the native build side, dependency on make has been removed entirely in favour of cmake along with the install phase. Pickup of native artifacts is now done based on output build location.

Thanks using cmake is a good change for the build phase.

I would recommend looking at the following commits for decisions I made getting the native side building:

* [27dbcf4](https://github.com/TileDB-Inc/TileDB-Java/commit/27dbcf40381ea984ccd4d0305d5c383566981456)

* [58d4a9d](https://github.com/TileDB-Inc/TileDB-Java/commit/58d4a9da170383ff2250ccbe1f66e3dd834c3cb3)

While getting that building I noted that currently the Intel Thread Building Blocks library is not being built statically on Windows with TileDB. This appears a deliberate choice:

...
  CMake Warning at CMakeLists.txt:113 (message):
    Static TBB build not supported on Windows.  Continuing with
    TILEDB_TBB_SHARED=ON.
...

(https://ci.appveyor.com/project/gs-jenkins/tiledb-java/builds/33269697#L65)

This fact has a dramatic influence on some of the Java runtime environment changes I have made. If TBB could be built statically with TileDB some of the changes would be unnecessary.

Unfortunately this is a limitation of TBB. Intel recommends shared library linking everywhere, but they do provide a posix compatible build of a static. Windows is not supported for this, so we must use the shared lib.

There appears to be a double free in the JNI bindings when dealing with native array retrieval. The JVM would segfault while running the test cases without the following change:

* [11eb8e9](https://github.com/TileDB-Inc/TileDB-Java/commit/11eb8e9b2b6b555135e1580ad10f6e64772134d0)

I'm honestly not sure why the above code works when run on Linux but it certainly appears to. At best there is a memory leak with the current implementation.

I agree, this code was wrong. Thanks for catching it.

I have tried very hard to make no changes to the Java API but some might be warranted. The changes to the test cases are the bulk of the diff and I tried to keep the spirit of each as much as possible while refactoring. The only ones that were problematic were ArrayTest.testArrayOpenAt() and ArrayTest.testArrayOpenAtEncrypted() which would fail repeatedly on CI. I could not reproduce the failures locally but someone will want to look at the following changes to make sure the test still tests what it was originally intended to test:

* [4a676a4](https://github.com/TileDB-Inc/TileDB-Java/commit/4a676a4cc1dcfa6ad0988095c42ce7395a0ac05c)

* [063a1fb](https://github.com/TileDB-Inc/TileDB-Java/commit/063a1fb555a13de20875f70b5d6dd89c3ce2c3c5)

* [1cafcf4](https://github.com/TileDB-Inc/TileDB-Java/commit/1cafcf412de8937292e62669d19f823ccf206930)

I can only guess that there are some timing/system clock differences that were exposed by the AppVeyor running environment.

The changes made to NativeLibLoader are close to a complete rewrite but I really didn't have another option. On Windows it is also likely that temporary files will get left around due to the dynamic linking interdependence between tbb.dll, tiledb.dll and tiledbjni.dll. There is likely significant discussion to be had. Notably the lack of any log factory in the library makes debugging of native library loading issues extremely difficult currently. Also something to consider.

I see you've added the addShutdownHook for removing the libraries from the temp folder. Is the risk of these files being left behind due to a potential crash in which the shutdown hook doesn't get called?

Thanks, @Shelnutt2.

Again with the weird test failures, this time on Travis, but only on ArrayTest.testArrayOpenAt():

* https://travis-ci.org/github/TileDB-Inc/TileDB-Java/jobs/693939457

Maybe I've broken something or disrupted some fundamental timing.

+1 from me on going to Azure Pipelines, GitHub Actions or similar. We have several native libraries that we are involved in packaging, including https://github.com/glencoesoftware/jxrlib (Travis for macOS and Linux; AppVeyor for Windows), with a variety of different build strategies. The native library loader is factored out here and used for most of them:

* https://github.com/SciJava/native-lib-loader

For logging JUL is certainly an option. I would personally suggest slf4j but that is possibly dragging in more dependencies than you want. Much of the SciJava community will have it on their CLASSPATH already though.

Thanks for pointing out the native-lib-loader package. We'll evaluate switching to that package, they seem to have logging already setup and its used in a variety of packages so its well tested.

[chris-allan]

Unfortunately this is a limitation of TBB. Intel recommends shared library linking everywhere, but they do provide a posix compatible build of a static. Windows is not supported for this, so we must use the shared lib.

Understood,, bummer. Guess we just have to accept the wrapping will be more complicated in Java then.

I see you've added the addShutdownHook for removing the libraries from the temp folder. Is the risk of these files being left behind due to a potential crash in which the shutdown hook doesn't get called?

No. In the previous implementation File.deleteOnExit() was being used; behind the scenes this adds a shutdown hook for each invocation. Thus the implementations are functionally equivalent and both are definitely subject to crashes, JVM halt, etc. creating situations where the shutdown hook would not be called.

The real issue is that, I assume because of the dynamic linking, the interdependence between tbb.dll, tiledb.dll, and tiledbjni.dll creates a scenario where all the files are locked even when the JVM shutdown hook is run. This results in each deletion silently failing (a "feature" of File.delete()). If we were using Files.delete(), which throws exceptions if deletes fail, you would see each of the DLL deletions fail with an AccessDeniedException.

Thanks for pointing out the native-lib-loader package. We'll evaluate switching to that package, they seem to have logging already setup and its used in a variety of packages so its well tested.

👍 I think as long as we have the lack of a statically linked TBB something hand rolled is likely to be required unfortunately.

[Shelnutt2]

@chris-allan I've tested this on windows and it looks great. Looks like appveyor is failing with the heap size, feel free to add that line back, 1GB is reasonable and used just for the tests. Once you add that back and CI passes I'll go ahead and merge this.

Thank you for the contribution, as you continue to explore and use TileDB please let us know if you run into any issue or have questions. You can post issues here, in our forums or shoot us an email at [email protected]. Also feel free to suggest any improvements you'd like to TileDB-Java. we've mostly been using TileDB-Java for our Spark and PrestoDB connectors so feedback on your usage is welcome.

[chris-allan]

Thanks, @Shelnutt2. After completing our first major project with TileDB I have started collating some of our experiences. The first bit of feedback, on the ergonomics of working with unsigned types in TileDB-Java, I have written up on #177.