Andrew, There's a hint - search() called for several distinct String[] arrays. Please see my patch, it eliminates search() at all, there you can find the search() consumers.
Thanks, Aleksey. On Tue, Jul 8, 2008 at 8:43 PM, Andrew Cornwall <[EMAIL PROTECTED]> wrote: > I applied Sian's patch from HARMONY-5900, but that doesn't appear to change > the runtime characteristics very much. In particular, it appears that > CpBands.search is still being called - did I miss something in the patch? > > Andrew Jr. > > On Tue, Jul 8, 2008 at 9:25 AM, Andrew Cornwall <[EMAIL PROTECTED]> > wrote: > >> Sian, are we still using the DOMAIN_* code for ordering? I thought you'd >> changed the code to use the ordering defined in the pack200 archive? If so, >> we might be able to get rid of all the DOMAIN_ code. >> >> Andrew Jr. >> >> >> >> On Tue, Jul 8, 2008 at 4:05 AM, Aleksey Shipilev < >> [EMAIL PROTECTED]> wrote: >> >>> Ah! IMO, that's ok in terms of performance. >>> Anyway, we can implement Map storage for CpSignatures and have no >>> degradation at all. >>> >>> Let's wait for Andrew. >>> >>> Thanks, >>> Aleksey. >>> >>> On Tue, Jul 8, 2008 at 3:00 PM, Sian January <[EMAIL PROTECTED]> >>> wrote: >>> > I posted a suggested alternative on the JIRA, which doesn't do this >>> search >>> > at all and just uses the same objects for CpSignatures that were >>> transmitted >>> > in CpUtf8 if they exist. The downside of this is that >>> > CpBands.cpSignatureValue(..) >>> > will always be searching a much larger map, so I'm not sure if there are >>> > performance implications from that. I'm also not 100% sure that we >>> don't >>> > need the code I've taken out, but Andrew might be able to answer that. >>> > >>> > On 08/07/2008, Aleksey Shipilev <[EMAIL PROTECTED]> wrote: >>> >> >>> >> This method is the hell for performance. >>> >> It is not only accounts for 15% of CPU time, but instrumentation also >>> >> shows: >>> >> - average size of array is 4700 elements >>> >> - 99% times the search traverses entire array and finds nothing >>> >> >>> >> We should consider move to *Map here :) >>> >> >>> >> Thanks, >>> >> Aleksey. >>> >> >>> >> On Tue, Jul 8, 2008 at 1:30 PM, Sian January < >>> [EMAIL PROTECTED]> >>> >> wrote: >>> >> > The purpose of this code is to get the class constant pool ordering >>> >> right, >>> >> > so that if a signature string has already been transmitted in the >>> CpUtf8 >>> >> > band it has the correct global index. However it might be possible >>> to do >>> >> > the search a different way if this method is taking a long time. E.g >>> if >>> >> I >>> >> > get rid of the code that differentiates between signatures and other >>> >> Ascii >>> >> > values (i.e. replace all occurrences of >>> >> > ClassConstantPool.DOMAIN_SIGNATUREASCIIZ >>> >> > with ClassConstantPool.DOMAIN_NORMALASCIIZ) then my tests seem to >>> pass, >>> >> > although Andrew you wrote that code so it might be that I'm not >>> testing >>> >> all >>> >> > the cases where this is needed. Also there might be worse >>> performance >>> >> > implications from making that change, but I can attach a patch to the >>> >> JIRA >>> >> > if you would like to test it. >>> >> > >>> >> > >>> >> > >>> >> > On 08/07/2008, Aleksey Shipilev (JIRA) <[EMAIL PROTECTED]> wrote: >>> >> >> >>> >> >> >>> >> >> [ >>> >> >> >>> >> >>> https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611488#action_12611488 >>> >> ] >>> >> >> >>> >> >> Aleksey Shipilev commented on HARMONY-5900: >>> >> >> ------------------------------------------- >>> >> >> >>> >> >> From my measurements for 20Mb .jar.pack.gz file on Sun JDK 1.6.0_05 >>> >> >> -server, CPBands.search accounts for 15% of CPU time. >>> >> >> Let's move the discussion to dev. >>> >> >> >>> >> >> > [classlib][pack200] >>> CpBands.parseCpSignature(Ljava/io/InputStream;) is >>> >> >> hot >>> >> >> > >>> >> >> >>> >> >>> -------------------------------------------------------------------------- >>> >> >> > >>> >> >> > Key: HARMONY-5900 >>> >> >> > URL: >>> >> https://issues.apache.org/jira/browse/HARMONY-5900 >>> >> >> > Project: Harmony >>> >> >> > Issue Type: Wish >>> >> >> > Components: Classlib >>> >> >> > Affects Versions: 5.0M6 >>> >> >> > Environment: All Pack200 HEAD >>> >> >> > Reporter: Andrew Cornwall >>> >> >> > >>> >> >> > The method >>> >> >> >>> >> >>> org/apache/harmony/unpack200/CpBands.parseCpSignature(Ljava/io/InputStream;) >>> >> >> appears to be very hot. I tried initially to optimize it by caching >>> some >>> >> of >>> >> >> its arrays: >>> >> >> > static void clearArrayCache() { >>> >> >> > arrayCache = new SegmentConstantPoolArrayCache(); >>> >> >> > } >>> >> >> > >>> >> >> > private static SegmentConstantPoolArrayCache arrayCache = new >>> >> >> SegmentConstantPoolArrayCache(); >>> >> >> > >>> >> >> > private int search(String[] array, String string) { >>> >> >> > if(array.length > 30) { >>> >> >> > List indexes = arrayCache.indexesForArrayKey(array, >>> >> >> string); >>> >> >> > if (indexes.size() == 0) { >>> >> >> > return -1; >>> >> >> > } >>> >> >> > return ((Integer)indexes.get(0)).intValue(); >>> >> >> > } else { >>> >> >> > for (int i = 0; i < array.length; i++) { >>> >> >> > if(array[i].equals(string)) { >>> >> >> > return i; >>> >> >> > } >>> >> >> > } >>> >> >> > return -1; >>> >> >> > } >>> >> >> > } >>> >> >> > ... but that didn't appear to increase performance. (Maybe all the >>> >> >> searches are done once?) >>> >> >> > Any ideas how to tune parseCpSignature to get it faster? >>> >> >> >>> >> >> -- >>> >> >> This message is automatically generated by JIRA. >>> >> >> - >>> >> >> You can reply to this email to add a comment to the issue online. >>> >> >> >>> >> >> >>> >> > >>> >> > >>> >> > -- >>> >> > Unless stated otherwise above: >>> >> > IBM United Kingdom Limited - Registered in England and Wales with >>> number >>> >> > 741598. >>> >> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire >>> PO6 >>> >> 3AU >>> >> > >>> >> >>> > >>> > >>> > >>> > -- >>> > Unless stated otherwise above: >>> > IBM United Kingdom Limited - Registered in England and Wales with number >>> > 741598. >>> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 >>> 3AU >>> > >>> >> >> >
