--- jsr166/src/jsr166y/LinkedTransferQueue.java 2009/11/14 20:27:18 1.67 +++ jsr166/src/jsr166y/LinkedTransferQueue.java 2010/09/09 16:52:49 1.78 @@ -15,6 +15,7 @@ import java.util.Iterator; import java.util.NoSuchElementException; import java.util.Queue; import java.util.concurrent.locks.LockSupport; + /** * An unbounded {@link TransferQueue} based on linked nodes. * This queue orders elements FIFO (first-in-first-out) with respect @@ -321,12 +322,12 @@ public class LinkedTransferQueue exte * situations in which we cannot guarantee to make node s * unreachable in this way: (1) If s is the trailing node of list * (i.e., with null next), then it is pinned as the target node - * for appends, so can only be removed later when other nodes are + * for appends, so can only be removed later after other nodes are * appended. (2) We cannot necessarily unlink s given a * predecessor node that is matched (including the case of being - * cancelled): the predecessor may already be already unspliced, - * in which case some previous reachable node may still point to - * s. (For further explanation see Herlihy & Shavit "The Art of + * cancelled): the predecessor may already be unspliced, in which + * case some previous reachable node may still point to s. + * (For further explanation see Herlihy & Shavit "The Art of * Multiprocessor Programming" chapter 9). Although, in both * cases, we can rule out the need for further action if either s * or its predecessor are (or can be made to be) at, or fall off @@ -343,23 +344,23 @@ public class LinkedTransferQueue exte * When these cases arise, rather than always retraversing the * entire list to find an actual predecessor to unlink (which * won't help for case (1) anyway), we record a conservative - * estimate of possible unsplice failures (in "sweepVotes). We - * trigger a full sweep when the estimate exceeds a threshold - * indicating the maximum number of estimated removal failures to - * tolerate before sweeping through, unlinking cancelled nodes - * that were not unlinked upon initial removal. We perform sweeps - * by the thread hitting threshold (rather than background threads - * or by spreading work to other threads) because in the main - * contexts in which removal occurs, the caller is already - * timed-out, cancelled, or performing a potentially O(n) - * operation (i.e., remove(x)), none of which are time-critical - * enough to warrant the overhead that alternatives would impose - * on other threads. + * estimate of possible unsplice failures (in "sweepVotes"). + * We trigger a full sweep when the estimate exceeds a threshold + * ("SWEEP_THRESHOLD") indicating the maximum number of estimated + * removal failures to tolerate before sweeping through, unlinking + * cancelled nodes that were not unlinked upon initial removal. + * We perform sweeps by the thread hitting threshold (rather than + * background threads or by spreading work to other threads) + * because in the main contexts in which removal occurs, the + * caller is already timed-out, cancelled, or performing a + * potentially O(n) operation (e.g. remove(x)), none of which are + * time-critical enough to warrant the overhead that alternatives + * would impose on other threads. * * Because the sweepVotes estimate is conservative, and because * nodes become unlinked "naturally" as they fall off the head of * the queue, and because we allow votes to accumulate even while - * sweeps are in progress, there are typically signficantly fewer + * sweeps are in progress, there are typically significantly fewer * such nodes than estimated. Choice of a threshold value * balances the likelihood of wasted effort and contention, versus * providing a worst-case bound on retention of interior nodes in @@ -422,13 +423,13 @@ public class LinkedTransferQueue exte } final boolean casItem(Object cmp, Object val) { - assert cmp == null || cmp.getClass() != Node.class; + // assert cmp == null || cmp.getClass() != Node.class; return UNSAFE.compareAndSwapObject(this, itemOffset, cmp, val); } /** - * Creates a new node. Uses relaxed write because item can only - * be seen if followed by CAS. + * Constructs a new node. Uses relaxed write because item can + * only be seen after publication via casNext. */ Node(Object item, boolean isData) { UNSAFE.putObject(this, itemOffset, item); // relaxed write @@ -446,7 +447,7 @@ public class LinkedTransferQueue exte /** * Sets item to self and waiter to null, to avoid garbage * retention after matching or cancelling. Uses relaxed writes - * bacause order is already constrained in the only calling + * because order is already constrained in the only calling * contexts: item is forgotten only after volatile/atomic * mechanics that extract items. Similarly, clearing waiter * follows either CAS or return from park (if ever parked; @@ -488,7 +489,7 @@ public class LinkedTransferQueue exte * Tries to artificially match a data node -- used by remove. */ final boolean tryMatchData() { - assert isData; + // assert isData; Object x = item; if (x != null && x != this && casItem(x, null)) { LockSupport.unpark(waiter); @@ -541,7 +542,7 @@ public class LinkedTransferQueue exte @SuppressWarnings("unchecked") static E cast(Object item) { - assert item == null || item.getClass() != Node.class; + // assert item == null || item.getClass() != Node.class; return (E) item; } @@ -656,7 +657,7 @@ public class LinkedTransferQueue exte for (;;) { Object item = s.item; if (item != e) { // matched - assert item != s; + // assert item != s; s.forgetContents(); // avoid garbage return this.cast(item); } @@ -876,17 +877,19 @@ public class LinkedTransferQueue exte } /** - * Unlink matched nodes encountered in a traversal from head + * Unlinks matched (typically cancelled) nodes encountered in a + * traversal from head. */ private void sweep() { - Node p = head, s, n; - while (p != null && (s = p.next) != null && (n = s.next) != null) { - if (p == s || s == n) - p = head; // stale - else if (s.isMatched()) - p.casNext(s, n); - else + for (Node p = head, s, n; p != null && (s = p.next) != null; ) { + if (p == s) // stale + p = head; + else if (!s.isMatched()) p = s; + else if ((n = s.next) == null || s == n) // trailing node is pinned + break; + else + p.casNext(s, n); } } @@ -1124,7 +1127,11 @@ public class LinkedTransferQueue exte * @return {@code true} if this queue contains no elements */ public boolean isEmpty() { - return firstOfMode(true) == null; + for (Node p = head; p != null; p = succ(p)) { + if (!p.isMatched()) + return !p.isData; + } + return true; } public boolean hasWaitingConsumer() {