RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread

[Aleksey Shipilev]

On 01/15/2013 06:50 PM, Peter Levart wrote:
> On 01/15/2013 03:38 PM, Aleksey Shipilev wrote:
>> On 01/15/2013 06:33 PM, Peter Levart wrote:
>>> One more thing, not related to serialization:
>>>
>>> If a TLR reference is somehow passed from the thread that obtained it
>>> via TLR.current() to some other thread that did never call TLR.current()
>>> and this other thread calls methods on such instance (nextInt(), ...),
>>> it will start the random sequence from the zero seed, bypassing
>>> localInit() call...
>>>
>>> Is this ok?
>> I think this counts as "accidental" sharing, and Javadoc recommends to
>> always do TLR.current().* to take the appropriate TLR; hence I think
>> this is the adopted failure scenario. The upside for *not* fixing this
>> is skipping the initialized checks from each of next*() methods, which
>> are needed in corner cases only, thus saving the hot-path performance.
> 
> If javadoc recommends to always do TLR.current() then this method is
> also hot-path. So why not move the initialization check from current()
> to next(int bits)? Well, the following usage pattern can save some
> checks in the original case:
> 
> TLR r = TLR.current();
> 
> r.next*(); .... r.next*(); ....

I'm not sure I understand your comment, Peter. Your code calls
TLR.current() first, and that is a good move, as per Javadoc; it should
be probably mentioned that TLR should be *acquired* via current(). This
does not prevent acquiring TLR once per scope, or even per thread.
Before, the performance bets were off for rogue non-current() TLRs, now
we should extend that to the statistical properties as well.

-Aleksey.