[PATCH FOR REVIEW] 7036559: ConcurrentHashMap footprint and contention improvements

[Omair Majid]

Hi,

On 06/03/2013 04:26 PM, Jon VanAlten wrote:
> From: "Omair Majid" <omajid at redhat.com>
>> On 05/31/2013 05:38 PM, Andrew Hughes wrote:
>>> This fix was determined to be a prerequisite backport for the 2013-04
>>> security fixes.
>>>
>>> http://cr.openjdk.java.net/~andrew/jdk6/7036559/
>>
>> I did a diff between the result of this patch and the file in jdk7u
>> after revision 005c0c85b0de and it looks identical, except for minor
>> changes:
>>
>>> --- jdk7u/jdk/src/share/classes/java/util/concurrent/ConcurrentHashMap.java
>>> 	2013-05-31 18:32:28.179433269 -0400
>>> +++ temp/ConcurrentHashMap.java	2013-05-31 17:34:06.000000000 -0400
>>> @@ -30,7 +30,7 @@
>>>   *
>>>   * Written by Doug Lea with assistance from members of JCP JSR-166
>>>   * Expert Group and released to the public domain, as explained at
>>> - * http://creativecommons.org/publicdomain/zero/1.0/
>>> + * http://creativecommons.org/licenses/publicdomain
>>>   */
>>>  
>>>  package java.util.concurrent;
>>> @@ -1412,7 +1412,7 @@
>>>       * for each key-value mapping, followed by a null pair.
>>>       * The key-value mappings are emitted in no particular order.
>>>       */
>>> -    private void writeObject(java.io.ObjectOutputStream s) throws
>>> IOException {
>>> +    private void writeObject(java.io.ObjectOutputStream s) throws
>>> IOException  {
>>>          // force all segments for serialization compatibility
>>>          for (int k = 0; k < segments.length; ++k)
>>>              ensureSegment(k);
>>> @@ -1446,7 +1446,7 @@
>>>       */
>>>      @SuppressWarnings("unchecked")
>>>      private void readObject(java.io.ObjectInputStream s)
>>> -        throws IOException, ClassNotFoundException {
>>> +        throws IOException, ClassNotFoundException  {
>>>          s.defaultReadObject();
>>>  
>>>          // Re-initialize segments to be minimally sized, and let grow.
>>
>> I don't see any API changes in the webrev; that's good.
>>
> 
> It looks to me like those differences from cleanup changes pushed to
> jdk7u and not backported.  They certainly aren't introduced in this
> webrev.  Being as it's just minor whitespace and comment changes, I
> don't think it's worth hunting down the specific cleanup changes to
> backport from 7u, and also it doesn't seem like any reason to avoid
> backporting these ConcurrentHashMap changes.
> 

I did not mean to object to the backport; I was just curious about why
there is a small difference. I am also curious about whether we intend
to do additional backports to address the performance problems (7038542:
Small performace regression in ConcurrentHashMap on c1 since CR 703655).

As you said, though, this is not a reason to hold back this patch.
Please count me as a reviewer.

Cheers,
Omair
-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681