Skip to content

Add support for Linux and Mac ARM platforms#216

Closed
alandavidreyes wants to merge 2 commits intolmdbjava:masterfrom
alandavidreyes:master
Closed

Add support for Linux and Mac ARM platforms#216
alandavidreyes wants to merge 2 commits intolmdbjava:masterfrom
alandavidreyes:master

Conversation

@alandavidreyes
Copy link

This commit is the counterpart change (and depends on) this PR in the native repo. It adds support for two additional versions of the underlying LMDB library, which are cross-compiled to add support for Linux and Mac ARM64 platforms.

@bp-alex
Copy link
Member

bp-alex commented Apr 23, 2023

Thank you to @alandavidreyes for this PR and the corresponding lmdbjava/native#13. Upon taking a look at these PRs I felt adding more platforms via the native project was likely to create a maintenance issue. Indeed I removed previous Android native support for this reason. @JorenSix suggested in a comment about using Zig and I thought your PR represented a timely motivation to take a closer look.

I was pleased to find the Zig approach worked really well and as such I made a branch in my own repo and created PR #217 which provides cross-compilation of five targets using Zig. It also refactors a lot of the related code.

The PR passes all tests (including the Verifier on each native operating system) under GitHub Actions and on my Linux workstation but I was hoping it could be tested on Apple Silicon and Windows by those users with such machines. There may be errors with platform detection for example. Anyhow if you (and perhaps @wardle) could please have a look and let me know what you think it would be good before I merge it.

@wardle
Copy link
Contributor

wardle commented Apr 23, 2023

This looks fantastic. Thank you for all of this work!

I've successfully installed zig 0.10.1 on my Apple M1, and cross-compiled PR #217 using the cross-compile.sh file. I then used maven to install into my local repository. I did have test errors about environment mapsize reached with a basic mvn install:

e.g.

[INFO] Running org.lmdbjava.EnvTest
[ERROR] Tests run: 24, Failures: 1, Errors: 1, Skipped: 0, Time elapsed: 0.551 s <<< FAILURE! - in org.lmdbjava.EnvTest
[ERROR] setMapSize(org.lmdbjava.EnvTest)  Time elapsed: 0.003 s  <<< ERROR!
org.lmdbjava.Env$MapFullException: Environment mapsize reached (-30792)
	at org.lmdbjava.ResultCodeMapper.checkRc(ResultCodeMapper.java:86)
	at org.lmdbjava.Dbi.<init>(Dbi.java:72)
	at org.lmdbjava.Env.openDbi(Env.java:401)
	at org.lmdbjava.Env.openDbi(Env.java:350)

But compiled/packaged/installed using

mvn -B install -Dtest=VerifierTest -DverificationSeconds=10

instead as per the GitHub actions. I'll see if I have the same issue on Linux later on.

In any case, I then switched to using lmdbjava 0.9.0-SNAPSHOT in my own application and have successfully run my own test suite, which obviously uses lmdbjava.

➜  hermes git:(main) ✗ clj -M:test

Running tests in #{"test"}

Testing com.eldrix.hermes.api-test

Testing com.eldrix.hermes.cmd-test

Testing com.eldrix.hermes.core-test

Testing com.eldrix.hermes.ecl-test

Testing com.eldrix.hermes.graph-test

Testing com.eldrix.hermes.scg-test

Testing com.eldrix.hermes.search-test

Testing com.eldrix.hermes.ser-test

Testing com.eldrix.hermes.server-test

Testing com.eldrix.hermes.snomed-test

Testing com.eldrix.hermes.store-test

Testing com.eldrix.hermes.synth-test

Testing com.eldrix.hermes.verhoeff-test

Ran 82 tests containing 307060 assertions.
0 failures, 0 errors.

which would imply that the native arm64 lmdb library is working well.

I then built an uberjar of my own application using 0.9.0-SNAPSHOT, and have successfully run on Mac OS X M1 (ARM), Linux x86 and Windows x86 without having to install a native library manually.

So, in summary, v0.9.0-SNAPSHOT appears to work on M1 Mac, Linux x86 and Windows x86.


Out of interest, I also have the application installed on FreeBSD... so I took the liberty of testing there. Previously, with 0.8.3 this worked without needing to set a path manually, as the lmdb library was somehow picked up automatically [obviously after I manually installed it using the FreeBSD package manager).

But with 0.9.0-SNAPSHOT:

2023-04-23 10:31:07,760 [main] ERROR com.eldrix.hermes.cmd.core - Uncaught exception on "main"
java.lang.ExceptionInInitializerError: null
	at org.lmdbjava.Library.<clinit>(Library.java:75)
	at org.lmdbjava.Env$Builder.open(Env.java:580)
	at org.lmdbjava.Env$Builder.open(Env.java:606)
	at com.eldrix.hermes.impl.lmdb$open_STAR_.invokeStatic(lmdb.clj:94)
	at com.eldrix.hermes.impl.lmdb$open_store.invokeStatic(lmdb.clj:123)
	at com.eldrix.hermes.impl.lmdb$open_store.invoke(lmdb.clj:123)
	at com.eldrix.hermes.impl.lmdb$open_store.invokeStatic(lmdb.clj:125)
	at com.eldrix.hermes.impl.store$open_store.invokeStatic(store.clj:49)
	at com.eldrix.hermes.core$open.invokeStatic(core.clj:1004)
	at com.eldrix.hermes.core$status.invokeStatic(core.clj:1138)
	at com.eldrix.hermes.cmd.core$status.invokeStatic(core.clj:67)
	at com.eldrix.hermes.cmd.core$status.invoke(core.clj:66)
	at com.eldrix.hermes.cmd.core$invoke_command.invokeStatic(core.clj:124)
	at com.eldrix.hermes.cmd.core$_main.invokeStatic(core.clj:152)
	at com.eldrix.hermes.cmd.core$_main.doInvoke(core.clj:133)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at com.eldrix.hermes.cmd.core.main(Unknown Source)
Caused by: java.lang.UnsupportedOperationException: Unsupported os.name (please set system property lmdbjava.native.lib to the path of an external LMDB native library or property lmdbjava.embedded.lib to the name of an LmdbJava embedded library; os.arch='amd64' os.name='FreeBSD')
	at org.lmdbjava.TargetName.resolveOs(TargetName.java:149)
	at org.lmdbjava.TargetName.resolveFilename(TargetName.java:98)
	at org.lmdbjava.TargetName.<clinit>(TargetName.java:74)
	... 17 common frames omitted

As an aside, I can confirm that setting the flag lmdbjava.native.lib works on FreeBSD:

ec2-user@freebsd:~ $ java -Dlmdbjava.native.lib=/usr/local/lib/liblmdb.so -jar hermes-1.2.1147.jar --db Dev/hermes/snome
d.db status

@bp-alex
Copy link
Member

bp-alex commented Apr 24, 2023

Thanks for testing this, @wardle. I have since merged PR #217.

The FreeBSD probably worked as it used to fall back to the default LMDB library name if it couldn't extract. I feel it's better what we're now doing, namely requiring an explicit lmdbjava.native.lib property.

I wonder why the Apple Silicon test gave the org.lmdbjava.Env$MapFullException: Environment mapsize reached (-30792). I've seen this occasionally over the years where tests haven't closed the Env and tidying that up resolved it. Still I don't think it's worth worrying about at present as it's unlikely related to the Zig change and we should address it as a separate issue if it persists.

I am closing this PR and the corresponding native project PR given PR 217 provides the same capability (ie Apple Silicon native library and consistent naming conventions for all native libraries).

@bp-alex bp-alex closed this Apr 24, 2023
@JorenSix
Copy link

Thanks for all the work and for taking the Zig approach suggestion!

I have seen a similar Unit tests fail with similar messages as the Environment mapsize reached. I think the culprit is the 16kB filesystem block sizes which is the default on Apple Silicon, but not on other systems. So I think everything works fine but the unit tests are not all prepared to handle this case?

@bp-alex
Copy link
Member

bp-alex commented Apr 25, 2023

I have seen a similar Unit tests fail with similar messages as the Environment mapsize reached. I think the culprit is the 16kB filesystem block sizes which is the default on Apple Silicon, but not on other systems.

I'm really not in a position to explore this as I don't have an Apple machine. There is an old PR #165 which includes some treatment related to page sizes and "Environment mapsize reached (-30792)" errors. For example:

https://github.com/lmdbjava/lmdbjava/pull/165/files#diff-1041f9d7e33e13b0a27128193a67d62eb98eea6c97e713e96126769eb4e4e8b1L128

I wonder if someone with an Apple might be able to comment, try it out, or update the PR (or raise a new one)? I am happy to apply changes that fix this of course.

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 this pull request may close these issues.

4 participants