You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Andrew Cornwall (JIRA)" <ji...@apache.org> on 2008/07/07 19:56:31 UTC
[jira] Created: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
[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.
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Aleksey Shipilev <al...@gmail.com>.
Well, I have a quick patch with moving to HashMap. It gives +8% to
unpacking scenario, available in JIRA.
Thanks,
Aleksey.
On Tue, Jul 8, 2008 at 1:41 PM, Aleksey Shipilev
<al...@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 <si...@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) <ji...@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
>>
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Andrew Cornwall <an...@gmail.com>.
It's good to see it gone. Thanks!
On Thu, Jul 10, 2008 at 8:21 AM, Sian January <si...@googlemail.com>
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 <an...@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 <al...@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) <ji...@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
> > > >
> > >
> >
>
>
>
> --
> 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
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Sian January <si...@googlemail.com>.
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 <an...@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 <al...@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) <ji...@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
> > >
> >
>
--
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
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Aleksey Shipilev <al...@gmail.com>.
Andrew,
Sorry, there was a glitch in the patch: index is -1 always there.
The change is pretty trivial but I attached updated patch in HARMONY-5900.
Thanks,
Aleksey.
On Wed, Jul 9, 2008 at 3:21 AM, Andrew Cornwall
<an...@gmail.com> wrote:
> Errm, I might take that back. I'm seeing a failure in
> ArchiveTest.testWithAnnotations:
>
> java.lang.NullPointerException
> at
> org.apache.harmony.unpack200.CpBands.cpNameAndTypeValue(CpBands.java:776)
> at
> org.apache.harmony.unpack200.MetadataBandGroup.getNextValue(MetadataBandGroup.java:225)
> at
> org.apache.harmony.unpack200.MetadataBandGroup.getAnnotation(MetadataBandGroup.java:199)
> at
> org.apache.harmony.unpack200.MetadataBandGroup.getAttribute(MetadataBandGroup.java:167)
> at
> org.apache.harmony.unpack200.MetadataBandGroup.getAttributes(MetadataBandGroup.java:140)
> at
> org.apache.harmony.unpack200.ClassBands.parseClassMetadataBands(ClassBands.java:1204)
> at
> org.apache.harmony.unpack200.ClassBands.parseClassAttrBands(ClassBands.java:518)
> at org.apache.harmony.unpack200.ClassBands.unpack(ClassBands.java:152)
> at org.apache.harmony.unpack200.Segment.parseSegment(Segment.java:385)
> at org.apache.harmony.unpack200.Segment.unpack(Segment.java:405)
> at org.apache.harmony.unpack200.Archive.unpack(Archive.java:159)
> at
> org.apache.harmony.unpack200.tests.ArchiveTest.testWithAnnotations(ArchiveTest.java:147)
>
> Looks as if something's missing: the compiler is flagging:
> cpNameAndType = new CPNameAndType(name, descriptorU, domain,
> index.intValue() + descrOffset);
>
> with: "The variable index can only be null at this location".
>
> Andrew Jr.
>
> On Tue, Jul 8, 2008 at 10:03 AM, Andrew Cornwall <an...@gmail.com>
> wrote:
>
>> I guess maybe Sian's patch wasn't meant to get search out, but just test if
>> the CPUTF8 signatures appeared in the right place? If so, they do - I've
>> just confirmed it. I think the signature code may be obsolete. Does this
>> mean we can get rid of it? Do we need to do the search (or Aleksey's
>> equvialent) if we don't need to distinguish different kinds of CPUTF8?
>>
>> Aleksey, I can confirm your patch provides a performance boost and does not
>> appear to change any behaviour in unpacking.
>>
>> Andrew Jr.
>>
>>
>> On Tue, Jul 8, 2008 at 9:48 AM, Aleksey Shipilev <
>> aleksey.shipilev@gmail.com> wrote:
>>
>>> 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
>>> <an...@gmail.com> 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 <
>>> 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 <al...@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) <ji...@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
>>> >>> >
>>> >>>
>>> >>
>>> >>
>>> >
>>>
>>
>>
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Andrew Cornwall <an...@gmail.com>.
Errm, I might take that back. I'm seeing a failure in
ArchiveTest.testWithAnnotations:
java.lang.NullPointerException
at
org.apache.harmony.unpack200.CpBands.cpNameAndTypeValue(CpBands.java:776)
at
org.apache.harmony.unpack200.MetadataBandGroup.getNextValue(MetadataBandGroup.java:225)
at
org.apache.harmony.unpack200.MetadataBandGroup.getAnnotation(MetadataBandGroup.java:199)
at
org.apache.harmony.unpack200.MetadataBandGroup.getAttribute(MetadataBandGroup.java:167)
at
org.apache.harmony.unpack200.MetadataBandGroup.getAttributes(MetadataBandGroup.java:140)
at
org.apache.harmony.unpack200.ClassBands.parseClassMetadataBands(ClassBands.java:1204)
at
org.apache.harmony.unpack200.ClassBands.parseClassAttrBands(ClassBands.java:518)
at org.apache.harmony.unpack200.ClassBands.unpack(ClassBands.java:152)
at org.apache.harmony.unpack200.Segment.parseSegment(Segment.java:385)
at org.apache.harmony.unpack200.Segment.unpack(Segment.java:405)
at org.apache.harmony.unpack200.Archive.unpack(Archive.java:159)
at
org.apache.harmony.unpack200.tests.ArchiveTest.testWithAnnotations(ArchiveTest.java:147)
Looks as if something's missing: the compiler is flagging:
cpNameAndType = new CPNameAndType(name, descriptorU, domain,
index.intValue() + descrOffset);
with: "The variable index can only be null at this location".
Andrew Jr.
On Tue, Jul 8, 2008 at 10:03 AM, Andrew Cornwall <an...@gmail.com>
wrote:
> I guess maybe Sian's patch wasn't meant to get search out, but just test if
> the CPUTF8 signatures appeared in the right place? If so, they do - I've
> just confirmed it. I think the signature code may be obsolete. Does this
> mean we can get rid of it? Do we need to do the search (or Aleksey's
> equvialent) if we don't need to distinguish different kinds of CPUTF8?
>
> Aleksey, I can confirm your patch provides a performance boost and does not
> appear to change any behaviour in unpacking.
>
> Andrew Jr.
>
>
> On Tue, Jul 8, 2008 at 9:48 AM, Aleksey Shipilev <
> aleksey.shipilev@gmail.com> wrote:
>
>> 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
>> <an...@gmail.com> 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 <
>> 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 <al...@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) <ji...@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
>> >>> >
>> >>>
>> >>
>> >>
>> >
>>
>
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Andrew Cornwall <an...@gmail.com>.
I guess maybe Sian's patch wasn't meant to get search out, but just test if
the CPUTF8 signatures appeared in the right place? If so, they do - I've
just confirmed it. I think the signature code may be obsolete. Does this
mean we can get rid of it? Do we need to do the search (or Aleksey's
equvialent) if we don't need to distinguish different kinds of CPUTF8?
Aleksey, I can confirm your patch provides a performance boost and does not
appear to change any behaviour in unpacking.
Andrew Jr.
On Tue, Jul 8, 2008 at 9:48 AM, Aleksey Shipilev <al...@gmail.com>
wrote:
> 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
> <an...@gmail.com> 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 <
> 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 <al...@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) <ji...@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
> >>> >
> >>>
> >>
> >>
> >
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Aleksey Shipilev <al...@gmail.com>.
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
<an...@gmail.com> 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 <an...@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 <si...@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 <al...@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) <ji...@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
>>> >
>>>
>>
>>
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Andrew Cornwall <an...@gmail.com>.
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 <an...@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 <si...@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 <al...@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) <ji...@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
>> >
>>
>
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Andrew Cornwall <an...@gmail.com>.
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 <al...@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 <si...@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 <al...@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) <ji...@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
> >
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Aleksey Shipilev <al...@gmail.com>.
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 <si...@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 <al...@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 <si...@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) <ji...@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
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Sian January <si...@googlemail.com>.
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 <al...@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 <si...@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) <ji...@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
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Aleksey Shipilev <al...@gmail.com>.
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 <si...@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) <ji...@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
>
Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by Sian January <si...@googlemail.com>.
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) <ji...@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
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611310#action_12611310 ]
Aleksey Shipilev commented on HARMONY-5900:
-------------------------------------------
Andrew, I see there's a speculation over the hotness of this method :)
Is there any benchmark to show that this particular method is hot? If yes, then the optimization worth it.
BTW, having a quick look in pack200 code, I suspect it's the natural design choice, though not the optimal one.
> [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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Nathan Beyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611431#action_12611431 ]
Nathan Beyer commented on HARMONY-5900:
---------------------------------------
The use of a String[] instead of a HashMap should be pragmatic based on both efficiency and resources. As a general rule if you have less than 100 items the efficiency of item retrieval/insertion isn't even trivially better with a HashMap/Set, so the resource overhead of HashMap/Set isn't worth the trouble.
What's execution profile of the array? How large does it normally get?
> [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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andrew Cornwall updated HARMONY-5900:
-------------------------------------
Attachment: (was: unified-stringlookup-noSearchForSignautres.patch)
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612547#action_12612547 ]
Sian January commented on HARMONY-5900:
---------------------------------------
Applied unified patch at r675616. Andrew and Aleksey - are you both happy for me to resolve this issue?
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sian January updated HARMONY-5900:
----------------------------------
Attachment: pack200.patch
Attached a patch that could be used for testing a potential solution.
> [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
> Attachments: pack200.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sian January updated HARMONY-5900:
----------------------------------
Attachment: (was: pack200.patch)
> [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
> Attachments: pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611318#action_12611318 ]
Andrew Cornwall commented on HARMONY-5900:
------------------------------------------
(These numbers are ticks and %ticks). And right below them is:
63 2.4% java/io/DataOutputStream.writeChar(I)V_4a21ad7c
55 2.1% org/apache/harmony/unpack200/CpBands.search([Ljava/lang/String;Ljava/lang/String;)I_41187efc
> [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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611485#action_12611485 ]
Sian January commented on HARMONY-5900:
---------------------------------------
Can we move this discussion to the dev list please?
> [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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sian January updated HARMONY-5900:
----------------------------------
Comment: was deleted
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611305#action_12611305 ]
Andrew Cornwall commented on HARMONY-5900:
------------------------------------------
Good question - I didn't write the code, and I'm hoping Sian did :-) I tried a HashMap with my SegmentConstantPoolArrayCache code above, but that didn't seem to give me any speedup. (No matter whether I used it for all arrays or just large arrays, the time it took was about the same as the time for a linear search - hence my suspicion that many arrays are searched infrequently. Or I might have made an error because I'm not too familiar with CpBands other than as a consumer.)
> [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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Aleksey Shipilev updated HARMONY-5900:
--------------------------------------
Attachment: pack200-stringlookup-v1.patch
pack200-stringlookup-v1.patch
Updated, should fix NPE on ArchiveTest.testWithAnnotations.
> [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
> Attachments: pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, pack200.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612101#action_12612101 ]
Sian January commented on HARMONY-5900:
---------------------------------------
Sorry - ignore my patch - it doesn't do what I thought it did. I will think again about if we can avoid this search completely, but if not it would be good to go with something like Aleksey's solution.
> [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
> Attachments: pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, pack200.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Aleksey Shipilev updated HARMONY-5900:
--------------------------------------
Attachment: pack200-stringlookup-v1.patch
pack200-stringlookup-v1.patch
The patch moves out search() usage to HashMap.get().
This patch is proof-of-concept, though there is just a little polishing to production grade.
Gives +8% on unpacking scenario (unpacking of 40 Mb Eclipse JDT jars) on Core2Duo: 68 secs vs 74 secs.
> [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
> Attachments: pack200-stringlookup-v1.patch, pack200.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sian January updated HARMONY-5900:
----------------------------------
Attachment: pack200-noSearchForSignatures.patch
Ok - I've just attached pack200-noSearchForSignatures.patch, which does what I thought the other one did, i.e. doesn't call search from parseCpSignature(). I haven't got rid of the other calls to search in this patch because I think it only happens a very few times so I'm not sure if it's worth creating a map for. That might be worth testing though.
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612546#action_12612546 ]
Sian January commented on HARMONY-5900:
---------------------------------------
Applied unified patch at r675615. Andrew and Aleksey - are you both happy for me to resolve this issue?
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611315#action_12611315 ]
Andrew Cornwall commented on HARMONY-5900:
------------------------------------------
A colleague of mine ran tprof on the IBM VM with the Harmony Pack200 unpack code and got the following results for top JITted methods:
211 8.1% java/util/zip/Deflater.setInputImpl([BIIJ)V_411926f4
177 6.8% java/util/zip/Deflater.deflateImpl([BIIJI)I_41192854
149 5.7% java/io/DataOutputStream.writeChar(I)V_4a23a230
97 3.7% java/util/zip/DeflaterOutputStream.write([BII)V_4a1fffc6
94 3.6% org/apache/harmony/unpack200/CpBands.parseCpSignature(Ljava/io/InputStream;)V_4a23c0be
77 2.9% org/apache/harmony/pack200/BHSDCodec.decode(Ljava/io/InputStream;J)J_4118519c
73 2.8% org/apache/harmony/unpack200/bytecode/ClassConstantPool.add(Lorg/apache/harmony/unpack200/bytecode/ClassFileEntry;)Lorg/apache/harmony/unpack200/bytecode/ClassFileEntry;_4a231b1c
67 2.6% org/apache/harmony/unpack200/bytecode/ByteCode.doWrite(Ljava/io/DataOutputStream;)V_4a218cbc
65 2.5% org/apache/harmony/unpack200/CpBands.cpUTF8Value(Ljava/lang/String;IZ)Lorg/apache/harmony/unpack200/bytecode/CPUTF8;_411fc606
64 2.4% org/apache/harmony/unpack200/CpBands.parseCpSignature(Ljava/io/InputStream;)V_4a208866
63 2.4% java/io/DataOutputStream.writeChar(I)V_4a21ad7c
I suspect HARMONY-5888 and HARMONY-5889 will help with the stream methods, which weren't buffered when the test was run. After that, parseCpSignature and BHSDCodec.decode are next on the list of top methods.
> [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.
[jira] Resolved: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Sian January (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sian January resolved HARMONY-5900.
-----------------------------------
Resolution: Fixed
Fix Version/s: 5.0M7
Resolved at r675616.
> [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
> Fix For: 5.0M7
>
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611273#action_12611273 ]
Aleksey Shipilev commented on HARMONY-5900:
-------------------------------------------
Andrew, why does searching is done over String[] anyway? You get O(n) here.
Is there any opportunity to use HashMap/TreeMap?
> [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.
[jira] Closed: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Tim Ellison (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tim Ellison closed HARMONY-5900.
--------------------------------
Everyone appears happy!
> [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
> Fix For: 5.0M7
>
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andrew Cornwall updated HARMONY-5900:
-------------------------------------
Attachment: unified-stringlookup-noSearchForSignatures.patch
(Updated unified-stringlookup-noSearchForSignatures.patch to remove an extra change I'd introduced unrelated to this issue.)
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Aleksey Shipilev (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612550#action_12612550 ]
Aleksey Shipilev commented on HARMONY-5900:
-------------------------------------------
I'm happy. Thanks. :)
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.
[jira] Updated: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andrew Cornwall updated HARMONY-5900:
-------------------------------------
Attachment: unified-stringlookup-noSearchForSignautres.patch
With Sian's patch applied, we still end up doing 23182 searches in my test case (which has 466 plugins). I unified the two patches to produce the attached patch, which both eliminates all searches and eliminates unnecessary signature lookups. It runs a little faster than either patch on its own.
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignautres.patch
>
>
> 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.
[jira] Commented: (HARMONY-5900) [classlib][pack200]
CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Posted by "Andrew Cornwall (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-5900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612556#action_12612556 ]
Andrew Cornwall commented on HARMONY-5900:
------------------------------------------
I'm happy too. Thanks!
> [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
> Attachments: pack200-noSearchForSignatures.patch, pack200-stringlookup-v1.patch, pack200-stringlookup-v1.patch, unified-stringlookup-noSearchForSignatures.patch
>
>
> 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.