'RFR: 6988220: java.lang.ObjectName use of String.intern() causes major performance issues at scale'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    RFR: 6988220: java.lang.ObjectName use of String.intern() causes major performance issues at scale
From:       eamonn () mcmanus ! net (Eamonn McManus)
Date:       2012-02-24 16:06:55
Message-ID: CACBEn44RAOopNwr=R3CpgFXXCxbGg1VE22y-UD7VuEC9d7Px+w () mail ! gmail ! com
[Download RAW message or body]

Hi Dan,

I got a bounce from serviceability-dev because I wasn't subscribed to
it, but the message went out to core-libs-dev because I was subscribed
to that. That probably explains what you saw.

Regards,
?amonn


On 24 February 2012 09:33, Daniel D. Daugherty
<daniel.daugherty at oracle.com> wrote:
> 
> Just FYI: I haven't seen ?amonn's posting come in. Just replies to
> his posting. This may mean that other comments are stuck in the
> ether somewhere...
> 
> I suspect that the OpenJDK list server is again having issues...
> 
> Dan
> 
> 
> On 2/24/12 8:21 AM, Olivier Lagneau wrote:
> 
> 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>>
> 
> 
> 
> 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic