RFR: jsr166 jdk10 integration wave 5

Martin Buchholz martinrb at google.com
Mon Nov 6 22:12:29 UTC 2017


On Mon, Nov 6, 2017 at 1:36 PM, David Holmes <david.holmes at oracle.com>
wrote:

>
> src/java.base/share/classes/java/util/concurrent/ConcurrentL
> inkedDeque.java
>
>       final Node<E> pred(Node<E> p) {
> !         Node<E> q = p.prev;
> !         return (p == q) ? last() : q;
>       }
>
>       /**
>        * Returns the first node, the unique node p for which:
>        *     p.prev == null && p.next != p
> --- 693,705 ----
>        * Returns the predecessor of p, or the last node if p.prev has been
>        * linked to self, which will only be true if traversing with a
>        * stale pointer that is now off the list.
>        */
>       final Node<E> pred(Node<E> p) {
> !         if (p == (p = p.prev))
> !             p = last();
> !         return p;
>       }
>
> The original version is quite clear, the new version is trying to be far
> too clever with order of evaluation and to me is far less clear.


Well, this idiom is already in widespread use in these files, notably in
succ, which was non-symmetrical with pred for no reason.  The trickier code
saves 2 bytes of bytecode; admittedly unlikely to make a difference; we are
not near the 35-bytecode limit.  The more common form of the idiom is

                if (p == (p = p.next))
                    continue restart;

After 10 years working on lock-free queues it starts to look natural (but
don't try it in C++!)


More information about the core-libs-dev mailing list