> On 7 May 2017, at 22:30, David Holmes <david.hol...@oracle.com> 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. :) >
Yes, it might be possible after some careful performance analysis (we might run into some subtle issues). >> - 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 :) ). > I do :-) Paul.
