RFR: 8372134: ThreadLocalRandom no longer overrides nextGaussian [v2]

[Alan Bateman]

On Tue, 25 Nov 2025 18:23:39 GMT, James Yuzawa <duke 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(...
>
> James Yuzawa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8372134: ThreadLocalRandom no longer overrides nextGaussian
>   
>   Fix javadoc

src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java line 507:

> 505:      *           {@link java.util.Random#nextGaussian()} and instead uses the
> 506:      *           ziggurat-based algorithm from the default implementation of
> 507:      *           {@link java.util.random.RandomGenerator#nextGaussian()}.

The implNotes in this class start with "This method is implemented ...".  How about: "This implementation invokes the default implementation of RandomGenerator.nextGaussian and so uses McFarland's fast modified ziggurat algorithm rather than the polar method described in the super class."

src/java.base/share/classes/java/util/concurrent/ThreadLocalRandom.java line 513:

> 511:      * {@inheritDoc}
> 512:      * @throws IllegalArgumentException {@inheritDoc}
> 513:      * @implNote {@inheritDoc}

Good.

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

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