Request for review (S): 8001049: VM crashes when running with large -Xms and not specifying ObjectAlignmentInBytes

[Bengt Rutisson]

Hi all,

Could I have a couple of reviews for this change please?
http://cr.openjdk.java.net/~brutisso/8001049/webrev.00/

I'm sending this to both the GC and the runtime teams since I think 
compressed oops is mixed responsibility for both teams.

Background (mostly from the bug report):

Hotspot crashes if it is run with a large initial size:

 >./java -Xms70g -version
#
# A fatal error has been detected by the Java Runtime Environment:
#
# SIGSEGV (0xb) at pc=0x0000000000000000, pid=14132, tid=140305232803584

The reason is that we enable UseCompressedOops but we use the default 
value for ObjectAlignmentInBytes. With a large heap size we would have 
needed to adjust the object alignment to be able to use compressed oops.

However, after reviewing the code it looks like the fix is not to try to 
adjust the object alignment but rather to not enable compressed oops for 
large heaps. If someone wants to use compressed oops on a very large 
heap they need to explicitly set both UseCompressedOops and 
ObjectAlignmentInBytes on the command line. As far as I can tell this is 
how it is intended to work.

Here is the reason for the crash and the rational behind the fix:

In Arguments::set_ergonomics_flags() we check that the max heap size is 
small enough before we enable compressed oops:

   if (MaxHeapSize <= max_heap_for_compressed_oops()) {
#if !defined(COMPILER1) || defined(TIERED)
     if (FLAG_IS_DEFAULT(UseCompressedOops)) {
       FLAG_SET_ERGO(bool, UseCompressedOops, true);
     }
#endif

And after that we print a warning message if the heap is too large:

     if (UseCompressedOops && !FLAG_IS_DEFAULT(UseCompressedOops)) {
       warning("Max heap size too large for Compressed Oops");
       FLAG_SET_DEFAULT(UseCompressedOops, false);
       FLAG_SET_DEFAULT(UseCompressedKlassPointers, false);
     }

Now the problem is that when we don't set the max heap size on the 
command line it will be adjusted based on the initial size (-Xms) and 
this happens in Arguments::set_heap_size(), which is called *after* 
Arguments::set_ergonomics_flags() has been called. So, when we do the 
check against the max size in Arguments::set_ergonomics_flags(), we 
check against the default value for the max size. This value fits well 
with a compressed heap, so we enable compressed oops and crash later on 
when we can't address the upper part of the heap.

The fix is to move the call to set_heap_size() to *before* the call to 
set_ergonomics_flags(). This way the check is done against the correct 
value. This has two effects:

1) We don't enable UseCompressedOops on heap sizes that are too large
2) If someone sets -XX:+UseCompressedOops on the command line but 
specifies a too large heap a warning message will be logged and 
UseCompressedOops will be turned off.

I am always hesitant to rearrange the order of calls in 
Arguments::parse(). But in this case it is necessary to get the correct 
behavior and I think it is safe. As far as I can tell there is no other 
code between the two methods that try to read the MaxHeapSize value.

Thanks,
Bengt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130308/8d361a2a/attachment.htm>