On Tue, 25 Nov 2025 18:23:39 GMT, James Yuzawa <[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(...
>
> 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