RFR: 8222252 - Java ergonomics limits heap to 128GB with disabled compressed oops

[Bob Vandette]

I’ve updated the webrev based on your comments.

http://cr.openjdk.java.net/~bobv/8222252/webrev.02/ <http://cr.openjdk.java.net/~bobv/8222252/webrev.02/>

Changes:

1. MaxRAM is updated if fractional flags cause us to use OS memory size for heap selection.
2. If MaxRAM is specified with the other fractional based flags, we will use MaxRAM as
   upper limit instead of OS memory size.


Here’s a proposed CSR for this behavioral change.

https://bugs.openjdk.java.net/browse/JDK-8223957 <https://bugs.openjdk.java.net/browse/JDK-8223957>


Bob.

> On May 14, 2019, at 9:26 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi,
> 
> On Mon, 2019-05-13 at 10:03 -0400, Bob Vandette wrote:
>> Please review this suggested fix for correcting Heap size selection
>> when -XX:{Max,Min,Initial)RAMPercentage and their Factional
>> equivalent options are used.
>> 
>> There are two bugs filed that are related to this issue (8222252 and
>> 8213175)
>> 
>> SUMMARY of FIX:
>> The fix corrects what I believe is an incorrect implementation of the
>> *Percentage/*Fraction options.  These options should not have caused
>> the heap size to be fractionally based on MaxRAM or limited by
>> MaxRAM.  They instead should be based on the host memory. This is
>> what I assume was meant by “real memory”.  
>> 
>> product(double, MaxRAMPercentage,
>> 25.0,                                   \
>>         "Maximum percentage of real memory used for maximum heap
>> size") \
>>         range(0.0, 100.0) 
>> 
>> When these options are selected, they should take precedence over
>> UseCompressedOops unless UseCompressedOops is also specified on the
>> command line.
>> 
>> 
>> WEBREV:
>> http://cr.openjdk.java.net/~bobv/8222252/webrev.01
> 
> - this is a style preference: I would assign the result of the
> condition in the if (arguments.cpp:1721+) to the initialization of
> use_os_mem_limit directly instead of first setting it to false, and
> then to true again if needed. Feel free to ignore.
> 
> - as David pointed out, this change needs a CSR.
> 
> - the interaction between MaxRAM and MaxRAMPercentage is a bit unclear;
> if they are independent as implemented now, then the update of one
> should probably update the other. Or the VM should fail if they
> disagree?
> 
> If they are dependent, if David suggests (i.e. MaxRAMPercentage uses
> MaxRAM as maximum real memory if set; this is what I would tend to
> prefer), then not of course.
> 
> - there should be regressions tests verifying this in cases where
> possible.
> 
> I.e. it should be doable to get current os::physical_memory  using e.g.
> whitebox and then run a few tests verifying the results of MaxHeapSize
> and coop mode.
> 
> Particularly the interaction between MaxRAM, MaxRAMPercentage and coops
> mode would be interesting.
> 
> Thanks,
>  Thomas
> 
>