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

[coleen.phillimore at oracle.com]

Please review version .02 which removes use of replace_if_null, but not 
the function.  A separate RFE can be filed to discuss that.

open webrev at http://cr.openjdk.java.net/~coleenp/8188220.02/webrev

Thanks,
Coleen

On 10/11/17 7:07 AM, coleen.phillimore at oracle.com wrote:
>
>
> On 10/11/17 4:12 AM, Robbin Ehn wrote:
>> 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
>
> Thank you for all your feedback.  Erik best described what I was 
> thinking.  I will remove it then.  There were not that many instances 
> and one instance that people thought would be useful, needed the old 
> return value.
>
> Coleen
>>
>> 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.
>>>>>>
>>>>
>