Hi Chris & Doug;

- I don't feel strongly about the removal of AbstractMap. I don't see this as 
very likely to cause problems in real world code though there is probably some 
test code somewhere that assigns CHM to an AbstractMap.


- I don't see the advantage to exposing the ConcurrentHashMap.KeySetView type 
particularly for newKeySet(). Why not return Set<K>? The additional methods 
don't seem to offer much that's desirable for the newKeySet() case.


- I am reluctant to deprecate contains(Object) here unless we deprecate it in 
Hashtable as well. I recognize that this has been a source of errors 
(https://issues.apache.org/bugzilla/show_bug.cgi?id=48755 for one example). Is 
it time to deprecate it there as well?


- I think there could be more complete description of the parallelismThreshold 
and interaction with common pool. i.e. does "maximal parallelism" mean one 
thread per element or "size() / getCommonPoolParallelism()". Some advice for 
choosing in-between values would be good unless "1" is the best advice for 
cases where you just don't know. It would be a shame to see people shooting 
themselves in the foot with this.

Mike


On May 27 2013, at 07:30 , Chris Hegarty wrote:

> Since my previous failed attempt to update the j.u.c. world, this review is 
> for the update to j.u.c.ConcurrentHashMap v8 from Doug's CVS.
> 
> http://cr.openjdk.java.net/~chegar/8005704/ver.00/specdiff/java/util/concurrent/package-summary.html
> http://cr.openjdk.java.net/~chegar/8005704/ver.00/webrev/
> 
> A few initial comments:
> 
> 1) CHM no longer extends AbstractMap. I guess this should not be a
>   problem in the real world, and I guess users would not be too
>   surprised by instanceof checks. Just worth highlighting the change
>   for compatibility.
> 
> 2) KeySetView.spliterator()
> 
>   I guess the API should also report CONCURRENT, NONNULL & SUBSIZED?
>   And the implementation should return SIZED too?
> 
> 3) Value/EntrySpliterator.spliterator() should return SIZED?
> 
> 4) Does is make sense for KeySetView to be Serializable? It looks a
>   little odd with value as its only field.
> 
> -Chris.

Reply via email to