RFR 10 JDK-8159995: Rename internal Unsafe.compare methods (original) (raw)

David Holmes david.holmes at oracle.com
Mon May 8 05:30:42 UTC 2017

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: - compareAndExchangeVolatile -> 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 - weakCompareAndSwapVolatile -> 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 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 hotspot-dev mailing list