RFR(s): 8144949: TestOptionsWithRanges -XX:NUMAInterleaveGranularity=2147483648 crashes VM

[Jesper Wilhelmsson]

Den 16/12/15 kl. 02:42, skrev sangheon:
> Hi Jon again,
>
> On 12/15/2015 04:50 PM, sangheon wrote:
>> Hi Jon,
>>
>> Thank you for reviewing this.
>>
>> On 12/15/2015 04:38 PM, Jon Masamitsu wrote:
>>>
>>>
>>> On 12/15/2015 3:47 PM, sangheon wrote:
>>>> Hi Jesper,
>>>>
>>>> Thanks for the review!
>>>>
>>>> Sangheon
>>>>
>>>>
>>>> On 12/15/2015 03:33 PM, Jesper Wilhelmsson wrote:
>>>>> Looks good!
>>>>> /Jesper
>>>>>
>>>>> Den 16/12/15 kl. 00:10, skrev sangheon:
>>>>>> Hi all,
>>>>>>
>>>>>> Could I get some reviews for this change?
>>>>>>
>>>>>> Current 32bit binary with NUMAInterleavingGranularity=2g on server mode
>>>>>> fires an
>>>>>> assert on Windows as we are proceeding without memory allocation failure
>>>>>> check
>>>>>> at CodeCache::reserve_heap_memory.
>>>>>>
>>>>>> I think the constraint function can be removed with maximum range of
>>>>>> 2G/8192G.
>>>>>> These are the maximum available memory on Windows and smaller values can
>>>>>> be used
>>>>>> but I wanted to avoid adding artificial limit. With this limitation, current
>>>>>> constraint function for overflow check is not needed.
>>>>>> And we need to check allocation failure.
>>>>>>
>>>>>> This issue is not reproducible with client mode as the different default
>>>>>> value
>>>>>> of ReservedCodeCacheSize sets SegmentedCodeCache to false. And
>>>>>> CodeCache::reserve_heap_memory() is called only when SegmentedCodeCache is
>>>>>> enabled.
>>>>>>
>>>>>> Skipped adding a test as TestOptionsWithRanges.java is enough when this is
>>>>>> combined with nightly vm option rotation.
>>>>>>
>>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8144949
>>>>>> Webrev: http://cr.openjdk.java.net/~sangheki/8144949/webrev.00
>>>>>> Testing: JPRT (with TestOptionsWithRanges.java enabled), manual tests on
>>>>>> Windows
>>>>>> machine(to test several option combination).
>>>
>>> Sangheon,
>>>
>>> Could you check if the changes in globals.hpp can be put in
>>>
>>> "os/windows/vm/globals_windows.hpp"
>>>
>>> Since this is a windows specific flag, it should work to
>>> go in windows.hpp.
>> This flag is only used for Windows (as its description says), so I am not sure
>> whether we need to move it to windows.hpp.
> I understood your comment to move my changes(range and constraint) to
> globals_windows.hpp. i.e. making the flag as product_pd. Since this flag is only
> used on Windows, I am not sure it is a good idea.
> If you are suggesting to move the whole definition of the flag into
> globals_windows.hpp, let me consider it. I'm not sure whether it's feasible or not.

The description of the flag kind of mentions that this is a Windows specific 
flag, but it's not fool prof. It's possible that this flag is used on 
non-Windows set ups as well. For instance applications could use the same set of 
flags on multiple platforms. Removing this flag on non-Windows platforms would 
be a breaking change in this case and will most likely require a CCC.
/Jesper

>
> Thanks,
> Sangheon
>
>
>>
>>>
>>> On 32bit upper limit is 2G.  Is that the largest value that can be used
>>> without having to check for arithmetic overflow?
>> No.
>> That is Windows OS limitation that each process would use. And within this
>> range we don't need to check arithmetic overflow as it will never happen.
>>
>> Thanks,
>> Sangheon
>>
>>
>>>
>>> Jon
>>>>>>
>>>>>> Thanks,
>>>>>> Sangheon
>>>>>>
>>>>
>>>
>>
>