<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="">Here’s the updated webrev. <div class=""><br class=""></div><div class=""><a href="http://cr.openjdk.java.net/~bobv/8222252/webrev.03" class="">http://cr.openjdk.java.net/~bobv/8222252/webrev.03</a></div><div class=""><br class=""></div><div class=""> I’ve also updated the CSR (<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="">I’ve filed a bug to create tests for this change (<a href="https://bugs.openjdk.java.net/browse/JDK-8224764" class="">https://bugs.openjdk.java.net/browse/JDK-8224764</a>) which</div><div class="">I will start working on in parallel with the integration of this change. </div><div class=""><br class=""></div><div class="">Here are the latest results showing the old and new MaxHeapSize and UseCompressedOops values before</div><div class="">and after this change.</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [NEW] = 113883742208 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">/java -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAM=192g -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [NEW] = 51539607552 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAM=192g -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAM=192g -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [NEW] = 154618822656 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAM=192g -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAMPercentage=10 -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [OLD] = 13744734208 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize [NEW] = 15183380480 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:MaxRAMPercentage=10 -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops = true {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops = true {lp64_product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:-UseCompressedOops -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize = 34359738368 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:-UseCompressedOops -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops = false {lp64_product} {command line}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+UseCompressedOops -XX:MaxRAM=128g -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+UseCompressedOops -XX:MaxRAM=128g -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops = true {lp64_product} {command line}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+UseCompressedOops -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> size_t MaxHeapSize = 32178700288 {product} {ergonomic}</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class="">./java -XX:+UseCompressedOops -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops</span></div><div style="margin: 0px; font-size: 18px; line-height: normal; font-family: 'Courier New'; background-color: rgb(255, 255, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures" class=""> bool UseCompressedOops = true {lp64_product} {command line}</span></div></div><div class=""><span style="font-variant-ligatures: no-common-ligatures" class=""><br class=""></span></div><div class="">Bob.</div><div class=""><br class=""><div class=""><div><blockquote type="cite" class=""><div class="">On May 23, 2019, at 9:15 AM, Bob Vandette <<a href="mailto:bob.vandette@oracle.com" class="">bob.vandette@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class=""><br class=""><blockquote type="cite" class="">On May 16, 2019, at 11:34 PM, David Holmes <<a href="mailto:David.Holmes@oracle.com" class="">David.Holmes@oracle.com</a>> wrote:<br class=""><br class="">Hi Bob,<br class=""><br class="">On 16/05/2019 12:52 am, Bob Vandette wrote:<br class=""><blockquote type="cite" class="">I’ve updated the webrev based on your comments.<br class=""><a href="http://cr.openjdk.java.net/~bobv/8222252/webrev.02/" class="">http://cr.openjdk.java.net/~bobv/8222252/webrev.02/</a><br class="">Changes:<br class="">1. MaxRAM is updated if fractional flags cause us to use OS memory size for heap selection.<br class="">2. If MaxRAM is specified with the other fractional based flags, we will use MaxRAM as<br class=""> upper limit instead of OS memory size.<br class=""></blockquote><br class="">That seems fine, but here:<br class=""><br class="">1799 if (FLAG_IS_ERGO(UseCompressedOops) && use_os_mem_limit) {<br class=""><br class="">Shouldn't this also be checking for FLAG_IS_DEFAULT(MaxRAM) ? Or better that check would fold into the use_os_mem_limit determination:<br class=""><br class=""> bool use_os_mem_limit = (!FLAG_IS_DEFAULT(MaxRAMPercentage) ||<br class=""> !FLAG_IS_DEFAULT(MaxRAMFraction) ||<br class=""> !FLAG_IS_DEFAULT(MinRAMPercentage) ||<br class=""> !FLAG_IS_DEFAULT(MinRAMFraction) ||<br class=""> !FLAG_IS_DEFAULT(InitialRAMPercentage) ||<br class=""> !FLAG_IS_DEFAULT(InitialRAMFraction)) &&<br class=""> FLAG_IS_DEFAULT(MaxRAM);<br class=""> if (use_os_mem_limit) {<br class=""> phys_mem = os::physical_memory();<br class=""> FLAG_SET_ERGO(uint64_t, MaxRAM, (uint64_t)phys_mem);<br class=""> } else {<br class=""> phys_mem = FLAG_IS_DEFAULT(MaxRAM) ? MIN2(os::physical_memory(), (julong)MaxRAM)<br class=""> : (julong)MaxRAM;<br class=""> }<br class=""><br class=""></blockquote><br class="">Yes, you are correct. I’ll add MaxRAM to the use_os_mem_limit check. This is the behavior<br class="">I documented in the CSR. <br class=""><br class=""> "Also, If any of these flags including MaxRAM are specified on the command line, the selected Java heap size will not be limited by the compressed oop limit.”<br class=""><br class="">I’ll update the patch provided in the CSR as well.<br class=""><br class=""><br class="">Can you take a look and review the CSR for this change?<br class=""><br class="">Thanks,<br class="">Bob.<br class=""><br class=""><br class=""><blockquote type="cite" class="">?<br class=""><br class="">Thanks,<br class="">David<br class="">-----<br class=""><br class=""><blockquote type="cite" class="">Here’s a proposed CSR for this behavioral change.<br class=""><a href="https://bugs.openjdk.java.net/browse/JDK-8223957" class="">https://bugs.openjdk.java.net/browse/JDK-8223957</a><br class="">Bob.<br class=""><blockquote type="cite" class="">On May 14, 2019, at 9:26 AM, Thomas Schatzl <thomas.schatzl@oracle.com <mailto:thomas.schatzl@oracle.com>> wrote:<br class=""><br 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="">http://cr.openjdk.java.net/~bobv/8222252/webrev.01<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=""></blockquote></blockquote></blockquote><br class=""></div></div></blockquote></div><br class=""></div></div></body></html>