Atomic::add(jlong) broken?

[David Holmes]

Following up ...

On 3/02/2016 4:14 PM, David Holmes wrote:
> On 3/02/2016 6:55 AM, Roman Kennke wrote:
>> Hello,
>>
>> I believe Atomic::add(jlong) is broken. The comment above it says:
>>
>>    // Atomically add to a location, return updated value
>>
>> Except in atomic.cpp, add(jlong) returns the old value.
>
> Yes that seems broken.
>
>> It causes quite some headscratching on my side :-)
>>
>> Fixing this seems easy. I am wonder if any code uses this though, maybe
>> it should be removed altogether?
>
> It was added here:
>
> http://hg.openjdk.java.net/jdk8/jdk8/hotspot//rev/2a241e764894
>
> for the GC log file rotation code back in 2011. But then removed here:
>
> http://hg.openjdk.java.net/jdk8/jdk8/hotspot/rev/0598674c0056
>
> because it was unnecessary and because of the missing Atomic:load(jlong)
> support it required.
>
> I think it can be deleted now and probably should be.

Note when I say "removed here" I meant its use was removed. 
Unfortunately at some point memoryTracker.hpp also started to use it, 
but we can fix that too and this will be handled under:

https://bugs.openjdk.java.net/browse/JDK-8157709

David
-----

>> On the other hand, the implementation there uses a CAS-based loop. I
>> think an easier fix would be to cast to size_t or intptr_t and use the
>> atomic impl of that.
>>
>> What do you think?
>
> Delete it.
>
> Thanks,
> David
>
>> Roman
>>