RFR: 8222252 - Java ergonomics limits heap to 128GB with disabled compressed oops
David Holmes
david.holmes at oracle.com
Wed May 29 07:21:53 UTC 2019
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
- when do I not allow coop_limit to set the limit
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?
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