On Tue, 25 Nov 2025 15:08:47 GMT, Raffaello Giulietti <[email protected]>
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
