> On May 11, 2018, at 9:33 AM, Martin Buchholz <marti...@google.com> wrote: > > > > On Fri, May 11, 2018 at 9:06 AM, Paul Sandoz <paul.san...@oracle.com > <mailto:paul.san...@oracle.com>> wrote: > > >> On May 9, 2018, at 11:17 AM, Martin Buchholz <marti...@google.com >> <mailto: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 >> >> <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 >> >> <http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/replaceAll/index.html> >> https://bugs.openjdk.java.net/browse/JDK-8202685 >> <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.
I mentally revised the history when doing the collections/stream API work since we added more bulk operations, since this is on a “best effort” basis and if it’s cheap to do then there is no real harm in it and it might help. > 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 > > <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) I tend to agree, the cost of specifying and implementing is quite high (i have been saved once by this functionality). > > >> 8201386: Miscellaneous changes imported from jsr166 CVS 2018-05 >> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html >> >> <http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html> >> https://bugs.openjdk.java.net/browse/JDK-8201386 >> <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? If it inlines it probably won’t :-) Paul.
