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

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed Oct 11 11:07:28 UTC 2017

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.

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

More information about the hotspot-dev mailing list