RFR: 8320759: Creation of random BigIntegers can be made faster [v3]

fabioromano1 duke at openjdk.org
Tue Dec 5 12:51:39 UTC 2023


On Mon, 4 Dec 2023 22:26:32 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> fabioromano1 has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Create RandomBigIntegers.java
>>   
>>   Create a benchmark to measure the performance of BigInteger(int, Random) constructor implementation.
>
> 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
About point 1. The behavior of `nextInt()` depends by the implementation of the Random object specified, which of course is not predictable. Furthermore, the claim that `nextInt()` invokes `nextLong()` by default and throws away the lower 32 bits is evidently denied by the facts: simply look at the last version of the method implementation in the class Random: https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/Random.java#L494
So, in the light of that, I think it's not worthwhile using `nextLong()` instead of `nextInt()`, it would only complicate the code.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1840727834


More information about the core-libs-dev mailing list