[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Request for reviews (XS): 6925573: vm/classfmt/lmt/mthnum001/mthnum00101m1/mthnum00101m1.html
From:       coleen.phillimore () oracle ! com (Coleen Phillimore)
Date:       2010-05-04 18:59:02
Message-ID: 4BE06E76.4080206 () oracle ! com
[Download RAW message or body]

Yes, this looks great.  I'll push it today.

Thanks!
Coleen

On 05/04/10 14:06, Tom Rodriguez wrote:
> Yes that looks good.  Coleen or I will push this.
> 
> tom
> 
> On May 4, 2010, at 10:27 AM, Volker Simonis wrote:
> 
> 
> > I've implemented the two specialized compare functions for narrow oops
> > as requested. I've also updated the webrev with some absolute time
> > measurements for the loading of a class with 60000 methods on
> > Solaris/SPARC. Please see:
> > 
> > http://www.progdoc.de/webrev/6925573.v2/
> > 
> > Reagrds,
> > Volker
> > 
> > PS: having two versions of the idempotent function is indeed a little
> > bit ugly. I tried to solve it with a template function but
> > unfortunately the compare functions have to be "extern C" because they
> > are called from qsort, so we can't use templates. It's probably not
> > worth thinking about it any more...
> > 
> > On Mon, May 3, 2010 at 7:36 PM, Tom Rodriguez <tom.rodriguez at oracle.com> \
> > wrote: 
> > > On May 3, 2010, at 10:29 AM, Coleen Phillimore wrote:
> > > 
> > > 
> > > > Volker,
> > > > Thank you for fixing this!  It was on my list but I hadn't gotten to it yet.
> > > > 
> > > > I thought that we'd want to have a special narrow_oop_compare_fn() so that we \
> > > > don't have to test if (UseCompressedOops) for each comparison, but your \
> > > > testing shows no penalty for the frequent global value test. 
> > > I'd considered the same issue but you'd also need \
> > > method_compare_narrow_idempotent which uglies it up a bit.  It would be nicer \
> > > in some ways though. 
> > > tom
> > > 
> > > 
> > > > To be clear though,  in your table, the -UseCompressedOops case was before \
> > > > your fix?  If so, your fix looks great, and I'd be happy to check it in after \
> > > > another review. 
> > > > Thanks,
> > > > Coleen
> > > > 
> > > > On 05/03/10 12:57, Volker Simonis wrote:
> > > > 
> > > > > Hi,
> > > > > 
> > > > > I fixed 6925573 which was caused by the Compressed Oops change which
> > > > > changed the method sorting algorithm for class methods during
> > > > > classloading from quicksort to bubblesort.
> > > > > 
> > > > > Could somebody please review the change at
> > > > > 
> > > > > http://www.progdoc.de/webrev/6925573/
> > > > > 
> > > > > and submit it if is deemed to be suitable.
> > > > > 
> > > > > Thanks,
> > > > > Volker
> > > > > 
> > > > > 
> > > 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20100504/3bc03ea7/attachment.html \



[prev in list] [next in list] [prev in thread] [next in thread]