RFR: 6988220: java.lang.ObjectName use of String.intern() causes major performance issues at scale
Olivier Lagneau
olivier.lagneau at oracle.com
Fri Feb 24 07:21:05 PST 2012
I think I have not been clear enough here.
I Agree with Eammon's argument, and anyway ok with this change.
Olivier.
Olivier Lagneau said on date 2/24/2012 12:38 PM:
> Hi Éamonn,
>
> Eamonn McManus said on date 2/23/2012 8:44 PM:
>> I am not sure it is worth the complexity of extra checks. Do you have
>> data showing that ObjectName.equals usually returns false?In a
>> successful HashMap lookup, for example, it will usually return true
>> since the equals method is used to guard against collisions, and
>> collisions are rare by design. Meanwhile, String.equals is intrinsic
>> in HotSpot so we may assume that it is highly optimized, and you are
>> giving up that optimization if you use other comparisons.
> Don't have this kind of data indeed. I don't know of any
> benchmark/data about usage of ObjectName.equals()
> in most applications. That would be needed to evaluate the exact
> impact of the change.
> And I agree with the argument that usual semantics of an equals call
> is to check for equality,
> not the difference.
>
> My argument is mainly that we are moving from comparing identity to
> equality.
> Thus there will be an impact on the throughput of equals, possibly
> impacting
> some applications.
>
> Olivier.
>
>> Éamonn
>>
>>
>> On 23 February 2012 10:52, Olivier Lagneau
>> <olivier.lagneau at oracle.com <mailto:olivier.lagneau at oracle.com>> wrote:
>>
>> Hi Frederic,
>>
>> Performance and typo comments.
>>
>> Regarding performance of ObjectName.equals method, which is
>> certainely
>> a frequent call on ObjectNames, I think that using inner fields
>> (Property array for canonical name and domain length) would be
>> more efficient
>> than using String.equals() on these potentially very long strings.
>>
>> Two differents objectNames may often have the same length with
>> different key/properties values, and may often be different only
>> on the last property of the canonical name.
>>
>> The Property array field ca_array (comparing length and property
>> contents)
>> and domain length are good candidates to filter out more efficiently
>> different objectNames, knowing that String.equals will compare every
>> single char of the two char arrays.
>>
>> So for performance purpose, I suggest to filter out different
>> objectNames
>> by doing inner comparisons in the following order : length of the
>> two
>> canonical names, then domain_length, then ca_array size, then its
>> content,
>> and lastly if all of this fails to filter out, then use
>> String.equals.
>>
>> _canonicalName = (new String(canonical_chars, 0, prop_index));
>>
>> Typo : useless parentheses.
>>
>> Thanks,
>> Olivier.
>>
>> -- Olivier Lagneau, olivier.lagneau at oracle.com
>> <mailto:olivier.lagneau at oracle.com>
>> Oracle, Grenoble Engineering Center - France
>> Phone : +33 4 76 18 80 09 <tel:%2B33%204%2076%2018%2080%2009> Fax
>> : +33 4 76 18 80 23 <tel:%2B33%204%2076%2018%2080%2023>
>>
>>
>>
>>
>> Frederic Parain said on date 2/23/2012 6:01 PM:
>>
>> No particular reason. But after thinking more about it,
>> equals() should be a better choice, clearer code, and
>> the length check in equals() implementation is likely
>> to help performance of ObjectName's comparisons as
>> ObjectNames are often long with a common section at the
>> beginning.
>>
>> I've updated the webrev:
>> http://cr.openjdk.java.net/~fparain/6988220/webrev.01/
>> <http://cr.openjdk.java.net/%7Efparain/6988220/webrev.01/>
>>
>> Thanks,
>>
>> Fred
>>
>> On 2/23/12 4:58 PM, Vitaly Davidovich wrote:
>>
>> Hi Frederic,
>>
>> Just curious - why are you checking string equality via
>> compareTo()
>> instead of equals()?
>>
>> Thanks
>>
>> Sent from my phone
>>
>> On Feb 23, 2012 10:37 AM, "Frederic Parain"
>> <frederic.parain at oracle.com
>> <mailto:frederic.parain at oracle.com>
>> <mailto:frederic.parain at oracle.com
>> <mailto:frederic.parain at oracle.com>>> wrote:
>>
>> This a simple fix to solve CR 6988220:
>>
>> http://bugs.sun.com/__bugdatabase/view_bug.do?bug___id=6988220
>> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6988220>
>>
>> The use of String.intern() in the ObjectName class
>> prevents
>> the class the scale well when more than 20K
>> ObjectNames are
>> managed. The fix simply removes the use of
>> String.intern(),
>> and uses regular String instead. The Object.equals()
>> method
>> is modified too to make a regular String comparison. The
>> complexity of this method now depends on the length of
>> the ObjectName's canonical name, and is not impacted any
>> more by the number of ObjectName instances being handled.
>>
>> The Webrev:
>> http://cr.openjdk.java.net/~__fparain/6988220/webrev.00/
>> <http://cr.openjdk.java.net/%7E__fparain/6988220/webrev.00/>
>> <http://cr.openjdk.java.net/~fparain/6988220/webrev.00/
>> <http://cr.openjdk.java.net/%7Efparain/6988220/webrev.00/>>
>>
>> I've tested this fix with the jdk_lang and jdk_management
>> test suites.
>>
>> Thanks,
>>
>> Fred
>>
>> --
>> Frederic Parain - Oracle
>> Grenoble Engineering Center - France
>> Phone: +33 4 76 18 81 17
>> <tel:%2B33%204%2076%2018%2081%2017>
>> <tel:%2B33%204%2076%2018%2081%2017>
>> Email: Frederic.Parain at oracle.com
>> <mailto:Frederic.Parain at oracle.com>
>> <mailto:Frederic.Parain at oracle.com
>> <mailto:Frederic.Parain at oracle.com>>
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120224/b3d59131/attachment-0001.html
More information about the serviceability-dev
mailing list