harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Cornwall" <andrew.pack...@gmail.com>
Subject Re: [jira] Commented: (HARMONY-5900) [classlib][pack200] CpBands.parseCpSignature(Ljava/io/InputStream;) is hot
Date Thu, 10 Jul 2008 16:33:52 GMT
It's good to see it gone. Thanks!

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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message