--- jsr166/src/jsr166e/ConcurrentHashMapV8.java 2013/07/01 19:19:31 1.108 +++ jsr166/src/jsr166e/ConcurrentHashMapV8.java 2013/07/04 18:34:49 1.110 @@ -568,9 +568,9 @@ public class ConcurrentHashMapV8 /* * Encodings for Node hash fields. See above for explanation. */ - static final int MOVED = 0x8fffffff; // (-1) hash for forwarding nodes - static final int TREEBIN = 0x80000000; // hash for roots of trees - static final int RESERVED = 0x80000001; // hash for transient reservations + static final int MOVED = -1; // hash for forwarding nodes + static final int TREEBIN = -2; // hash for roots of trees + static final int RESERVED = -3; // hash for transient reservations static final int HASH_BITS = 0x7fffffff; // usable bits of normal node hash /** Number of CPUS, to place bounds on some sizings */ @@ -597,7 +597,7 @@ public class ConcurrentHashMapV8 final int hash; final K key; volatile V val; - Node next; + volatile Node next; Node(int hash, K key, V val, Node next) { this.hash = hash; @@ -722,8 +722,9 @@ public class ConcurrentHashMapV8 * errors by users, these checks must operate on local variables, * which accounts for some odd-looking inline assignments below. * Note that calls to setTabAt always occur within locked regions, - * and so do not need full volatile semantics, but still require - * ordering to maintain concurrent readability. + * and so in principle require only release ordering, not need + * full volatile semantics, but are currently coded as volatile + * writes to be conservative. */ @SuppressWarnings("unchecked") @@ -737,7 +738,7 @@ public class ConcurrentHashMapV8 } static final void setTabAt(Node[] tab, int i, Node v) { - U.putOrderedObject(tab, ((long)i << ASHIFT) + ABASE, v); + U.putObjectVolatile(tab, ((long)i << ASHIFT) + ABASE, v); } /* ---------------- Fields -------------- */ @@ -2140,20 +2141,29 @@ public class ConcurrentHashMapV8 } Node find(int h, Object k) { - Node e; int n; - Node[] tab = nextTable; - if (k != null && tab != null && (n = tab.length) > 0 && - (e = tabAt(tab, (n - 1) & h)) != null) { - do { + // loop to avoid arbitrarily deep recursion on forwarding nodes + outer: for (Node[] tab = nextTable;;) { + Node e; int n; + if (k == null || tab == null || (n = tab.length) == 0 || + (e = tabAt(tab, (n - 1) & h)) == null) + return null; + for (;;) { int eh; K ek; if ((eh = e.hash) == h && ((ek = e.key) == k || (ek != null && k.equals(ek)))) return e; - if (eh < 0) - return e.find(h, k); - } while ((e = e.next) != null); + if (eh < 0) { + if (e instanceof ForwardingNode) { + tab = ((ForwardingNode)e).nextTable; + continue outer; + } + else + return e.find(h, k); + } + if ((e = e.next) == null) + return null; + } } - return null; } } @@ -2327,10 +2337,11 @@ public class ConcurrentHashMapV8 int nextn = nextTab.length; ForwardingNode fwd = new ForwardingNode(nextTab); boolean advance = true; + boolean finishing = false; // to ensure sweep before committing nextTab for (int i = 0, bound = 0;;) { int nextIndex, nextBound, fh; Node f; while (advance) { - if (--i >= bound) + if (--i >= bound || finishing) advance = false; else if ((nextIndex = transferIndex) <= transferOrigin) { i = -1; @@ -2346,14 +2357,19 @@ public class ConcurrentHashMapV8 } } if (i < 0 || i >= n || i + n >= nextn) { + if (finishing) { + nextTable = null; + table = nextTab; + sizeCtl = (n << 1) - (n >>> 1); + return; + } for (int sc;;) { if (U.compareAndSwapInt(this, SIZECTL, sc = sizeCtl, ++sc)) { - if (sc == -1) { - nextTable = null; - table = nextTab; - sizeCtl = (n << 1) - (n >>> 1); - } - return; + if (sc != -1) + return; + finishing = advance = true; + i = n; // recheck before commit + break; } } } @@ -2395,6 +2411,10 @@ public class ConcurrentHashMapV8 else hn = new Node(ph, pk, pv, hn); } + setTabAt(nextTab, i, ln); + setTabAt(nextTab, i + n, hn); + setTabAt(tab, i, fwd); + advance = true; } else if (f instanceof TreeBin) { TreeBin t = (TreeBin)f; @@ -2426,13 +2446,11 @@ public class ConcurrentHashMapV8 (hc != 0) ? new TreeBin(lo) : t; hn = (hc <= UNTREEIFY_THRESHOLD) ? untreeify(hi) : (lc != 0) ? new TreeBin(hi) : t; + setTabAt(nextTab, i, ln); + setTabAt(nextTab, i + n, hn); + setTabAt(tab, i, fwd); + advance = true; } - else - ln = hn = null; - setTabAt(nextTab, i, ln); - setTabAt(nextTab, i + n, hn); - setTabAt(tab, i, fwd); - advance = true; } } } @@ -2453,7 +2471,7 @@ public class ConcurrentHashMapV8 U.compareAndSwapInt(this, SIZECTL, sc, -2)) transfer(tab, null); } - else if ((b = tabAt(tab, index)) != null) { + else if ((b = tabAt(tab, index)) != null && b.hash >= 0) { synchronized (b) { if (tabAt(tab, index) == b) { TreeNode hd = null, tl = null;