RFR(M): 8190308: Supporting heap allocation on alternative memory devices and CSR review

[David Holmes]

On 2/12/2017 4:59 AM, sangheon.kim wrote:
> Hi Martin,
> 
> On 11/30/2017 10:08 AM, Doerr, Martin wrote:
>> Hi,
>>
>> I just noticed that "MAP_NORESERVE" is not defined on AIX 
>> (os_posix.cpp reserve_mmapped_memory).
>> I guess it's not POSIX, so this looks like a bug.
> Yes, I agree.
> I filed https://bugs.openjdk.java.net/browse/JDK-8192919 for this.
> 
>> Would it make sense to replace it by "NOT_AIX( | MAP_NORESERVE )"?
> That would be a good quick-fix for AIX.
> 
> But MAP_ANONYMOUS (used with MAP_NORESERVE in that line) is also not 
> specified on POSIX as well. The difference is that most OS seem to 
> define it. Or line 56(#ifndef MAP_ANONYMOUS) makes to avoid the problem.
> So I think it would be better to completely move these 2 flags out of 
> os_posix.cpp.

Moved out or only used for each OS that has established they are correct 
for that OS. I've updated the bug with a request to re-open it.

Thanks,
David

> Thanks,
> Sangheon
> 
> 
>>   Or is there a better idea?
>>
>> Best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: hotspot-gc-dev [mailto:hotspot-gc-dev-bounces at openjdk.java.net] 
>> On Behalf Of Kharbas, Kishor
>> Sent: Montag, 13. November 2017 22:33
>> To: sangheon.kim <sangheon.kim at oracle.com>; Thomas Schatzl 
>> <thomas.schatzl at oracle.com>; 'hotspot-gc-dev at openjdk.java.net' 
>> <hotspot-gc-dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net
>> Cc: Kharbas, Kishor <kishor.kharbas at intel.com>
>> Subject: RE: RFR(M): 8190308: Supporting heap allocation on 
>> alternative memory devices and CSR review
>>
>> Thanks Sangheon and Thomas!
>>
>> -----Original Message-----
>> From: sangheon.kim [mailto:sangheon.kim at oracle.com]
>> Sent: Monday, November 13, 2017 12:41 PM
>> To: Thomas Schatzl <thomas.schatzl at oracle.com>; Kharbas, Kishor 
>> <kishor.kharbas at intel.com>; 'hotspot-gc-dev at openjdk.java.net' 
>> <hotspot-gc-dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8190308: Supporting heap allocation on 
>> alternative memory devices and CSR review
>>
>> Hi Kishor,
>>
>> On 11/13/2017 12:40 PM, Thomas Schatzl wrote:
>>> Hi Kishor,
>>>
>>> On Mon, 2017-11-13 at 19:40 +0000, Kharbas, Kishor wrote:
>>>> Greetings,
>>>> I have an updated webrev to remove compilation warning on Windows 32-
>>>> bit.
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15/
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.15_to_14/
>>>> Sorry missed this earlier. I request for a review on this update.
>>>     looks good. The other changes from webrev.13 on also look good.
>> +1
>>
>> Thanks,
>> Sangheon
>>
>>
>>> Thanks,
>>>     Thomas
>>>
>>>> Thanks
>>>> Kishor
>>>> From: sangheon.kim [mailto:sangheon.kim at oracle.com]
>>>> Sent: Friday, November 3, 2017 4:07 PM
>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Thomas Schatzl <thoma
>>>> s.schatzl at oracle.com>; 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
>>>> dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(M): 8190308: Supporting heap allocation on
>>>> alternative memory devices and CSR review
>>>> Hi Kishor,
>>>>
>>>>
>>>> On 11/03/2017 02:59 PM, Kharbas, Kishor wrote:
>>>> Hi Sangheon,
>>>> Here is link to the updated webrev-
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.14/
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.14_to_13/
>>>> Looks good to me.
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>>
>>>>
>>>> Thanks
>>>> Kishor
>>>> From: sangheon.kim [mailto:sangheon.kim at oracle.com]
>>>> Sent: Friday, November 3, 2017 2:38 PM
>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Thomas Schatzl <thoma
>>>> s.schatzl at oracle.com>; 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
>>>> dev at openjdk.java.net>; hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(M): 8190308: Supporting heap allocation on
>>>> alternative memory devices and CSR review
>>>> Hi Kishor,
>>>>
>>>> On 11/03/2017 11:39 AM, Kharbas, Kishor wrote:
>>>> Thanks a lot!
>>>> Link to updated webrevs -
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.13/
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.13_to_12/
>>>> Thank you for fixing all.
>>>> Looks good to me except below.
>>>>
>>>> Could you update the copyright format in TestAllocateHeapAt.java?
>>>> 2  * Copyright (c) 2017 Oracle and/or its affiliates. All rights
>>>> reserved.
>>>> - Missing comma: * Copyright (c) 2017, Oracle and/or its affiliates.
>>>> All rights reserved.
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>>
>>>>
>>>>
>>>> @Sangheon: Please let me know if you see any corrections needed.
>>>> -Kishor
>>>> -----Original Message-----
>>>> From: Thomas Schatzl [mailto:thomas.schatzl at oracle.com]
>>>> Sent: Friday, November 3, 2017 7:31 AM
>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; sangheon.kim
>>>> <sangheon.kim at oracle.com>; 'hotspot-gc-dev at openjdk.java.net'
>>>> <hotspot-gc-dev at openjdk.java.net>; hotspot-runtime-
>>>> dev at openjdk.java.net
>>>> Subject: Re: RFR(M): 8190308: Supporting heap allocation on
>>>> alternative memory devices and CSR review
>>>> Hi,
>>>> On Fri, 2017-11-03 at 08:55 +0000, Kharbas, Kishor wrote:
>>>> Hi Sangheon,
>>>> Thanks for the review and comments. Here is an updated webrev-
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.12
>>>> http://cr.openjdk.java.net/~kkharbas/8190308/webrev.12_to_11
>>>> In addition to your suggested corrections, I added code to set Linux
>>>> core dump filter ensuring Heap is dumped correctly when this feature
>>>> is used. This is follow-up to Jini George’s comment
>>>> (http://openjdk.5641.n7.nabble.com/RFR-M-8171181-Supporting-heap-
>>>> allocation-on-alternative-memory-devices-td300109.html#a300450).
>>>> Some minor nits:
>>>>    - os_posix.cpp:300: please move the else next to the brace
>>>>    - arguments.cpp:4624: please add a space between "if" and the
>>>> bracket
>>>> I do not need to see a new webrev for these changes. Looks good.
>>>> Thanks,
>>>>     Thomas
>