Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (athena.apache.org: domain of andrew.pack200@gmail.com
 designates 74.125.46.152 as permitted sender)
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:references;
        b=WZa3WMbejJXhawLcmFTAVE9nRi5yMrwmMTz74vDVZ/w04pdXZWx0hnRlzZbnb2oTQD
         nWXpvcndz+neAMk+MnZCoqFoQJ9o04oRhAUQ31dSbJinnUVR85XgG2NbbzWfVWIx/02G
         FyaZxdhuOuXpnQGkqS7QvBXtU0SeflEgVF+OM=
Message-ID: <6aa6e3690807080943w7165001eh10db0f88c80ed377@mail.gmail.com>
Date: Tue, 8 Jul 2008 09:43:40 -0700
From: "Andrew Cornwall" <andrew.pack200@gmail.com>
To: dev@harmony.apache.org
Subject: Re: [jira] Commented: (HARMONY-5900) [classlib][pack200]
 CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
In-Reply-To: <6aa6e3690807080925l7dce525dl391a3245d96ece0@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="----=_Part_18218_12767693.1215535420676"
References: <55920016.1215453391789.JavaMail.jira@brutus>
	 <1125226487.1215509252982.JavaMail.jira@brutus>
	 <fc2fc95c0807080230o19800553kc5d92bfe759d51d7@mail.gmail.com>
	 <4bebff790807080241qd9c3bccl38a8be0623d6e4e@mail.gmail.com>
	 <fc2fc95c0807080400q23d286d8yf89aabc1e3563a8d@mail.gmail.com>
	 <4bebff790807080405v61667caj116e15ca1febf8da@mail.gmail.com>
	 <6aa6e3690807080925l7dce525dl391a3245d96ece0@mail.gmail.com>

------=_Part_18218_12767693.1215535420676
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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 <andrew.pack200@gmail.com>
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 <sianjanuary@googlemail.com>
>> 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 <aleksey.shipilev@gmail.com> 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) <jira@apache.org> 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
>> >
>>
>
>

------=_Part_18218_12767693.1215535420676--