RFR(M): 8204908: NVDIMM for POGC and G1GC - ReserveSpace.cpp changes are mostly eliminated/no collector specific code.

Wed Aug 29 17:17:20 UTC 2018

Hi Kishor,

On 8/29/18 9:52 AM, Kharbas, Kishor wrote:
> Hi Sangheon,
>
> Thanks for reviewing the design.
> Since the collectors do not use them for heap memory, I did not have to override VirtualSpace
Sorry, I meant ReservedSpace and its friends at 
share/memory/virtualspace.hpp.

Thanks,
Sangheon

>
> -Kishor
>> -----Original Message-----
>> From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
>> Sent: Tuesday, August 28, 2018 2:38 PM
>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Thomas Stüfe
>> <thomas.stuefe at gmail.com>; Thomas Schatzl
>> <thomas.schatzl at oracle.com>
>> Cc: hotspot-gc-dev at openjdk.java.net; Hotspot dev runtime <hotspot-
>> runtime-dev at openjdk.java.net>; Viswanathan, Sandhya
>> <sandhya.viswanathan at intel.com>; Aundhe, Shirish
>> <shirish.aundhe at intel.com>
>> Subject: Re: RFR(M): 8204908: NVDIMM for POGC and G1GC -
>> ReserveSpace.cpp changes are mostly eliminated/no collector specific code.
>>
>> Hi Kishor,
>>
>> On 8/21/18 10:57 AM, Kharbas, Kishor wrote:
>>> Hi All,
>>>
>>> Thank you for your valuable comments and feedback in this thread so far.
>>> After taken in all the comments and reading thoroughly through the code, I
>> feel that the complexity added to virtualSpace.cpp is due to lack of
>> abstraction in VirtualSpace and at GC level. NV-DIMM size and file paths are
>> being passed all the way to OS calls.
>>> So I went back to the drawing board and created a high level design to
>>> remove all the pain points of current implementation. I have uploaded
>>> the design at
>>>
>> http://cr.openjdk.java.net/~kkharbas/8202286/Design%20for%20JEP%20JDK
>> -
>>> 8202286.pdf I would love to hear your feedback and suggestions. Once
>>> we get confidence in the design, I will work on the implementation.
>> The design looks good to me.
>> If you are planning to change VirtualSpace at
>> share/memory/virtualspace.hpp, including it on the design document would
>> be helpful too.
>>
>> Probably folks involved in the previous discussions would say whether the
>> design well covers their concerns or not.
>>
>> Thanks,
>> Sangheon
>>
>>
>>> PS:
>>> 1. Vinay has transitioned to another team in Intel, so he won't be able to
>> continue on this jep.
>>> 2. If you feel this should be part of JEP discussion thread, we can take it
>> there.
>>> Thanks and regards,
>>> Kishor
>>>
>>>> -----Original Message-----
>>>> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>>>> Sent: Friday, June 22, 2018 9:25 AM
>>>> To: Thomas Schatzl <thomas.schatzl at oracle.com>
>>>> Cc: Awasthi, Vinay K <vinay.k.awasthi at intel.com>; Paul Su
>>>> <paul.su at oracle.com>; hotspot-gc-dev at openjdk.java.net; Hotspot dev
>>>> runtime <hotspot-runtime-dev at openjdk.java.net>; Viswanathan,
>> Sandhya
>>>> <sandhya.viswanathan at intel.com>; Aundhe, Shirish
>>>> <shirish.aundhe at intel.com>; Kharbas, Kishor
>>>> <kishor.kharbas at intel.com>
>>>> Subject: Re: RFR(M): 8204908: NVDIMM for POGC and G1GC -
>>>> ReserveSpace.cpp changes are mostly eliminated/no collector specific
>> code.
>>>> Hi Thomas,
>>>>
>>>> On Fri, Jun 22, 2018 at 4:44 PM, Thomas Schatzl
>>>> <thomas.schatzl at oracle.com> wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> On Tue, 2018-06-19 at 13:40 +0200, Thomas Stüfe wrote:
>>>>>> Hi Vinay,
>>>>>>
>>>>>> On Mon, Jun 18, 2018 at 6:47 PM, Awasthi, Vinay K
>>>>>> <vinay.k.awasthi at intel.com> wrote:
>>>>>>> Hi Thomas,
>>>>>>>
>>>>>>> Os::commit_memory calls map_memory_to_file which is same as
>>>>>>> os::reserve_memory.
>>>>>>>
>>>>>>> I am failing to see why os::reserve_memory can call
>>>>>>> map_memory_to_file (i.e. tie it to mmap) but commit_memory
>> can't...
>>>>>>> Before this patch, commit_memory never dealt with incrementally
>>>>>>> committing pages to device so there has to be a way to pass file
>>>>>>> descriptor and offset. Windows has no such capability to manage
>>>>>>> incremental commits. All other OSes do and that is why
>>>>>>> map_memory_to_file is used (which by the way also works on
>>>>>>> Windows).
>>>>>> AIX uses System V shared memory by default, which follows a
>>>>>> different allocation scheme (semantics more like Windows
>> VirtualAlloc...
>>>>>> calls).
>>>>>>
>>>>>> But my doubts are not limited to that one, see my earlier replies
>>>>>> and those of others. It really makes sense to step back one step
>>>>>> and discuss the JEP first.
>>>>>>
>>>>> There is already a discussion thread as David mentioned
>>>>> (http://mail.op
>>>>> enjdk.java.net/pipermail/hotspot-gc-dev/2018-May/022092.html) that
>>>>> so far nobody answered to.
>>>>>
>>>> Ah, I thought he wanted to have the JEP discussed in the comments
>>>> section of the JEP itself.
>>>>
>>>>> I believe discussion about contents the JEP and the implementation
>>>>> should be separate.
>>>>>
>>>> makes sense.
>>>>
>>>>> So far what I gathered from the responses to the implementation, the
>>>>> proposed idea itself is not the issue here (allowing the use of
>>>>> NVDIMM memory for parts of the heap for allowing the use of larger
>>>>> heaps to improve overall performance; I am not saying that the text
>>>>> doesn't need a bit more work :) ), but rather how an implementation
>>>>> of this JEP should proceed.
>>>> I have no problem with adding NVDIMM support. I think it is a cool
>> feature.
>>>> Also, the awkwardness off the memory management abstraction layer in
>>>> hotspot has always been a sore point with me (I originally
>>>> implemented the AIX mm layer in os_aix.cpp, and those are painful
>>>> memories). So, I have a lot of sympathy for Vinays struggles.
>>>> Unfortunately not much time atm, but I will respond to your mail.
>>>>
>>>>> Let's discuss the non-implementation stuff in that thread. Vinay or
>>>>> I can repost the proposal email if you do not have it any more so
>>>>> that answers will be in-thread.
>>>>>
>>>> Okay, sounds good.
>>>>
>>>> Thanks,
>>>>     Thomas
>>>>
>>>> (one of us should change his name to make this less confusing :-)
>>>>
>>>>> Thanks,
>>>>>     Thomas
>>>>>