Request for review (S): 8008382: Remove redundant use of Atomic::add(jlong, jlong *) in create_new_gc_id()
David Holmes
david.holmes at oracle.com
Tue Feb 19 04:08:37 UTC 2013
This looks good to me too! :)
I was going to say that you changed the semantics of the addition by
returning the pre-incremented value, but it turns out that another bug
with the jlong version of Atomic::add is that it returns the original
value instead of the updated one as it is supposed to do. :(
Thanks,
David
Bengt writes:
> On 2/18/13 2:49 PM, Stefan Karlsson wrote:
>> Looks good.
>
> Thanks, Stefan!
>
>>
>> You should probably remove the atomic.hpp include.
>
> Good point. Done.
>
> Bengt
>
>>
>> StefanK
>>
>> On 02/18/2013 02:21 PM, Bengt Rutisson wrote:
>>>
>>> Hi all,
>>>
>>> Could I have a couple of reviews for this change?
>>> http://cr.openjdk.java.net/~brutisso/8008382/webrev.00/
>>>
>>> There is no need to use atomics in create_new_gc_id() since it is not
>>> called by multiple threads in parallel. Also, Atomic::add(jlong,
>>> jlong *) is broken for ARM.
>>>
>>> We should remove the use of Atomic::add in create_new_gc_id() for
>>> now. If we need this to be called by multiple threads in the future
>>> we have to reconsider how this should be done. Either adding a lock
>>> or doing some kind of 32 bit atomic work.
>>>
>>> Thanks,
>>> Bengt
>>
More information about the hotspot-gc-dev
mailing list