On Fri, May 11, 2018 at 9:06 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
> > > On May 9, 2018, at 11:17 AM, Martin Buchholz <marti...@google.com> wrote: > > Time to do this, since Claes is also touching ArrayList. > > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166- > integration/overview.html > > 8202685: Improve ArrayList replaceAll > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166- > integration/replaceAll/index.html > https://bugs.openjdk.java.net/browse/JDK-8202685 > > > ArrayList.java > — > > @Override > public void replaceAll(UnaryOperator<E> operator) { > Objects.requireNonNull(operator); > final int expectedModCount = modCount; > final Object[] es = elementData;! final int size = > this.size;! for (int i = 0; modCount == expectedModCount && i < size; > i++) > es[i] = operator.apply(elementAt(es, i)); > if (modCount != expectedModCount) > throw new ConcurrentModificationException();- > modCount++; > } > > > A CME is not necessarily associated with just structural modifications it > could, on a best effort basis, be associated with any modification, which > is cheaper to do for bulk operations rather than individual operations, and > this operation can be used to perturb all the elements of the list (just > like sort) causing strange and hard to track bugs while in the middle of > iterating. IMHO its better to fail under such circumstances rather than be > silent. > It's tempting to treat modCount as a count of ALL modifications, especially given its name, but that's different from the historical behavior and design of these classes. Consistency with existing spec and implementations is the biggest argument. You can argue that the very idea of structural modifications was a bad idea, but it's still there in https://download.java.net/java/early_access/jdk11/docs/api/java.base/java/util/List.html (Personally, I think checking for concurrent modification at all is not worth the small improvement in safety) > > 8201386: Miscellaneous changes imported from jsr166 CVS 2018-05 > http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166- > integration/miscellaneous/index.html > https://bugs.openjdk.java.net/browse/JDK-8201386 > > > > > PriorityQueue > — > > 587 public E poll() { 588 final Object[] es; 589 final E > result; 590 591 if ((result = (E) ((es = queue)[0])) != null) { > 592 modCount++; 593 final int n; 594 > final E x = (E) es[(n = --size)]; 595 es[n] = null; 596 > if (n > 0) { 597 final Comparator<? super E> cmp; 598 > if ((cmp = comparator) == null) 599 > siftDownComparable(0, x, es, n); 600 else 601 > siftDownUsingComparator(0, x, es, n, cmp); 602 } 603 } > 604 return result; > 605 } > > > Why inline the siftDown method? > There is an effort here to remove gratuitous implementation differences between PriorityQueue and PriorityBlockingQueue. Maybe this one should go the other way - refactor the corresponding code in PBQ into siftDown. But can we trust the VM to not re-read the queue and size fields?
