Code Review 7051516: ThreadLocalRandom seed is never initialized so all instances generate the same sequence
David Holmes
David.Holmes at oracle.com
Fri Jun 17 01:40:20 UTC 2011
Chris Hegarty said the following on 06/17/11 02:37:
> On 06/16/11 05:32 PM, Mike Duigou wrote:
>> Hi Chris;
>>
>> The getClass() seems unnecessary. Why not :
>>
>> public Random(long seed) {
>> this.seed = new AtomicLong();
>> setSeed(seed);
>> }
>>
>> or
>>
>> private final AtomicLong seed = new AtomicLong();
>>
>> public Random(long seed) {
>> setSeed(seed);
>> }
>>
>> Both of these would seem to have the same effect without needing to do
>> the explicit class check.
>
> The change ( originally from Martin ) tries to save a couple of volatile
> writes in the common case. Yes, it's not a pretty as it could be.
But is the getClass + check + branch cheaper than those volatile writes?
I know the history is somewhat convoluted here and the real problem was
caused by trying to define a subclass (ThreadLocalRandom) of Random that
stripped away Random's thread-safety mechanisms. You not only end up
with redundant state (two seeds!) but it breaks type substitutability. :(
But the milk has been spilt here and right now we just need to have TLR
properly initialize its local seed. So its a thumbs up from me.
David
More information about the core-libs-dev
mailing list