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

Coleen Phillimore coleen.phillimore at
Fri May 31 11:46:16 UTC 2013

Hi Thomas,  It doesn't look like this change fixes the set before used 
problem of ClassMetaspaceSize but is a more comprehensive fix so I think 
you should fix Harold's bug with your fix.   I only looked at 
arguments.cpp,hpp though.

Actually, both changes are wrong for this because you only set 
ClassMetaspaceSize to 100*M (agreed, not a good size) if 
UseCompressedKlassPointers is set which you don't set until later.   
Harold is working on a fix move the class metaspace somewhere else, not 
attached to the Java heap calculations which will resolve this issue.   
So I think we should leave this broken for now.


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
> 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
>> <>
>> Bug link at
>> JBS bug link at
> Apologies again for not CC'ing hotspot-runtime-dev about the 8010722.
> issue.
> Thanks,
>    Thomas

More information about the hotspot-gc-dev mailing list