RFR (XS): JDK-8017483: G1 tests fail with native OOME on Solaris x86 after HeapBaseMinAddress has been increased

[Bengt Rutisson]

Thanks Vladimir!

Bengt

On 6/26/13 11:10 PM, Vladimir Kozlov wrote:
> Good.
>
> Vladimir
>
> On 6/26/13 2:03 PM, Bengt Rutisson wrote:
>>
>> Thanks Stefan and Jon for looking at this!
>>
>> On 6/26/13 4:20 PM, Jon Masamitsu wrote:
>>> Bengt,
>>>
>>> Change looks fine.  Is it worth a comment to say why you don't use
>>> FLAG_SET_ERGO() ?
>>
>> Done.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~brutisso/8017483/webrev.01/
>>
>> Complete diff:
>>
>> diff --git a/src/share/vm/runtime/arguments.cpp
>> b/src/share/vm/runtime/arguments.cpp
>> --- a/src/share/vm/runtime/arguments.cpp
>> +++ b/src/share/vm/runtime/arguments.cpp
>> @@ -1571,7 +1571,9 @@
>>       // By default HeapBaseMinAddress is 2G on all platforms except
>> Solaris x86.
>>       // G1 currently needs a lot of C-heap, so on Solaris we have to
>> give G1
>>       // some extra space for the C-heap compared to other collectors.
>> -    FLAG_SET_ERGO(uintx, HeapBaseMinAddress, 1*G);
>> +    // Use FLAG_SET_DEFAULT here rather than FLAG_SET_ERGO to make sure
>> that
>> +    // code that checks for default values work correctly.
>> +    FLAG_SET_DEFAULT(HeapBaseMinAddress, 1*G);
>>     }
>>   }
>>
>> Also, I filed JDK-8019212 to track the strange behavior for
>> HeapBaseMinAddress.
>>
>> Bengt
>>
>>>
>>> Jon
>>>
>>> On 6/26/2013 4:57 AM, Bengt Rutisson wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Could I have a couple of reviews for this really small change?
>>>>
>>>> http://cr.openjdk.java.net/~brutisso/8017483/webrev.00/
>>>>
>>>> Background:
>>>>
>>>> For JDK-8012265 I increased the default value for HeapBaseMinAddress
>>>> on Solaris x86 from 256m to 1g when using G1. In that change I used
>>>> FLAG_SET_ERGO() to set the new value of HeapBaseMinAddress. Now it
>>>> turns out that there is code that checks
>>>> !FLAG_IS_DEFAULT(HeapBaseMinAddress), which means that by setting the
>>>> flag as "ergo" I changed the behavior of the VM.
>>>>
>>>> What happens is that Universe::preferred_heap_base() will use exactly
>>>> the address HeapBaseMinAddress if it is not default. Otherwise it
>>>> will calculate a base adress "backwards" from the maximum compressed
>>>> oops heap size and the desired heap size. The effect is that if
>>>> HeapBaseMinAddress=1g as default and we want a 16g heap we will get
>>>> an actual heap base of at about address 16g. But if I set
>>>> HeapBaseMinAddress=1g on the command line or using "ergo" we will
>>>> pick the address 1g for the heap base.
>>>>
>>>> This means that in the first case we leave almost 16g free for the
>>>> Solaris C-heap, but in the second case the C-heap will be limited 
>>>> to 1g.
>>>>
>>>> I think this is a strange behavior and I will file a separate bug to
>>>> track that. There are also other issues with the compressed oops set
>>>> up. But to make the currently failing tests pass I suggest to change
>>>> from "ergo" to "default" when updating the HeapBaseMinAddress value.
>>>> This is small, quick and safe change. Here is the complete diff:
>>>>
>>>> diff --git a/src/share/vm/runtime/arguments.cpp
>>>> b/src/share/vm/runtime/arguments.cpp
>>>> --- a/src/share/vm/runtime/arguments.cpp
>>>> +++ b/src/share/vm/runtime/arguments.cpp
>>>> @@ -1571,7 +1571,7 @@
>>>>      // By default HeapBaseMinAddress is 2G on all platforms except
>>>> Solaris x86.
>>>>      // G1 currently needs a lot of C-heap, so on Solaris we have to
>>>> give G1
>>>>      // some extra space for the C-heap compared to other collectors.
>>>> -    FLAG_SET_ERGO(uintx, HeapBaseMinAddress, 1*G);
>>>> +    FLAG_SET_DEFAULT(HeapBaseMinAddress, 1*G);
>>>>    }
>>>>  }
>>>>
>>>> Thanks,
>>>> Bengt
>>>>
>>>
>>