java-fpe icon indicating copy to clipboard operation
java-fpe copied to clipboard

Use `libj.math.BigInt` for arithmetic

Open altr-benjamin opened this issue 1 year ago • 8 comments

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 BigInteger will allocate and return a new BigInteger), 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.

altr-benjamin avatar Nov 22 '24 22:11 altr-benjamin

@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).

bschoening avatar Nov 26 '24 16:11 bschoening

@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/"):

image

ghost avatar Nov 26 '24 16:11 ghost

@altr-benjamin

  1. 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.

  1. 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 avatar Nov 30 '24 21:11 bschoening

@bschoening In order:

  1. 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.
  2. libj:math makes 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.

ghost avatar Dec 03 '24 17:12 ghost

@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 avatar Dec 21 '24 02:12 bschoening

@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.

ghost avatar Dec 21 '24 04:12 ghost

@altr-benjamin BigInteger is immutable for thread safety. But I don't see that as needed in a Feistel cipher.

bschoening avatar Dec 21 '24 05:12 bschoening

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.

ghost avatar May 07 '25 19:05 ghost