RFR: 8222252 - Java ergonomics limits heap to 128GB with disabled compressed oops
David Holmes
david.holmes at oracle.com
Wed May 29 13:05:36 UTC 2019
On 29/05/2019 9:42 pm, Bob Vandette wrote:
>
>> 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.
Okay I see it now. Thanks.
I still think there should be some way to simplify the code further but
what you have does what you specified.
Thanks for bearing with me.
CSR also reviewed.
David
-----
>
> 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-runtime-dev
mailing list