From dev-return-34534-apmail-harmony-dev-archive=harmony.apache.org@harmony.apache.org Tue Jul 08 16:49:06 2008 Return-Path: Delivered-To: apmail-harmony-dev-archive@www.apache.org Received: (qmail 97980 invoked from network); 8 Jul 2008 16:49:06 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 8 Jul 2008 16:49:06 -0000 Received: (qmail 69780 invoked by uid 500); 8 Jul 2008 16:49:04 -0000 Delivered-To: apmail-harmony-dev-archive@harmony.apache.org Received: (qmail 69739 invoked by uid 500); 8 Jul 2008 16:49:04 -0000 Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@harmony.apache.org Delivered-To: mailing list dev@harmony.apache.org Received: (qmail 69728 invoked by uid 99); 8 Jul 2008 16:49:04 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 Jul 2008 09:49:04 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of aleksey.shipilev@gmail.com designates 209.85.198.244 as permitted sender) Received: from [209.85.198.244] (HELO rv-out-0708.google.com) (209.85.198.244) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 Jul 2008 16:48:13 +0000 Received: by rv-out-0708.google.com with SMTP id k29so2693338rvb.0 for ; Tue, 08 Jul 2008 09:48:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=Lnyw9l8HQRetSkOGfCvy+mBcFpSAysarBL6HS03GwMU=; b=ICPdfj/fNO1ZTHxpYEcnOCGCAPz18qMzrVP4f007HsvUmFemdMSDPp9MDm1AuHNnSs aucgc9vJS/PFS1jNvlUDkg4oM38cquFcYBVlEGIMgK/HjlerBFz37O2YqEmMSCT9bCF0 33ZY1/nCY72qTuR3DoqYV6TeGoUQ01KAR0ZAo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=Ov4DwzY1yw/k7USyF3GaliNuog6Ar+UHt7zgK7f3VtzGUXz64+obCjNcnzwsAB8wwm PKE9qRvfg9Jk0VfO5zH4fniRYKA3U3QK1C8uvbI0rhVumXG+Y+kHTfiFEq/WPYbbcxSo OCcLHveUlEnwwgTtR7aKH+Jt/gL8wObXyYtT8= Received: by 10.141.170.10 with SMTP id x10mr3333477rvo.105.1215535715008; Tue, 08 Jul 2008 09:48:35 -0700 (PDT) Received: by 10.141.51.9 with HTTP; Tue, 8 Jul 2008 09:48:34 -0700 (PDT) Message-ID: <4bebff790807080948q37b075dejc91e9922c0d39d22@mail.gmail.com> Date: Tue, 8 Jul 2008 20:48:34 +0400 From: "Aleksey Shipilev" To: dev@harmony.apache.org Subject: Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot In-Reply-To: <6aa6e3690807080943w7165001eh10db0f88c80ed377@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <55920016.1215453391789.JavaMail.jira@brutus> <1125226487.1215509252982.JavaMail.jira@brutus> <4bebff790807080241qd9c3bccl38a8be0623d6e4e@mail.gmail.com> <4bebff790807080405v61667caj116e15ca1febf8da@mail.gmail.com> <6aa6e3690807080925l7dce525dl391a3245d96ece0@mail.gmail.com> <6aa6e3690807080943w7165001eh10db0f88c80ed377@mail.gmail.com> X-Virus-Checked: Checked by ClamAV on apache.org 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 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 > 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 < >> aleksey.shipilev@gmail.com> 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 >>> 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 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 < >>> sianjanuary@googlemail.com> >>> >> 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) 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 >>> > >>> >> >> >