RFR: 8222252 - Java ergonomics limits heap to 128GB with disabled compressed oops
David Holmes
david.holmes at oracle.com
Mon May 27 22:54:19 UTC 2019
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;
}
> 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.
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>> wrote:
>>
>>
>>> On May 16, 2019, at 11:34 PM, David Holmes <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