It's good to see it gone. Thanks! On Thu, Jul 10, 2008 at 8:21 AM, Sian January <[EMAIL PROTECTED]> wrote:
> Andrew - after your previous comment I have managed to remove all the > domain > code. Somehow I hadn't realised you initially wrote that for sorting the > constant pool, so I didn't remove it when I rewrote it. It doesn't really > improve performance at all but it's always nice to have less code :-) I've > just checked it in. > > Thanks, > > Sian > > > On 08/07/2008, 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 > > > > > > > > > > > > > -- > 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 >
