RFR: 8320759: Creation of random BigIntegers can be made faster [v3]
Raffaello Giulietti
rgiulietti at openjdk.org
Tue Dec 5 11:51:36 UTC 2023
On Tue, 5 Dec 2023 10:46:41 GMT, fabioromano1 <duke at openjdk.org> wrote:
>> Relaying comments from a colleague:
>>
>> 1. Half of the random bits are wasted by the use of `nextInt()`, which by default invokes `nextLong()` and throws away the lower 32 bits. Not sure if complicating the code to use all the 64 bits of `nextLong()` is worthwhile, though.
>>
>> 2. The way the random bits fill the magnitude array is different. This might break existing reproducible tests with seeded random number generators and fixed seed. Not sure if this is a real problem in practice, though.
>>
>> 3. There seems to be no test coverage that ensures the `BigInteger` invariant has either `mag.length == 0` or `mag[0] != 0`. While the code obviously ensures it, future changes might not, so it might make sense to have this aspect covered by a test.
>
> @bplb
> 1. It would complicate the code, but even use the Random generator less times, so the choice is between code more complicated or more calls to Random generator.
>
> 2. This would be a problem only when, to make some comparisons, a test uses the order of filling the magnitude used by the algorithm, I think.
>
> 3. If it has to be done, I think it should be by an assertion at the end of `ranfomBits(int, Random)` method, since there is no way to access the class fields for a method outside the package.
@fabioromano1
About item 2. If a test external to OpenJDK uses a seedable random generator and initializes it with a fixed seed, and if it then compares the result with an expected value, after the change the test breaks. As mentioned, this might not be a problem in practice.
About item 3. You can take a look at [this test](https://github.com/openjdk/jdk/blob/master/test/jdk/java/math/BigInteger/ByteArrayConstructorTest.java) to see how to access the fields using [this class](https://github.com/openjdk/jdk/blob/master/test/jdk/java/math/BigInteger/java.base/java/math/Accessor.java).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1840635876
More information about the core-libs-dev
mailing list