Bug ID: JDK-8332618 Rename the internal os::random() to os::random31bits(), and/or provide os::random

Type: Enhancement
Component: hotspot
Sub-Component: runtime

Priority: P4
Status: Open
Resolution: Unresolved

Submitted: 2024-05-21
Updated: 2024-05-24

Other
tbdUnresolved

Our os::random() implementation only returns 31 bits, the 32nd bit is always set to 0 because of our choice of implementation:

   * next_rand = (16807*seed) mod (2**31-1)

If one wants to use it to choose between true/false, one might be tempted to use:

  if (os::random() > 0) {
  } else {
  }

but that will not work, since the returned value will always be positive.

We should rename os::random() to os::random31bits(), and/or provide os::random_interval(int range)

Adding a comment would also be useful.

Hopefully, this would help to use this API correctly.

I agree with [~dholmes], among other things because renaming makes life harder for downporters and forces mental adjustments when you work on different JDK versions. So there has to be enough benefit. I'm fine with someone providing a 64-bit os::random64, however, that would be useful.
24-05-2024
> Why force potential users of current os::random() to learn its quirks? So the basic complaint is that the name "random" doesn't convey enough information to understand the properties of the underlying RNG - I agree. But how far do you go with trying to convey such properties before just taking the stance that if you need detail go look at the docs (code comments in our case - and yes they certainly can be improved). Look at rand() in the C library for example - I guess the people that gave us os::random are from the same era. Do we perhaps need a 64-bit RNG? Sure, no problem. Can we get more uses out of os::random if we make it 32-bit? Okay I can buy that too - I have no issue if someone says "os::random only produces 31-bit results but if we switch to an algorithm that uses 32-bit then we could use it for X, Y, Z additional usecases.". But if we do that then we need to understand how that changes the properties of the RNG and how that might impact its use with hashcodes.
24-05-2024
[~dholmes] I am not the "one" I was referring to :-) I did in fact look at os::random() and how it is used and I think we can/should do better. Is there a reason why os::random() only returns 31 bits? Must it return 31 bits? Would anything break if it returned 32 bits? I would prefer to upgrade it to full 32bits, this way, it would be more robust (and would work fine for sign check). I see the need for 64 bit random number generator in hotspot as well: - ThreadHeapSampler::next_random() - JfrPRNG::next_uniform() - https://github.com/openjdk/jdk/pull/18289 might be others. With current os::random() we would need 3 calls to fill 64 bits (31bits + 31bits + 2bits) CRC are well know and hardware accelerated, even on microcontrollers, why not use CRC? And if CRC truly does not work for us, then there are other 32bit and 64bit random generating algorithms we should consider. Why force potential users of current os::random() to learn its quirks? The area of "random" API in hotspot could use a refresh and good comments, IMHO.
23-05-2024
[~dholmes] You refer to JDK-8329968, right? Pure coincidence :) That one was a fallout of investigations I did for CDS archive generation reproducibility, which is an effort we at RH step up to guard against supply chain attacks. Gerard's motivation comes from https://github.com/openjdk/jdk/pull/18289 .
23-05-2024
Is there something wrong with the algorithm used by os::random? The documentation is quite clear on what that algorithm is. Do crc32 or crc64 perform the same? How will using different algorithms affect the distribution of hashcodes that os::random was primarily intended for? From markWord.hpp // 64 bits: // -------- // unused:25 hash:31 -->\| unused_gap:1 age:4 unused_gap:1 lock:2 (normal object) // // - hash contains the identity hash value: largest value is // 31 bits, see os::random(). Also, 64-bit vm's require // a hash value no bigger than 32 bits because they will not // properly generate a mask larger than that: see library_call.cpp > If one wants to use it to choose between true/false, one might be tempted to use: > > if (os::random() > 0) { > } else { > } If one might be tempted to do that one should think hard about what one is assuming about how os::random distributes positive and negative values, if one wants a 50:50 spread. So one would then need to go and look at the actual algorithm. Given the range of requirements for randomness in different circumstances I'm somewhat surprised that people seem to see os::random() exists and then assume that it must be the kind of RNG they are looking for. If we need a different RNG for different uses then we can certainly add one (or more), but I think poor old os::random() is being treated unduly harshly recently. :)
23-05-2024
Should we just consider upgrading our random to crc32 and crc64 instead?
22-05-2024
Also, we could use a 64bit (or 62bit) version of this API as well. Since the current random() only return 31 bits we can return 62 bits, using 2 calls to random() For true 64 bits, we would need 3 calls to random() For true 32 bits, we would need2 calls to random() This area could really use some thought, comments and clean up...
22-05-2024
Renaming it random() to random_positive() could be a choice too.
21-05-2024