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

[Bob Vandette]

> 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.

After Davids comment, I’m thinking that the user setting of MaxRAM should have
the same side affect as the MaxRAMPercentage in that it should override coop_limit.
If you agree then I’ll change the flag name to something like “override_coop_limit”.

I agree that I should set MaxRAM to phys_mem if the MaxRAMPercentage flags
are specified.  I didn’t realize that this flag was referenced outside of set_heap_size function.


> 
> - there should be regressions tests verifying this in cases where
> possible.

I’ll look into that assuming I get a CSR approval on my proposed changes.

Bob.

> 
> 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
> 
>