-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
Add BigInteger type #10750
base: development
Are you sure you want to change the base?
Add BigInteger type #10750
Conversation
Thanks, this would be useful to have in the standard library indeed. There are a couple of high-level comments I have (I haven't yet looked through all of the implementation):
|
Thank you for reviewing this pull request.
|
I think this pull request can be reviewed and merged as I added the missing methods ( abs, pow, modPow, isProbablePrime, getLowestSetBit and bitLength) and all the tests for them. |
Yes, this does indeed have my blessing. Hope it's a useful addition. 👍 Thank you @flashultra for doing this work! |
Php tests fails on another test. I think the bigint tests should pass the Php target. |
All tests are green except PHP ( and mac-cpp which failed with other reason) .
|
The above error is when I test with PHP 8.1. |
} | ||
|
||
public static inline function fromInt(value:Int):BigInt { | ||
return new BigInt(BigInt_.fromInt(value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe some caching could be added for values e.g.from -10 to 10, so that in these cases no new instances are created but shared BigInts are re-used. In Java for example values from -16 to 16 are cached https://github.com/openjdk-mirror/jdk7u-jdk/blob/master/src/share/classes/java/math/BigInteger.java#L949
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about that. What is the purpose of this caching in Java ? The only logic for that is if there are any operations and they often use values between -16 and 16. Otherwise , why not cache between -100 and 100 or any other values ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is about reducing garbage collector pressure from very commonly used values. why they decided for +/-16 I don't know. For the regular Java Integer type it is actually -128 to 128 https://github.com/openjdk-mirror/jdk7u-jdk/blob/master/src/share/classes/java/lang/Integer.java#L638
Maybe we can returned the cached values for the once static variables are initialized anyways? I.e. ZERO, ONE, TWO, NEGATIVE_ONE (which I would actually call MINUS_ONE - a symbol that is 25 times more common when you do a GitHub code search).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the cache, since this is not my implementation, I just foundthat there is already a cache for values between -16 and 16 (
haxe/std/haxe/math/bigint/BigInt_.hx
Line 334 in 704100c
public static function fromInt(value:Int):BigInt_ { |
I think this is a good implementation and does not require change
About NEGATIVE_ONE , I changed it to MINUS_ONE
|
||
/* Original code courtesy Chuck Batson (github.com/cbatson) */ | ||
@:allow(haxe.math.bigint) | ||
abstract BigInt(BigInt_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible in case of Java to delegate to the BigInteger class? Mabye other targets also have native BigInt implementations that could be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that maybe will be a good solution as Java implementation have a very optimize methods for modPow and pow . The problems here is about some methods that not exist in Java, and need to be reimplemented here ( withouth using the current BigInt_.hx class). Most methods exists, but miss some for random and random prime numbers.
As I mention in above comments , there is also native BigInteger implementation in Python and Javascript which should be used here too. Native javascript implementation give better performance, but as the Java many current methods should be rewritten as Javascript implementation doesn't have any,
C# also have BigInteger class , but I think the performance is not great.
However, in some tests the current BigInteger have same or better performance against Java ( when use some of those benchmarks : https://github.com/maitag/littleBigInt/tree/master/benchmarks ) . Also Karatsuba multiplication in native Java is slower than the current implementation ( but the have a slighty faster one using implementation of Toom–Cook multiplication )
At the moment , I can't apply native BigInteger implementation for Java, C#, Javascript or Python , but hope so someone else will add them in the future.
For me , the bigger drawback for Haxe implementation is the slower modPow ( and partly pow ) operations which is used for RSA .
@flashultra for the php job to succeed you may need to specify haxe/tests/runci/targets/Php.hx Lines 88 to 95 in 614362f
|
@sebthom Thank you for your help. |
"core dumped" sounds like a segmentation fault. To be honest, I had no good experiences with Cppia myself. A lot of random crashes with it while things worked fine on all other targets. I gave up trying to support it in my projects. maybe @hughsando has some ideas? |
…into development
…into development
There is some problem with CI. Since I can't cancel the CI job, please someone to cancel CI #4960 as the new one (CI #4962) has been set. Some thoughts on this pull request. Any futher optimization will require a native Int64 type and/or Toom-Cook / Schonhage-Strassen implementation. |
CI is just completely broken at the moment it seems. |
This pull request add BigInteger type in Haxe . The implementation is created by Chuck Batson and I have his permission to relicense his code under the Haxe license ( thank you @cbatson ! ).
Some things about the current implementation. I compare three BigInteger implementations - on Chuck ( https://github.com/cbatson/hxBitcoin/tree/master/com/fundoware/engine/bigint) , in haxe-crypto lib ( https://github.com/Jens-G/haxe-crypto/blob/master/src/com/hurlant/math/BigInteger.hx ) and littleBigInt ( https://github.com/maitag/littleBigInt/blob/master/src/BigInt.hx ).
The Chuck implementation give about 5x better performance and also is better than native Java implementation ( 3x times better !).
As a additional info - many languages have a BigInteger , such as Java, C#, Javascript