On Tue, 5 Dec 2023 12:46:49 GMT, fabioromano1 <d...@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 > 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. @fabioromano1 I made the original observations reported by @bplb. You are right about item 1: I was erroneously looking at the implementation in interface `RandomGenerator`, not in class `Random`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16817#issuecomment-1841002629
