RFR: 8222252 - Java ergonomics limits heap to 128GB with disabled compressed oops
Bob Vandette
bob.vandette at oracle.com
Wed May 29 11:42:57 UTC 2019
> On May 29, 2019, at 3:21 AM, David Holmes <david.holmes at oracle.com> wrote:
>
> Hi Bob,
>
> On 28/05/2019 11:23 pm, Bob Vandette wrote:
>>> On May 27, 2019, at 6:54 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Hi Bob,
>>>
>>> On 25/05/2019 2:02 am, Bob Vandette wrote:
>>>> Here’s the updated webrev.
>>>> http://cr.openjdk.java.net/~bobv/8222252/webrev.03
>>>> I’ve also updated the CSR (https://bugs.openjdk.java.net/browse/JDK-8223957).
>>>
>>> Sorry but I'm having some trouble reconciling the code with the general description. I would have expected to see something like this:
>>>
>>> bool override_coop_limit = ((!FLAG_IS_DEFAULT(MaxRAMPercentage) ||
>>> !FLAG_IS_DEFAULT(MaxRAMFraction) ||
>>> !FLAG_IS_DEFAULT(MinRAMPercentage) ||
>>> !FLAG_IS_DEFAULT(MinRAMFraction) ||
>>> !FLAG_IS_DEFAULT(InitialRAMPercentage) ||
>>> ! !FLAG_IS_DEFAULT(InitialRAMFraction)) &&
>>> ! FLAG_IS_DEFAULT(MaxRAM));
>>>
>>> where override_coop_limit might also be called use_physical_memory, or ignore_maxram, if I understand correctly. Then things would simplify to:
>>>
>>> if (override_coop_limit) { // use physical, ignore MaxRAM
>>> phys_mem = os::physical_memory();
>>> FLAG_SET_ERGO(MaxRAM, (uint64_t)phys_mem);
>>> } else { // use MaxRAM
>>> phys_mem = FLAG_IS_DEFAULT(MaxRAM) ? MIN2(os::physical_memory(), (julong)MaxRAM)
>>> : (julong)MaxRAM;
>>> }
>> The problem with your implementation is that if the user specifies a MaxRAM value that’s higher than the coop_limit,
>> the heap size will be reduced in favor of UseCompressedOops.
>> My CSR states "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.”
>> My rationale is that if someone goes to the effort of specifying a large MaxRAM value, they
>> don’t want to end up with a different value without knowing it’s happening.
>
> So I'm having trouble establishing that the two problems:
> - when do I use MaxRAM over os::physical_memory(); and
In all cases where MaxRAM is set on the command line.
> - when do I not allow coop_limit to set the limit
When any of the RAM Percentage or MaxRAM values are set.
> are actually satisfied by the same condition that is being captured in override_coop_limit. I admit I focussed on the code needed to address the first problem, assuming it would also address the second. If that's not the case then perhaps a more explicit formulation of when to override coop_limit is needed?
Both problems are satisfied with the changes I’ve implemented in the new webrev.
Bob.
>
> David
> -----
>
>> If this is not an acceptable behavior, I can simplify my changes to match yours.
>>>
>>>> I’ve filed a bug to create tests for this change (https://bugs.openjdk.java.net/browse/JDK-8224764) which
>>>> I will start working on in parallel with the integration of this change.
>>>
>>> Given the change is taking somewhat longer than you may have hoped, I hope the test has now progressed. I think it would be good to get everything in together rather than break it up.
>> I split out the test since it can be integrated later in the process. I’m out of the office next week and might not have time to get
>> that done before I return.
>> Bob.
>>>
>>> Thanks,
>>> David
>>>
>>>> Here are the latest results showing the old and new MaxHeapSize and UseCompressedOops values before
>>>> and after this change.
>>>> ./java -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}
>>>> size_t MaxHeapSize [NEW] = 113883742208 {product} {ergonomic}
>>>> /java -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}
>>>> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}
>>>> ./java -XX:MaxRAM=192g -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}
>>>> size_t MaxHeapSize [NEW] = 51539607552 {product} {ergonomic}
>>>> ./java -XX:MaxRAM=192g -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}
>>>> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}
>>>> ./java -XX:MaxRAM=192g -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize [OLD] = 32178700288 {product} {ergonomic}
>>>> size_t MaxHeapSize [NEW] = 154618822656 {product} {ergonomic}
>>>> ./java -XX:MaxRAM=192g -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops [OLD] = true {lp64_product} {ergonomic}
>>>> bool UseCompressedOops [NEW] = false {lp64_product} {ergonomic}
>>>> ./java -XX:MaxRAMPercentage=10 -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize [OLD] = 13744734208 {product} {ergonomic}
>>>> size_t MaxHeapSize [NEW] = 15183380480 {product} {ergonomic}
>>>> ./java -XX:MaxRAMPercentage=10 -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops = true {lp64_product} {ergonomic}
>>>> ./java -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize = 32178700288 {product} {ergonomic}
>>>> ./java -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops = true {lp64_product} {ergonomic}
>>>> ./java -XX:-UseCompressedOops -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize = 34359738368 {product} {ergonomic}
>>>> ./java -XX:-UseCompressedOops -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops = false {lp64_product} {command line}
>>>> ./java -XX:+UseCompressedOops -XX:MaxRAM=128g -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize = 32178700288 {product} {ergonomic}
>>>> ./java -XX:+UseCompressedOops -XX:MaxRAM=128g -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops = true {lp64_product} {command line}
>>>> ./java -XX:+UseCompressedOops -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep MaxHeapSize
>>>> size_t MaxHeapSize = 32178700288 {product} {ergonomic}
>>>> ./java -XX:+UseCompressedOops -XX:MaxRAMPercentage=75 -XX:+PrintFlagsFinal -version | grep UseCompressedOops
>>>> bool UseCompressedOops = true {lp64_product} {command line}
>>>> Bob.
>>>>> On May 23, 2019, at 9:15 AM, Bob Vandette <bob.vandette at oracle.com <mailto:bob.vandette at oracle.com> <mailto:bob.vandette at oracle.com>> wrote:
>>>>>
>>>>>
>>>>>> On May 16, 2019, at 11:34 PM, David Holmes <David.Holmes at oracle.com <mailto:David.Holmes at oracle.com> <mailto:David.Holmes at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Bob,
>>>>>>
>>>>>> On 16/05/2019 12:52 am, Bob Vandette wrote:
>>>>>>> I’ve updated the webrev based on your comments.
>>>>>>> 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.
>>>>>>
>>>>>> That seems fine, but here:
>>>>>>
>>>>>> 1799 if (FLAG_IS_ERGO(UseCompressedOops) && use_os_mem_limit) {
>>>>>>
>>>>>> Shouldn't this also be checking for FLAG_IS_DEFAULT(MaxRAM) ? Or better that check would fold into the use_os_mem_limit determination:
>>>>>>
>>>>>> bool use_os_mem_limit = (!FLAG_IS_DEFAULT(MaxRAMPercentage) ||
>>>>>> !FLAG_IS_DEFAULT(MaxRAMFraction) ||
>>>>>> !FLAG_IS_DEFAULT(MinRAMPercentage) ||
>>>>>> !FLAG_IS_DEFAULT(MinRAMFraction) ||
>>>>>> !FLAG_IS_DEFAULT(InitialRAMPercentage) ||
>>>>>> !FLAG_IS_DEFAULT(InitialRAMFraction)) &&
>>>>>> FLAG_IS_DEFAULT(MaxRAM);
>>>>>> if (use_os_mem_limit) {
>>>>>> phys_mem = os::physical_memory();
>>>>>> FLAG_SET_ERGO(uint64_t, MaxRAM, (uint64_t)phys_mem);
>>>>>> } else {
>>>>>> phys_mem = FLAG_IS_DEFAULT(MaxRAM) ? MIN2(os::physical_memory(), (julong)MaxRAM)
>>>>>> : (julong)MaxRAM;
>>>>>> }
>>>>>>
>>>>>
>>>>> Yes, you are correct. I’ll add MaxRAM to the use_os_mem_limit check. This is the behavior
>>>>> I documented in the CSR.
>>>>>
>>>>> "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.”
>>>>>
>>>>> I’ll update the patch provided in the CSR as well.
>>>>>
>>>>>
>>>>> Can you take a look and review the CSR for this change?
>>>>>
>>>>> Thanks,
>>>>> Bob.
>>>>>
>>>>>
>>>>>> ?
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>> Here’s a proposed CSR for this behavioral change.
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8223957
>>>>>>> Bob.
>>>>>>>> On May 14, 2019, at 9:26 AM, Thomas Schatzl <thomas.schatzl at oracle.com <mailto: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
>>>>>>>>
>>>>>>>>
>>>>>
More information about the hotspot-gc-dev
mailing list