Request for review (S): 8008382: Remove redundant use of Atomic::add(jlong, jlong *) in create_new_gc_id()

[Bengt Rutisson]

Hi David,

Thanks for looking at this!

On 2/19/13 5:08 AM, David Holmes wrote:
> 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. :(

Yes, this was actually the reason why we used it instead of Atomic:inc. 
But I agree that it is a bug. All Atomic:add versions should have the 
same semantics.

Hopefully we can just remove the jlong Atomic:add.

Bengt

>
> 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
>>>
>