Resync j.u.c and Update ConcurrentHashMap to v8 - JEP 155

[David Holmes]

Hi Chris,

I'm adding this to the top because now I've gone through it I have to 
say that this really needs to be broken up. We have everything from 
trivial doc punctuation fixes, through significant performance 
enhancements, to streamification of various classes - with some actual 
bugs fixed inbetween! There have to be some existing CRs that cover some 
of this. :(

A few more comments below.

On 25/05/2013 2:12 AM, Chris Hegarty wrote:
> The final outstanding task from JEP 155 is to add CHMv8, and update the
> remainder of the j.u.c implementation, add spliterator implementation
> for concurrent collections, etc
>
> 8004138: Resync java.util.concurrent classes with Dougs CVS - 05/2013
> 8005704: Update ConcurrentHashMap to v8
>
> specdiff:
> http://cr.openjdk.java.net/~chegar/8004138/ver.00/specdiff/java/util/concurrent/package-summary.html

BlockingDeque has a spec update that is not part of your specdiff.

Have all spec clarifications gone through CCC?

Have all new API's gone through CCC? (Or flagged to go through - eg 
Executors.newWorkStealingPool ?)

> webrev
> http://cr.openjdk.java.net/~chegar/8004138/ver.00/webrev/

As Mike said a lot to look at. And thanks Mike for filtering the @code 
changes :)

I agree with Mike that it is a shame to see reversion to old-style code 
(loops versus iterators) because of performance issues. :(

I also find the seemingly gratuitous text reformatting to be somewhat 
inconvenient in a review of this size. Simple reformatting should be 
done as a separate item where we can show that despite the change to the 
source files, there is actually no change to the corresponding 
specification. As it stands there's no easy way to see if a 'changed' 
block of text has actually been changed. :(

> As per usual all changes are coming from Doug, and others. Most of the
> changes have been baking in the lambda repo for some time now.

So has the spec for the streamification of CHM been discussed elsewhere? 
This seems like something that should be part of the lambda work and I 
don't recall seeing it. Has the terminology been checked for consistency 
against java.util.stream package docs? I don't see any links from CHM to 
those docs, and I think there should be. And I see the reduction 
operation being named "reducer" when the chosen nomenclature was 
"accumulator".

I also question the ConcurrentMap change regarding getOrDefault. I don't 
know if its removal is accidental or intentional. As ConcurrentMap does 
not require "no nulls allowed" I'm uncertain about the validity of a 
default that will only work in that case.

Aside: the indentation of the doc-comment block for Map.getOrDefault is 
wrong. (Not caused by this changeset.)

FutureTask.java: the indentation of the changed code is all wrong.

atomic/package-info.java: the URL updates (to oracle.com) have been 
regressed to java.sun.com :(

> I'm not expecting an in dept review per say. All regression tests and
> JCK will be run. I guess the main focus should be on the new CHM API's

There are a number of new APIs and they should be reviewed separately in 
my opinion.

David
-----

> -Chris.