On 29/06/15 09:37, Vladimir Kozlov wrote: > Hi, Andrew > > Did you file RFE for this change? 8046943 is JEP.
No; I will do so. > typo? "less" -> "more". > > + * number of ints in the number is less than this value we do not > + * use the intrinsic. > + */ > + private static final int MONTGOMERY_INTRINSIC_THRESHOLD = 512; > > trailing spaces: > src/java.base/share/classes/java/math/BigInteger.java:273: Trailing whitespace > src/java.base/share/classes/java/math/BigInteger.java:2770: Trailing > whitespace > > I ran changes through JPRT and linux/solaris passed - thanks. > Next step - Windows: > > C:\jprt\T\P1\s\hotspot\src\cpu\x86\vm\sharedRuntime_x86_64.cpp(26) : fatal > error C1083: Cannot open include file: > 'alloca.h': No such file or directory Hmm, okay. This is going to be fun. :-) AFAIK OpenJDK builds with Visual Studio. The VS equivalent of alloca() is called _alloca() and its header file is <malloc.h>. I'm going to try to do this untested. I think that autoconf will #include malloc.h on Windows automagically, so all that I have to do is create a #define for alloca() on Windows. > I am fine with JDK changes. > > Would be nice to have a test for this change. Do existing tests > cover this code? They do. jdk/test/com/oracle/security/ucrypto/TestRSA looks like a pretty thorough test. If you make any mistake in the arithmetic RSA decryption simply will not work: the result is corrupt. The risks, then, are mistakes such as accidental side-effects. I don't know any way to test for that. The other possible think I could test for is unusual key sizes. I'll have a look. > I agree that we should limit size when to invoke multiplyToLen > intrinsic too. File bug I will assign it. OK. Andrew.