<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">I’ve updated the webrev based on your comments.</div><div class=""><br class=""></div><a href="http://cr.openjdk.java.net/~bobv/8222252/webrev.02/" class="">http://cr.openjdk.java.net/~bobv/8222252/webrev.02/</a><div class=""><br class=""></div><div class="">Changes:</div><div class=""><br class=""></div><div class="">1. MaxRAM is updated if fractional flags cause us to use OS memory size for heap selection.</div><div class="">2. If MaxRAM is specified with the other fractional based flags, we will use MaxRAM as</div><div class=""> upper limit instead of OS memory size.<br class=""><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Here’s a proposed CSR for this behavioral change.</div><div class=""><br class=""></div><div class=""><a href="https://bugs.openjdk.java.net/browse/JDK-8223957" class="">https://bugs.openjdk.java.net/browse/JDK-8223957</a></div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Bob.</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On May 14, 2019, at 9:26 AM, Thomas Schatzl <<a href="mailto:thomas.schatzl@oracle.com" class="">thomas.schatzl@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi,<br class=""><br class="">On Mon, 2019-05-13 at 10:03 -0400, Bob Vandette wrote:<br class=""><blockquote type="cite" class="">Please review this suggested fix for correcting Heap size selection<br class="">when -XX:{Max,Min,Initial)RAMPercentage and their Factional<br class="">equivalent options are used.<br class=""><br class="">There are two bugs filed that are related to this issue (8222252 and<br class="">8213175)<br class=""><br class="">SUMMARY of FIX:<br class="">The fix corrects what I believe is an incorrect implementation of the<br class="">*Percentage/*Fraction options. These options should not have caused<br class="">the heap size to be fractionally based on MaxRAM or limited by<br class="">MaxRAM. They instead should be based on the host memory. This is<br class="">what I assume was meant by “real memory”. <br class=""><br class=""> product(double, MaxRAMPercentage,<br class="">25.0, \<br class=""> "Maximum percentage of real memory used for maximum heap<br class="">size") \<br class=""> range(0.0, 100.0) <br class=""><br class="">When these options are selected, they should take precedence over<br class="">UseCompressedOops unless UseCompressedOops is also specified on the<br class="">command line.<br class=""><br class=""><br class="">WEBREV:<br class=""><a href="http://cr.openjdk.java.net/~bobv/8222252/webrev.01" class="">http://cr.openjdk.java.net/~bobv/8222252/webrev.01</a><br class=""></blockquote><br class="">- this is a style preference: I would assign the result of the<br class="">condition in the if (arguments.cpp:1721+) to the initialization of<br class="">use_os_mem_limit directly instead of first setting it to false, and<br class="">then to true again if needed. Feel free to ignore.<br class=""><br class="">- as David pointed out, this change needs a CSR.<br class=""><br class="">- the interaction between MaxRAM and MaxRAMPercentage is a bit unclear;<br class="">if they are independent as implemented now, then the update of one<br class="">should probably update the other. Or the VM should fail if they<br class="">disagree?<br class=""><br class="">If they are dependent, if David suggests (i.e. MaxRAMPercentage uses<br class="">MaxRAM as maximum real memory if set; this is what I would tend to<br class="">prefer), then not of course.<br class=""><br class="">- there should be regressions tests verifying this in cases where<br class="">possible.<br class=""><br class="">I.e. it should be doable to get current os::physical_memory using e.g.<br class="">whitebox and then run a few tests verifying the results of MaxHeapSize<br class="">and coop mode.<br class=""><br class="">Particularly the interaction between MaxRAM, MaxRAMPercentage and coops<br class="">mode would be interesting.<br class=""><br class="">Thanks,<br class=""> Thomas<br class=""><br class=""><br class=""></div></div></blockquote></div><br class=""></div></div></body></html>