java-fpe
java-fpe copied to clipboard
Use `libj.math.BigInt` for arithmetic
Java's built-in BigInteger class is a solid platform for building out programs that require the use of arbitrarily-sized numbers. However, when it comes to performance optimization, BigInteger can become problematic for the following reasons:
- While it has been optimized over the years, it does not deliver optimal performance for certain mathematical operations
- It only has immutable semantics (i.e. any mathematical operation on a
BigIntegerwill allocate and return a newBigInteger), which leads to unavoidable memory pressure at scale.
This commit replaces BigInteger with libj's BigInt class, which uses a value-encoding scheme to deliver faster operations and mutable behavior which allows for improving memory footprint.
This change delivers a 45% improvement in the current benchmark suite on a 16GB Macbook M1 Pro:
Benchmark Mode Cnt Score Error Units
FF3CipherPerf.testEncrypt thrpt 5 97629.798 ± 2972.249 ops/s
Benchmark Mode Cnt Score Error Units
FF3CipherPerf.testEncrypt thrpt 5 178592.429 ± 3097.461 ops/s
This commit introduces [email protected] as a project dependency.
References: https://mvnrepository.com/artifact/org.libj/math/0.6.8 https://github.com/libj/math https://javadoc.io/doc/org.libj/math/latest/index.html
@altr-benjamin What's the need for maven.repository.redhat.com in build.gradle.kts? libj is available MavenCentral (https://mvnrepository.com/artifact/org.libj/math).
@altr-benjamin What's the need for maven.repository.redhat.com in build.gradle.kts? libj is available MavenCentral (https://mvnrepository.com/artifact/org.libj/math).
libj:utils pulls in diff_match_patch which lives in Redhat GA.
Without maven("https://maven.repository.redhat.com/ga/"):
@altr-benjamin
- Github repo for diff-match-patch states: "This repository has been archived by the owner on Aug 4, 2024. It is now read-only." From a security perspective, that's likely a show-stopper.
Does libj:match mention this dependency? Do they have a plan going forward to replace it?
I filed #59 for this.
- With libj on a MacBook M2, I saw ~20% better performance vs the 40% you reported. What's libmathj_x86_64.dylib which didn't load on my test?
(original) Benchmark Mode Cnt Score Error Units FF3CipherPerf.testEncrypt thrpt 5 101072.369 ± 52038.015 ops/s
(with libj:math patch) // Warmup Iteration 1: Not found: /libmathj_x86_64.dylib // Starting without JNI bindings
Benchmark Mode Cnt Score Error Units FF3CipherPerf.testEncrypt thrpt 5 121699.745 ± 55075.206 ops/s
@bschoening In order:
- Thanks for opening that! I am going to follow that issue, and if there's anything needed in that ecosystem we'd be open to committing upstream as well. FWIW it would appear that the author of the math library is heavily or solely involved in the libj project as a whole.
libj:mathmakes low-level implementations and native bindings available and selects the optimal binding based on circumstances. It would appear in your case that the library is failing to identify the proper shared lib (it looks to be the wrong architecture) and falls back to plain-ol' Java code which the author claims is JIT-friendly. I can't speak to your toolchain specifically, but using IntelliJ and reloading Gradle collected the proper dependency for me.
@altr-benjamin seems it may take a while to resolve the unnecessary diff_match_patch dependency. Given this, I'm considering making a new release of java-fpe which would include the String the char array changes. Let me know if you would find that useful.
The discussion on libj has highlighted some of the downsides of hardwiring in lesser known 3rd party libraries maintained by a sole individual. I'm wondering if we can make this pluggable with a factory provider, the way SLF4J works.
@bschoening if you'd like to cut a new release with the char array changes that's fine by me -- any perf improvement is welcome there.
Good point and interesting comment on the pluggable pattern! IMO the big difference b/w BigInteger and libj is the mutability, but I'd be happy to take that to the drawing board and see if it could be abstracted out enough to be workable.
@altr-benjamin BigInteger is immutable for thread safety. But I don't see that as needed in a Feistel cipher.
Apologies for the many-month wait! I'm going to close this PR -- looks like the maintainer went dark and deleted the libj Github org, so this is no longer a viable change to be made to the project.