RFR (L, but tedious) 8188220: Remove Atomic::*_ptr() uses and overloads from hotspot

[Robbin Ehn]

On 10/11/2017 10:09 AM, David Holmes wrote:
> On 11/10/2017 5:45 PM, Erik Österlund wrote:
> 
> Removing the operation is a different argument to renaming it. Most of the above argues for removing it. :)

+1 on removing

Thanks, Robbin

> 
> Cheers,
> David
> -----
> 
>> I have not reviewed this completely yet - thought I'd wait with that until we agree about replace_if_null, if that is okay.
>>
>> Thanks,
>> /Erik
>>
>> On 2017-10-11 05:55, David Holmes wrote:
>>> On 11/10/2017 1:43 PM, Kim Barrett wrote:
>>>>> On Oct 10, 2017, at 6:01 PM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>> Summary: With the new template functions these are unnecessary.
>>>>>
>>>>> 2. renamed Atomic::replace_if_null to Atomic::cmpxchg_if_null.  I disliked the first name because it's not explicit from the callers that there's an underlying cas.  
>>>>> If people want to fight, I'll remove the function and use cmpxchg because there are only a couple places where this is a little nicer.
>>>>
>>>> I'm still looking at other parts, but I want to respond to this now.
>>>>
>>>> I object to this change.  I think the proposed new name is confusing,
>>>> suggesting there are two different comparisons involved.
>>>>
>>>> I originally called it something else that I wasn't entirely happy
>>>> with.  When David suggested replace_if_null I quickly adopted that as
>>>> I think that name exactly describes what it does.  In particular, I
>>>> think "atomic replace if" pretty clearly suggests a test-and-set /
>>>> compare-and-swap type of operation.
>>>
>>> I totally agree. It's an Atomic operation, the implementation will involve something atomic, it doesn't matter if it is cmpxchg or something else. The name 
>>> replace_if_null describes exactly what the function does - it doesn't have to describe how it does it.
>>>
>>> David
>>> -----
>>>
>>>> Further, I think any name involving "cmpxchg" is problematic because
>>>> the result of this operation is intentionally different from cmpxchg,
>>>> in order to better support the primary use-case, which is lazy
>>>> initialization.
>>>>
>>>> I also object to your alternative suggestion of removing the operation
>>>> entirely and just using cmpxchg directly instead.  I don't recall how
>>>> many occurrences there presently are, but I suspect more could easily
>>>> be added; it's part of a lazy initialization pattern similar to DCLP
>>>> but without the locks.
>>>>
>>