Hi Claes, Thanks for the review.
The previous incarnation of those tests was awkwardly designed to test a combination of getAndAdd and addAndGet rather than test specifically for overflow. With the removal of addAndGet i simplified it to be the same as the acquire/release variants. Paul. > On 10 Aug 2016, at 02:13, Claes Redestad <[email protected]> wrote: > > > > On 2016-08-10 02:15, Paul Sandoz wrote: >> Hi, >> >> Please review the following webrev which removes VarHandle.addAndGet: >> >> >> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162106-vhs-rm-addAndGet/webrev/ >> >> <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162106-vhs-rm-addAndGet/webrev/> > > Changes look good in general, but unless I'm reading this wrong it seems some > of the tests covered overflows previously, but no longer does so. > > In: > > http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162106-vhs-rm-addAndGet/webrev/test/java/lang/invoke/VarHandles/X-VarHandleTestMethodHandleAccess.java.template.udiff.html > > ... instead of: > > $type$ o = ($type$) > hs.get(TestAccessMode.GET_AND_ADD).invokeExact(recv, $value2$); > assertEquals(o, $value1$, "getAndAdd $type$"); > $type$ x = ($type$) hs.get(TestAccessMode.GET).invokeExact(recv); > assertEquals(x, ($type$)($value1$ + $value2$), "getAndAdd $type$ > value"); > > ... consider doing something like this: > > $type$ o = ($type$) > hs.get(TestAccessMode.GET_AND_ADD).invokeExact(recv, $value2$); > assertEquals(o, $value1$, "getAndAdd $type$"); > o = ($type$) hs.get(TestAccessMode.GET_AND_ADD).invokeExact(recv, > $value2$); > assertEquals(o, ($type$)($value1$ + $value2$), "getAndAdd $type$"); > $type$ x = ($type$) hs.get(TestAccessMode.GET).invokeExact(recv); > assertEquals(x, ($type$)($value1$ + $value2$ + $value2$), "get > $type$ value"); > > Thanks! > > /Claes > >> >> This turns out to be really just a helper method which is built upon >> getAndAdd. >> >> I had to update some classes in j.u.concurrent.atomics. > > > > > > > > >> >> Thanks, >> Paul. >
