Links to existing material in OpenJDK:

    
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-December/050326.html

    https://bugs.openjdk.java.net/browse/JDK-8193031

I agree with the removal of the performance advice. We should also remove "identical"; my suggested replacement was "as if".

We can even add some hedging about whether the operation is atomic for certain collections (i.e., the synchronized ones). However, I'm not sure if there is anything useful to say about atomicity of bulk updates. They're only atomic for the synchronized collections, which are largely disused. It's pointless to talk about atomicity for non-concurrent collections, and the operations aren't atomic for things like CopyOnWriteArrayList and a Set projection of ConcurrentHashMap. So I'm not sure discussion of atomicity is useful or warranted here.

(Also, adding a collection of elements one at a time to a CopyOnWriteArrayList: *shudder*.)

I think the most useful thing is to define a new array-reading default method. Each implementation can then override it to use the best technique for that implementation. (I had previously called this "addEach" but I'm flexible on naming.)

Adding a new default method is kind of far afield from where this started. If the spec is bothersome, perhaps we could consider a spec cleanup change separately from implementation changes and definition of a new default method.

s'marks




On 1/30/18 7:07 PM, Martin Buchholz wrote:
I tried to tackle this here:
http://openjdk.markmail.org/thread/eet2zd6ig3pfpv5g
and it's still on my TODO list but not likely to get to top spot soon.

On Tue, Jan 30, 2018 at 7:00 PM, Tagir Valeev <amae...@gmail.com> wrote:

Hello!

I suggest a patch for java.util.Collections#addAll JavaDoc:

--- Collections.java    2018-01-31 09:39:31.599107500 +0700
+++ Collections.java.patched    2018-01-31 09:51:11.929059600 +0700
@@ -5406,4 +5406,8 @@
       * The behavior of this convenience method is identical to that of
-     * {@code c.addAll(Arrays.asList(elements))}, but this method is
likely
-     * to run significantly faster under most implementations.
+     * {@code c.addAll(Arrays.asList(elements))} except possible
+     * difference in intermediate state visibility for concurrent or
+     * synchronized collections. Calling this method does not guarantee
+     * that the intermediate state (some of elements are added) is
invisible,
+     * even if the collection itself provides such guarantee for its
+     * {@link Collection#addAll(Collection)} method.
       *

First, currently it says that Collections#addAll is likely to run
significantly faster. However it's only marginally faster for
collections which delegate their addAll method to standard
AbstractCollection#addAll implementation. Also it could be much slower
for collections which have optimized addAll (like ArrayList,
CopyOnWriteArrayList, ConcurrentLinkedDeque, etc.). I don't know a
single example of collection where Collections#addAll is actually
significantly faster. Also it says that the behavior is identical,
while it's not. If, e.g. c is a collection returned from
synchronizedCollection, then intermediate state of
c.addAll(Arrays.asList(elements)) would not be visible under
synchronized(c) in another thread. On the other hand, replacing such
call with Collections.addAll(c, elements) (to make it "significantly
faster") will lift this guarantee: now you can see partially added
array.

What do you think? Should I file an issue?

With best regards,
Tagir Valeev.

Reply via email to