<html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <br>
    Hi all,<br>
    <br>
    Could I have a couple of reviews for this change please? <br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~brutisso/8001049/webrev.00/">http://cr.openjdk.java.net/~brutisso/8001049/webrev.00/</a><br>
    <br>
    I'm sending this to both the GC and the runtime teams since I think
    compressed oops is mixed responsibility for both teams.<br>
    <br>
    Background (mostly from the bug report):<br>
    <br>
    Hotspot crashes if it is run with a large initial size:<br>
    <br>
    &gt;./java -Xms70g -version<br>
    #<br>
    # A fatal error has been detected by the Java Runtime Environment:<br>
    #<br>
    # SIGSEGV (0xb) at pc=0x0000000000000000, pid=14132,
    tid=140305232803584 <br>
    <br>
    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.<br>
    <br>
    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.<br>
    <br>
    Here is the reason for the crash and the rational behind the fix:<br>
    <br>
    <meta http-equiv="content-type" content="text/html;
      charset=ISO-8859-1">
    In Arguments::set_ergonomics_flags() we check that the max heap size
    is small enough before we enable compressed oops:
    <br>
    <br>
    &nbsp;&nbsp;if (MaxHeapSize &lt;= max_heap_for_compressed_oops()) {
    <br>
    #if !defined(COMPILER1) || defined(TIERED)
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;if (FLAG_IS_DEFAULT(UseCompressedOops)) {
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;FLAG_SET_ERGO(bool, UseCompressedOops, true);
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;}
    <br>
    #endif
    <br>
    <br>
    And after that we print a warning message if the heap is too large:
    <br>
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;if (UseCompressedOops &amp;&amp;
    !FLAG_IS_DEFAULT(UseCompressedOops)) {
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;warning("Max heap size too large for Compressed Oops");
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;FLAG_SET_DEFAULT(UseCompressedOops, false);
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;FLAG_SET_DEFAULT(UseCompressedKlassPointers, false);
    <br>
    &nbsp;&nbsp;&nbsp;&nbsp;}
    <br>
    <br>
    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. <br>
    <br>
    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: <br>
    <br>
    1) We don't enable UseCompressedOops on heap sizes that are too
    large <br>
    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. <br>
    <br>
    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.<br>
    <br>
    Thanks,<br>
    Bengt<br>
  </body>
</html>