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

[Chris Hegarty]

Mike, David,

Given your feedback I'll slice'n'dice, where appropriate, and try to 
make this more manageable for review. I'll follow up with separate 
review emails, taking into account your comments.

-Chris.

On 27/05/2013 08:10, David Holmes wrote:
> 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.