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
