[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-gc-dev
Subject:    Re: RFR(M): 8190308: Supporting heap allocation on alternative memory devices and CSR review
From:       "sangheon.kim" <sangheon.kim () oracle ! com>
Date:       2017-12-04 19:38:58
Message-ID: b5bff7f9-4cbd-eda0-90a6-dc883a337452 () oracle ! com
[Download RAW message or body]

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@oracle.com]
> Sent: Sonntag, 3. Dezember 2017 03:51
> To: sangheon.kim <sangheon.kim@oracle.com>; Doerr, Martin <martin.doerr@sap.com>; \
> Kharbas, Kishor <kishor.kharbas@intel.com>; Thomas Schatzl \
> <thomas.schatzl@oracle.com>; 'hotspot-gc-dev@openjdk.java.net' \
> <hotspot-gc-dev@openjdk.java.net>; hotspot-runtime-dev@openjdk.java.net; Stuefe, \
>                 Thomas <thomas.stuefe@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@openjdk.java.net]
> > > On Behalf Of Kharbas, Kishor
> > > Sent: Montag, 13. November 2017 22:33
> > > To: sangheon.kim <sangheon.kim@oracle.com>; Thomas Schatzl
> > > <thomas.schatzl@oracle.com>; 'hotspot-gc-dev@openjdk.java.net'
> > > <hotspot-gc-dev@openjdk.java.net>; hotspot-runtime-dev@openjdk.java.net
> > > Cc: Kharbas, Kishor <kishor.kharbas@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@oracle.com]
> > > Sent: Monday, November 13, 2017 12:41 PM
> > > To: Thomas Schatzl <thomas.schatzl@oracle.com>; Kharbas, Kishor
> > > <kishor.kharbas@intel.com>; 'hotspot-gc-dev@openjdk.java.net'
> > > <hotspot-gc-dev@openjdk.java.net>; hotspot-runtime-dev@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@oracle.com]
> > > > > Sent: Friday, November 3, 2017 4:07 PM
> > > > > To: Kharbas, Kishor <kishor.kharbas@intel.com>; Thomas Schatzl <thoma
> > > > > s.schatzl@oracle.com>; 'hotspot-gc-dev@openjdk.java.net' <hotspot-gc-
> > > > > dev@openjdk.java.net>; hotspot-runtime-dev@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@oracle.com]
> > > > > Sent: Friday, November 3, 2017 2:38 PM
> > > > > To: Kharbas, Kishor <kishor.kharbas@intel.com>; Thomas Schatzl <thoma
> > > > > s.schatzl@oracle.com>; 'hotspot-gc-dev@openjdk.java.net' <hotspot-gc-
> > > > > dev@openjdk.java.net>; hotspot-runtime-dev@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@oracle.com]
> > > > > Sent: Friday, November 3, 2017 7:31 AM
> > > > > To: Kharbas, Kishor <kishor.kharbas@intel.com>; sangheon.kim
> > > > > <sangheon.kim@oracle.com>; 'hotspot-gc-dev@openjdk.java.net'
> > > > > <hotspot-gc-dev@openjdk.java.net>; hotspot-runtime-
> > > > > dev@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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic