Request for review: 8009778: NPG: ClassMetaspaceSize is used before set in set_ergonomics_flags()

[Thomas Schatzl]

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