Some notes:

- The Object padding (pad10 - pad1d) in WorkQueue and ForkJoinPool is sensitive 
to reference size and compressed OOPS. I was surprised to see Object used 
rather than long or int.

- how is getCommonPoolParallelism() different from 
commonPool().getParallelism() ? Seems redundant.

- I would document the effects of shutdown, shutdownNow, awaitTermination, 
isTerminating specific to the common pool on getCommonPool() rather than on the 
individual methods. The discussion seems out of place on the individual 
methods. Everything about the common pool should be consolidated (or 
replicated) on getCommonPool() for one-stop-shopping to understand the 
characteristics of the common pool.

- ForkJoinTask - "If the current thread is operating in a ForkJoinPool," and 
similar phrases. It doesn't say what happens if it isn't.

- ForkJoinTask - removing "This method may be invoked only from within 
ForkJoinPool computations (as may be determined using method inForkJoinPool()). 
Attempts to invoke in other contexts result in exceptions or errors, possibly 
including ClassCastException." etc. would seem to open up a lot of ambiguity 
about what happens when methods are called out of context.

- CountedCompleter : "Asuuming" -> "Assuming"

I didn't see anything that rang warning bells.

Mike

On Dec 13 2012, at 08:53 , Chris Hegarty wrote:

> 
> I would like to re-start [1] review discussion of the changes to ForkJoinXXX 
> ( add a default common pool, task tags, and implementation updates ), and the 
> addition of CountedCompleter, as part of part of JEP 155 [2].
> 
> These changes are of course coming form Doug and the JSR 166 EG members. I 
> have done a quick sanity review, and the changes reflect what is in Doug's 
> CVS. I need to spend more time reviewing myself, others on the list have more 
> experience with these new API already and may have valuable feedback.
> 
> specdiff:
> http://cr.openjdk.java.net/~chegar/8002356/ver.00/specdiff/java/util/concurrent/package-summary.html
> 
> webrev:
>  http://cr.openjdk.java.net/~chegar/8002356/ver.00/webrev/webrev/
> 
> -Chris.
> 
> [1] http://openjdk.java.net/jeps/155
> [2] 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-November/012153.html

Reply via email to