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

List:       openjdk-hotspot-runtime-dev
Subject:    Re: Request for review: 8009778: NPG: ClassMetaspaceSize is used before set in set_ergonomics_flags(
From:       harold seigel <harold.seigel () oracle ! com>
Date:       2013-05-31 20:47:30
Message-ID: 51A90C62.3010909 () oracle ! com
[Download RAW message or body]

Hi,

Since Thomas' fix for 8010722 also fixes bug 8009778, I'm withdrawing 
this fix in favor of Thomas' fix for 8010722.

Harold

On 5/30/2013 4:59 PM, Thomas Schatzl wrote:
> Hi all,
> 
> On Thu, 2013-05-30 at 14:05 -0400, harold seigel wrote:
> > Hi,
> > 
> > Please review this fix for bug 8009778.
> > 
> > The fix adds a function, class_metaspace_size_for_compressed_ptrs(),
> > that returns what the value will be for ClassMetaspaceSize if
> > CompressedKlassPointers are used.  This enables the correct value for
> > ClassMetaspaceSize to be used by max_heap_for_compressed_oops() before
> > ClassMetaspaceSize gets set in set_ergonomics_flags().
> > 
> You may want to have a look at the change for 8010722 which is currently
> under review on the hotspot-gc list, which (probably) also fixes this
> one. Unfortunately I forgot to CC hotspot-runtime-dev.
> 
> The mail at
> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2013-May/007155.html is the \
> start of that thread; it would be nice btw if somebody of you did have some time to \
> look over it. 
> That change is very similar, I have no particular preference for either
> solution.
> 
> I have some comment about the choice of 100M for the default table size:
> it is a very "odd" size for heap areas. I mean, later in
> Universe::reserve_heap() this size is aligned up by the alignment given
> to that function.
> 
> That alignment is always a power of two, and there are a few cases when
> this required alignment is quite big, i.e. 32M+.
> 
> One case is if G1 selects a heap region size of 32M, the other much more
> common is probably on SPARC where if the "small" OS page size is set to
> 64k, this alignment is also always 32M regardless of collector.
> 
> Hotspot further down actually expands the actual ClassMetaspaceSize
> size. So it is no real problem except that it seemed somewhat strange to
> me. This is just an observation, nothing to do about it (I think), I
> just asked myself why somebody would choose a value of 100M when doing
> the fix for 8010722 knowing that there's a lot of power-of-two
> alignments in that code.
> 
> Btw, I do think there's some issue here with
> Universe::set_class_metaspace_size() and the alignment in
> Universe::reserve_heap() though: the Universe::_class_metaspace_size
> does not seem to be synchronized with the ClassMetaspaceSize global, so
> any tool (jmap I think) that later reads and prints this value from the
> global will give wrong output. I.e. return the unaligned, not the
> actual, value at startup.
> 
> > The fix was tested with JCK lang and vm tests, UTE vm.quick.testlist and
> > vm.mlvm.testlist tests, JPRT, and jtreg tests. Tests were run on Linux
> > and Solaris. The debugger was used to step through the code to ensure
> > that the fix behaved as expected.
> > 
> > Open webrev at http://cr.openjdk.java.net/~hseigel/bug_8009778/
> > <http://cr.openjdk.java.net/%7Ehseigel/bug_8009778/>
> > 
> > Bug link at http://bugs.sun.com/view_bug.do?bug_id=8009778
> > 
> > JBS bug link at https://jbs.oracle.com/bugs/browse/JDK-8009778
> Apologies again for not CC'ing hotspot-runtime-dev about the 8010722.
> issue.
> 
> Thanks,
> Thomas
> 
> 


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