RFR: 8372134: ThreadLocalRandom no longer overrides nextGaussian

[Alan Bateman]

On Tue, 25 Nov 2025 15:08:47 GMT, Raffaello Giulietti <rgiulietti at openjdk.org> wrote:

>> I have noticed in Java 25 (and earlier versions) that calling `ThreadLocalRandom.current().nextGaussian()` uses the Random.nextGaussian() implementation, which is synchronized. Under heavy load in a multithreaded environment, I was detecting lock contention.
>> 
>> Here is a very simple reproducer:
>> 
>> ThreadLocalRandom r = ThreadLocalRandom.current();
>> // step into this call using a debugger
>> r.nextGaussian();
>> 
>> 
>> It dispatches to the synchronized Random implementation, since ThreadLocalRandom extends Random, thus the default implementation (not synchronizing) on RandomGenerator is not used.
>> 
>> Sketch:
>> 
>> public interface RandomGenerator {
>> 	default double nextGaussian() {
>> 		// remove TAOCP comment since it is out of date, and this uses the ziggurat algorithm instead
>> 		return RandomSupport.computeNextGaussian(this);
>> 	}
>> }
>> 
>> public class Random implements RandomGenerator {
>> 	@Override
>> 	public synchronized double nextGaussian() {
>> 		// synchronized version ...
>> 	}
>> }
>> 
>> public class ThreadLocalRandom extends Random {
>> 
>> 	// ADD THIS
>> 	@Override
>> 	public double nextGaussian() {
>> 		return RandomSupport.computeNextGaussian(this);
>> 	}
>> }
>> 
>> 
>> A comment on ThreadLocalRandom states "This implementation of ThreadLocalRandom overrides the definition of the nextGaussian() method in the class Random, and instead uses the ziggurat-based algorithm that is the default for the RandomGenerator interface.” However, there is none such override happening. It appears that prior to a0ec2cb289463969509fe508836e3faf789f46d8 the nextGaussian implementation was non-locking since it used proper ThreadLocals.
>> 
>> I conducted an audit of all of the RandomGenerator and Random methods to see if there are any others:
>> 
>> 
>> Set<String> tlrMethods = new HashSet<>();
>> for (Method method : java.util.concurrent.ThreadLocalRandom.class.getDeclaredMethods()) {
>> 	int mod = method.getModifiers();
>> 	if (!Modifier.isStatic(mod) && Modifier.isPublic(mod)) {
>> 		String desc =
>> 				method.getReturnType() + " " + method.getName() + " " + Arrays.toString(method.getParameters());
>> 		tlrMethods.add(desc);
>> 	}
>> }
>> for (Method method : java.util.Random.class.getDeclaredMethods()) {
>> 	int mod = method.getModifiers();
>> 	if (!Modifier.isStatic(mod) && Modifier.isPublic(mod)) {
>> 		String desc =
>> 				method.getReturnType() + " " + method.getName() + " " + Arrays.toString(method.getParameters());
>> 		if (!tlrMethods.contains(desc)) {
>> 			System.out.println(...
>
> src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java line 503:
> 
>> 501: 
>> 502:     /**
>> 503:      * {@inheritDoc}
> 
> Inheriting the doc from `Random` doesn't seem correct in this case.

Right, this will require attention. You kinda want `{@inheritDoc java.util.random.RandomGenerator}` but without overriding the method description then it won't appear in the Method Summary. The nextGaussian method defined by j.u.Random and j.u.random.RandomGenrator will be listed in the "Method declared by ..." section of course. 

In addition, Random's implSpec has the polar method whereas RandomGenrator's implSpec is ziggurat algorithm. So I think TLR.nextGaussian will need to copy from text RandomSupport.computeNextGaussian to get the right comment.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28483#discussion_r2560732582