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

Mon Dec 4 19:38:58 UTC 2017

Hi David and Martin,

I re-opened JDK-8192919 with a new title.
This enhancement is to remove non-portable flags used in os_posix.cpp.

Thanks,
Sangheon

On 12/04/2017 02:27 AM, Doerr, Martin wrote:
> Hi Sangheon and David,
>
> first of all, sorry that I forgot to mention JDK-8192898 "AIX build broken after JDK-8190308" in this email thread. It was a quick fix to get AIX working again for the jdk10 fork.
>
> Thanks for opening the new bug JDK-8192919 and for sharing your thoughts. Sounds like a nice cleanup for jdk11.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Sonntag, 3. Dezember 2017 03:51
> To: sangheon.kim <sangheon.kim at oracle.com>; Doerr, Martin <martin.doerr at sap.com>; Kharbas, Kishor <kishor.kharbas at intel.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; Stuefe, Thomas <thomas.stuefe at sap.com>
> Subject: Re: RFR(M): 8190308: Supporting heap allocation on alternative memory devices and CSR review
>
> 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