RFR: jsr166 jdk integration 2018-05

[Martin Buchholz]

On Fri, May 11, 2018 at 9:06 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:

>
>
> On May 9, 2018, at 11:17 AM, Martin Buchholz <martinrb at 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?