RFR 10 JDK-8159995: Rename internal Unsafe.compare methods

Ron Pressler ron.pressler at oracle.com
Wed May 10 20:57:07 UTC 2017


Thank you.

Your comments, and Paul's, have all been addressed in a revised patch 
(same webrev).

Ron

On 08/05/2017 08:30, David Holmes wrote:
> Hi Ron,
>
> On 6/05/2017 5:27 AM, Ron Pressler wrote:
>> Hi,
>> Please review the following core/hotspot change:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8159995
>> core webrev:
>> http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8159995-unsafe-compare-and-swap-to-set-jdk/webrev/ 
>>
>>
>> hotspot webrev:
>> http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8159995-unsafe-compare-and-swap-to-set-hotspot/webrev/ 
>>
>>
>>
>> This change is covered by existing tests.
>>
>> The following renaming was applied:
>>
>> - compareAndExchange*Volatile -> compareAndExchange*
>> - compareAndSwap* -> compareAndSet*
>
> So to clarify this for others, there was confusion surrounding the use 
> of "swap" versus "exchange" when both words mean the same thing 
> effectively, but the "swap" functions return a boolean, while the 
> "exchange" functions return the old value. So we changed "swap" to 
> "set" across the APIs - _except_ for the old 
> /jdk.unsupported/share/classes/sun/misc/Unsafe.java because we can't 
> change its exported API for compatibility reasons.
>
> Given any "swap(exp, new)" function can be implemented as 
> "exchange(exp, new) == exp" I'm not sure why we have two complete sets 
> of functions all the way through. But I guess that is a different 
> issue. :)
>
>> - weakCompareAndSwap* -> weakCompareAndSet*Plain
>> - weakCompareAndSwap*Volatile -> weakCompareAndSet*
>>
>> At this stage, only method and hotspot intrinsic names were changed;
>> node names were left as-is, and may be handled in a separate issue.
>
> Overall looks good for libs and hotspot changes.
>
> One nit I spotted:
>
> src/java.base/share/classes/java/util/concurrent/atomic/AtomicLong.java
>
> +     * compareAndSwap for longs. While the intrinsic compareAndSetLong
>
> compareAndSwap should be compareAndSet
>
> ---
>
> All hotspot files need their copyright years updated to 2017 (if not 
> already).
>
> As there are hotspot changes this must be pushed using JPRT and 
> "-testset hotspot" (but your sponsor should know that :) ).
>
> Thanks,
> David
>
>> Ron



More information about the core-libs-dev mailing list