<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    BTW, upon completion of this CR, please also investigate a CR with
    similar problems (most likely, close as a duplicate) .<br>
    <a class="moz-txt-link-freetext" href="https://jbs.oracle.com/bugs/browse/JDK-6374896">https://jbs.oracle.com/bugs/browse/JDK-6374896</a><br>
    <br>
    Please see inline.<br>
    Tao<br>
    <br>
    On 3/13/13 6:17 AM, Thomas Schatzl wrote:
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">Hi all,

  please review the following change which improves ergonomic heap
sizing. Automatic heap sizing now takes available virtual memory into
account.

The problem occurred when the user reduced the available virtual memory
via e.g. ulimit. As ergonomics did not take virtual memory into account,
using available physical memory only, the vm typically crashed at
startup.

This patch does not avoid all crashes due to out of virtual memory,
because only heap sizing now takes available virtual memory into account
now. Some GCs, and also other parts of the VM, try to reserve hundreds
of MB at startup.</pre>
    </blockquote>
    I don't quite understand what you mean here. Does it indicate that
    the default value (i.e. 2) chosen for MaxVirMemFraction is a bit
    large? (when it has the effect of limiting the head size, the
    virtual memory would be small already.) <br>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">

The main change is in Arguments::allocatable_physical_memory() (which
has been moved from the os class) where if an additional virtual memory
limit has been imposed on the java process, the given heap size is
bounded by a fraction of that limit.</pre>
    </blockquote>
    It kind of violates parallelism here, for
    Arguments::allocatable_physical_memory() and os::physical_memory()
    are now defined in different classes while they should have the same
    level of abstraction. Seems better to keep
    allocatable_physical_memory() back in os due to its closeness to os.<br>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">

That fraction is determined by a new global called MaxVirtMemFraction
(default value: 2).</pre>
    </blockquote>
    follow the style<br>
    <meta charset="utf-8">
    <pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">1946   product(uintx, MaxRAMFraction, 4,                                         \
1947           "Maximum fraction (1/n) of real memory used for maximum heap "    \
1948           "size")                                                           \
</pre>
    would be reasonable to phrase the definition similarly, say,<br>
    <meta charset="utf-8">
    <pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">1961   product(uintx, MaxVirtMemFraction, 2,                                     \
1962           "<b>Maximum</b> fraction (1/n) of virtual memory used for <b>maximum</b> heap size")            \
1963                                                                             \</pre>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">

Some discussion points:
  - maybe make MaxVirtMemFraction at least notproduct. It does not seem
to make much sense to expose that global to the end user, does it? If
the user is able to change MaxVirtMemFraction, he/she may as well set an
appropriate maximum heap size with the same effect.</pre>
    </blockquote>
    Makes sense.<br>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">
  - in the OS specific implementations I kept the code duplication for
the unix targets (bsd, linux, solaris) as it has been before. I think
there is some effort for that going on already.
  - also in 32 bit implementations, the previously used heuristically
found out virtual memory limits (around ~3.8GB) were kept.
  - for querying the virtual memory limit I used getrlimit() with
RLIMIT_AS as parameter. In Solaris there is also RLIMIT_VMEM. Searching
in opensolaris code showed that RLIMIT_AS is just another name for
RLIMIT_VMEM. Additionally, RLIMIT_AS is the only available parameter for
getrlimit() of the two in the posix spec
( <a class="moz-txt-link-freetext" href="http://pubs.opengroup.org/onlinepubs/009696799/functions/getrlimit.html">http://pubs.opengroup.org/onlinepubs/009696799/functions/getrlimit.html</a> ). This could help later when/if merging the various Unixes.

What do you think?

Webrev:
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~tschatzl/7112912/webrev/">http://cr.openjdk.java.net/~tschatzl/7112912/webrev/</a></pre>
    </blockquote>
    (1) you may want to rephrase the comment or delete it<br>
    <meta charset="utf-8">
    <pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">2607       // Make sure that if we have a lot of memory we cap the 32 bit
2608       // process space.  The 64bit VM version of this function is a nop.
2609       initHeapSize = allocatable_physical_memory(initHeapSize);</pre>
    (2) I also prefer a pointer argument here. It's more readable to
    recognize its intention of usage.<br>
    <meta charset="utf-8">
    <pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">187   static bool has_allocatable_memory_limit(julong&amp; limit);</pre>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">

CR:
<a class="moz-txt-link-freetext" href="http://bugs.sun.com/view_bug.do?bug_id=7112912">http://bugs.sun.com/view_bug.do?bug_id=7112912</a>

JIRA:
<a class="moz-txt-link-freetext" href="https://jbs.oracle.com/bugs/browse/JDK-7112912">https://jbs.oracle.com/bugs/browse/JDK-7112912</a>

Testing:
jprt, local testing with different ulimit values</pre>
    </blockquote>
    Did you test out all(or, most) platform combinations
    (solaris/bsd/linux/windows + 32/64 bit) to verify the current code
    can get the right size of virtual memory?<br>
    <blockquote cite="mid:1363180662.2535.79.camel@cirrus" type="cite">
      <pre wrap="">

Thomas

</pre>
    </blockquote>
  </body>
</html>