RFR: 8320759: Creation of random BigIntegers can be made faster [v3]
fabioromano1
duke at openjdk.org
Tue Dec 5 10:51:36 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
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.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1840496537
More information about the core-libs-dev
mailing list