Updated webrev:
http://cr.openjdk.java.net/~mduigou/JDK-8024688/3/webrev/
More notes below.
On Oct 16 2013, at 21:20 , David Holmes <david.hol...@oracle.com> wrote:
> Hi Mike,
>
> Map.java:
>
> The implNote for computeIfAbsent should be modified to match the
> implementation. Ditto for computeIfPresent. Ditto for compute, merge etc!
> Once these implementations have stabilized we need to check what the implNote
> says. It makes no sense to me for the impl note to describe anything other
> than the core logic of the actual implementation - particularly referring to
> putIfAbsent when put is used, or replace when put is used.
While removing the CME I opted to also remove some of use of the fancy new
methods as they didn't add anything and (usually) had a performance cost.
I will update the @implNote descriptions and review the other notes.
>
> HashMap.java:
>
> 1234 if(old.value != null)
>
> Space
Got it. Thank you.
>
>
> ConcurrentMap.java:
>
> 247 * @throws ClassCastException {@inheritDoc}
> 248 * @throws NullPointerException {@inheritDoc}
> 249 * @throws ClassCastException {@inheritDoc}
>
> CCE is repeated.
Got it.
>
> 274 * contain null values and get() returning null unambiguously means
> the key
>
> get() should be in code font as per line #69. Ditto for line 300, 332 and 395.
And a few more. Thanks.
>
> I now see that those Map implNotes were written with ConcurrentMap in mind so
> that it can just tag on the note about retries. But this seems wrong to me -
> each should have its own implNotes reflecting the true implementation.
>
>> It does bother me to be throwing out "good information" by not throwing the
>> CMEs but I'm willing to go with the flow. As a practical matter later
>> reintroduction of even valid error detection would almost certainly be
>> difficult. (https://bugs.openjdk.java.net/browse/JDK-5045147 for one
>> example).
>
> ... it still concerns me that a function object could mutate the map and so
> trigger a CCE.
There seems to be no way to prevent it.
>
>> The patch also fixes up missing @throws and @since from the ConcurrentMap
>> implementations.
>
> I think you have a one-line conflict with Henry's latest @since update.
Got it. I had planned for that fix to go in with changeset....
>
> Cheers,
> David
>
>> Mike
>>
