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 <[email protected]> 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 <[email protected] > <mailto:[email protected]>> 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, [email protected] > <mailto:[email protected]> > 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" > <[email protected] > <mailto:[email protected]> > <mailto:[email protected] > <mailto:[email protected]>>> 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: [email protected] > <mailto:[email protected]> > <mailto:[email protected] > <mailto:[email protected]>> > > > > > >
