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

Kharbas, Kishor kishor.kharbas at intel.com
Wed Sep 19 22:57:22 UTC 2018


Hi,
I have an small update to the patch to disable UseCompressedOops.
http://cr.openjdk.java.net/~kkharbas/8204908/webrev-8204908.02/
http://cr.openjdk.java.net/~kkharbas/8204908/webrev-8204908.02_to_01/

Regards,
Kishor

> -----Original Message-----
> From: Kharbas, Kishor
> Sent: Wednesday, September 19, 2018 9:35 AM
> To: 'sangheon.kim at oracle.com' <sangheon.kim at oracle.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>; 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.
> 
> Thanks Sangheon,
> 
> I have uploaded the updated patch at,
> http://cr.openjdk.java.net/~kkharbas/8204908/webrev-8204908.01/
> http://cr.openjdk.java.net/~kkharbas/8204908/webrev-8204908.01_to_00/
> 
> I have fixed all the indentation and format related comments, others I have
> pasted here below with my comments inline.
> 
> =============================
> > src/hotspot/share/gc/parallel/adjoiningGenerations.hpp
> > - Copyright update
> >
> > 62   AdjoiningGenerations();
> > - Why we need this ctor?
> >
> >> I need this default ctor for constructing
> adjoiningGenerationsForHeteroHeap, since I don't want to call the non-
> default constructor of adjoiningGenerations.
> 
> > =========================
> > /src/hotspot/share/gc/parallel/adjoiningVirtualSpaces.hpp
> > - Copyright update
> >
> >    88   virtual PSVirtualSpace* high() { return _high; }
> >    89   virtual PSVirtualSpace* low()  { return _low; }
> > - Those methods don't need 'virtual' specifier as high()/low() methods
> > are only used at ctor of AdjoiningGenerations. The other calling site
> > is ctor of AdjoiningGenerationsForHeteroHeap but it is used another
> > type of
> > high()/low() which returns _young_vs or _old_vs.
> >
> >> I feel overriding these methods is a good idea from design standpoint;
> code changes in future would take benefit of this.
> 
> ========================
> > src/hotspot/share/gc/parallel/adjoiningGenerationsForHeteroHeap.hpp
> >    45     PSVirtualSpace*    _young_vs;
> >    46     PSVirtualSpace*    _old_vs;
> > - Can't we use 'AdjoiningVirtualSpaces::_low' and '_high' instead? If
> > it is not the case, adding high(),low() may result in confusion
> > between
> > AdjoiningVirtualSpaces::high() and low(). So use other name for
> > current HeteroVirtualSpaces::high()/low().
> >
> >> AdjoiningVirtualSpaces contains two adjacent virtual spaces in the same
> reserved space and separated by a boundary. That’s why the name 'high' and
> 'low'.
> >> The class I added - HeteroVirtualSpaces, are not adjacent and do not
> share same reserved space. So I have names them '_young_vs' and '_old_vs'.
> But from outside, i.e, users of VirtualSpaces, they call high() and low() to
> access these spaces. So I have not changed the function names.
> 
> > -----Original Message-----
> > From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
> > Sent: Monday, September 17, 2018 11:32 AM
> > 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 9/4/18 10:41 PM, Kharbas, Kishor wrote:
> > > Hi,
> > > I have uploaded implementation for parallel scavenge at
> > > http://cr.openjdk.java.net/~kkharbas/8204908/webrev-8204908.00
> > > Majority of the implementation is handled in two new files -
> > adjoiningGenerationsForHeteroHeap.cpp and
> psFileBackedVirtualspace.cpp.
> > Would love to hear your feedback and suggestions.
> > Sorry for late review.
> >
> > ----------------
> > General comments.
> >
> > 1. Looks like this patch is not based on latest repository, as it
> > fails with - Wreorder issue.
> >
> > 2. I see many wrong alignment location of having only 2 spaces after
> > new line that is continued.
> > e.g.
> > a->b(c,
> > __d, xxxx); // 2 spaces
> > this should be
> > a->b(c,
> > _____d, xxx); // 'd' should locate under 'c' as those are in the same context.
> >
> > 3. I see assertion failures with below options combinations. There
> > could be more... Please run jtreg tests before posting webrev.
> > -XX:+UseLargePages -XX:+UseSHM -version -XX:+UseLargePages
> > -XX:+UseNUMA -version -XX:+UseLargePages - XX:AllocateHeapAt=.
> > -version
> >
> > =========================
> > src/hotspot/share/gc/parallel/adjoiningGenerations.cpp
> > - Copyright update
> >
> > 43   _virtual_spaces(new AdjoiningVirtualSpaces(old_young_rs,
> > policy->min_old_size(),
> > 44                   policy->min_young_size(), alignment) ) {
> > - Wrong alignment?
> >
> >
> > =========================
> > src/hotspot/share/gc/parallel/adjoiningGenerations.hpp
> > - Copyright update
> >
> > 62   AdjoiningGenerations();
> > - Why we need this ctor?
> >
> >
> > =========================
> > /src/hotspot/share/gc/parallel/adjoiningVirtualSpaces.hpp
> > - Copyright update
> >
> >    88   virtual PSVirtualSpace* high() { return _high; }
> >    89   virtual PSVirtualSpace* low()  { return _low; }
> > - Those methods don't need 'virtual' specifier as high()/low() methods
> > are only used at ctor of AdjoiningGenerations. The other calling site
> > is ctor of AdjoiningGenerationsForHeteroHeap but it is used another
> > type of
> > high()/low() which returns _young_vs or _old_vs.
> >
> >
> > =========================
> > src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp
> > - no comments.
> >
> >
> > =========================
> > src/hotspot/share/gc/parallel/psOldGen.cpp
> > =========================
> > src/hotspot/share/gc/parallel/psOldGen.hpp
> >    32 #include "gc/parallel/psFileBackedVirtualspace.hpp"
> > - Include this header file at cpp seems better rather than hpp.
> >
> > =========================
> > src/hotspot/share/gc/parallel/adjoiningGenerationsForHeteroHeap.cpp
> >     1 /*
> >     2 * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> >     3 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE
> HEADER.
> > - Wrong alignment. The second/below star should locate under first line
> star.
> > - Similarily there are more wrong alignment locations.
> >    . Line 49, 50
> >    . 52, 53
> >    . 54, 55
> >    . 64, 65
> >    . 67, 68 69 70
> >    . 72, 73 74 75 76
> >    . 79, 80
> >    . 81, 82
> >    . 85, 86
> >    . 87, 88
> >    . 106, 107 108 109
> >    . 114, 115
> >    . 166, 167 168
> >    . 178, 179 180
> >    . 186, 187 188
> >    . 218, 219 220
> >    . 230, 231 232
> >    . 239, 240 241
> >
> >
> >    59   // Initialize the virtual spaces.  Then pass the
> >    60   // a virtual to each generation for initialization of the
> > - Then pass 'the a' virtual to each generation. 'the a'?
> >
> >    64   (static_cast
> > <HeteroVirtualSpaces*>(_virtual_spaces))->initialize(max_old_byte_size
> > ,
> > init_old_byte_size,
> >    65     init_young_byte_size);
> > - Just making 'initilize' method as 'virtual' seems better.
> >
> >    39
> >
> AdjoiningGenerationsForHeteroHeap::AdjoiningGenerationsForHeteroHeap(
> > ReservedSpace
> > old_young_rs, size_t total_size_limit,
> >    40   GenerationSizer* policy, size_t alignment) :
> > _total_size_limit(total_size_limit) {
> > - Wrong alirnment at line 40. 'Generation' should be under 'ReservedSpace'
> > at line 39.
> >
> >    49   _virtual_spaces = new HeteroVirtualSpaces(old_young_rs,
> > min_old_byte_size,  70     (static_cast
> > <HeteroVirtualSpaces*>(_virtual_spaces))->max_young_size());
> >    75     (static_cast
> > <HeteroVirtualSpaces*>(_virtual_spaces))->max_old_size(),
> > - Instead assigning at line 49 and then cast to HeteroVirtualSpaces*,
> > create/initialize HeteroVirtualSpaces, get max_young/old_size() and
> > the assign to _virtual_spaces would be better alternative.
> >
> >   109   _min_young_byte_size(min_yg_byte_size),
> > _max_total_size(max_total_size) {
> >   110   _max_old_byte_size = _max_total_size - _min_young_byte_size;
> >   111   _max_young_byte_size = _max_total_size - _min_old_byte_size;
> > - _max_old_byte_size and _max_young_byte_size can move to
> > initialization list.
> >
> >   117   // This the reserved space exclusively for old generation.
> >   122   // This the reserved space exclusively for young generation.
> > - Missing 'is'? i.e. // This *is* the reserved space exclusively for
> > old generation.
> >
> >   131     vm_exit_during_initialization("Could not reserve enough space for "
> >   132       "object heap");
> > - 'object heap' can stay same line. Or changing align is necessary.
> >
> >   137     vm_exit_during_initialization("Could not reserve enough space for "
> >   138       "object heap");
> > - 'object heap' can stay same line. Or changing align is necessary.
> >
> >   152   if (uncommitted_in_old > 0) {
> > - This condition seems redundant as uncommitted_in_old is type of size_t.
> >
> >   159   if (bytes_needed == 0) {
> >   160     return true;
> >   161   }
> > - This condition, can move to inside of 'if (uncommitted_in_old > 0)'
> > condition because if uncommitted_in_old is zero, there's no way
> > bytes_needed to be zero - bytes_needed is guaranteed not to be zero
> > from caller site, so safe to move between line 154 and 155. The
> > condition to return true is 'uncommitted_size == bytes_needed &&
> > success of expand_by()'.
> >
> >   176     bool ret = _young_vs->shrink_by(shrink_size);
> >   177     assert(ret, "We should be able to shrink young space");
> > - I would prefer to add more conditions below if we fails to shrink.
> > i.e. just assert seems not enough.
> >
> >   189   _old_vs->expand_by(bytes_to_add_in_old);
> > - Check the return value of expand_by().
> >
> >   211   if (bytes_needed == 0) {
> >   212     return true;
> >   213   }
> > - Same as 'adjust_boundary_down()'
> >
> >   244   DEBUG_ONLY(size_t total_size_after =
> > _young_vs->reserved_size()
> > + _old_vs->reserved_size());
> >   245   assert(total_size_after == total_size_before, "should be
> > equal");
> > - Why adjust_boundary_up() doesn't have this kind of check?
> >
> >
> > =========================
> > src/hotspot/share/gc/parallel/adjoiningGenerationsForHeteroHeap.hpp
> >     1 /*
> >     2 * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> >     3 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE
> HEADER.
> >     4 *
> > - Wrong alignment. The second/below star should locate under first line
> star.
> >
> >    37   size_t total_size_limit() {
> >    38     return _total_size_limit;
> >    39   }
> > - I don't think we need this but if you prefer to have, add 'const'.
> >
> >    45     PSVirtualSpace*    _young_vs;
> >    46     PSVirtualSpace*    _old_vs;
> > - Can't we use 'AdjoiningVirtualSpaces::_low' and '_high' instead? If
> > it is not the case, adding high(),low() may result in confusion
> > between
> > AdjoiningVirtualSpaces::high() and low(). So use other name for
> > current HeteroVirtualSpaces::high()/low().
> >
> >    55     HeteroVirtualSpaces(ReservedSpace rs,
> >    56       size_t min_old_byte_size,
> >    57       size_t min_young_byte_size, size_t max_total_size,
> >    58       size_t alignment);
> > - Wrong alignment. Line 56 ~ 58 should be same as the location of
> > 'ReservedSpace' at line 55.
> >
> >    67     size_t max_young_size() { return _max_young_byte_size; }
> >    68     size_t max_old_size() { return _max_old_byte_size; }
> > - 'const'?
> >
> >    70     void initialize(size_t initial_old_reserved_size, size_t
> > init_low_byte_size,
> >    71       size_t init_high_byte_size);
> > - Wrong alignment
> >
> >    82   size_t reserved_byte_size();
> > - 'const'?
> >
> > =========================
> >   src/hotspot/share/gc/parallel/psFileBackedVirtualspace.cpp
> >     1 /*
> >     2 * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> > - Wrong alignment. The second/below star should locate under first line
> star.
> >
> >    36   if (_fd == -1) {
> >    37     return;
> >    38   }
> > - I prefer to have 'initialize()' method to handle when fails to
> > create the heap file. Current implementation seems easy to call additional
> 'initialize()'.
> >    Ctor of PSVirtualSpace doesn't have any failure path(e.g.
> > allocation), so no further check is needed around line 18:psOldGen.cpp.
> > But yours is different, so something should be checked.
> >
> >    33   _mapping_succeeded = false;
> >    34   _file_path = path;
> >    46   _mapping_succeeded = true;
> >    47   _special = true;
> > - Can move to initialization list with proper initial value.
> >
> >    55   assert(special(), "_special should always be true for
> > PSFileBackedVirtualSpace");
> >    66   assert(special(), "_special should always be true for
> > PSFileBackedVirtualSpace");
> > - For these 2, can we have more specific message? e.g. to include
> > meaning of expand or shrink.
> >
> >
> > =========================
> >   src/hotspot/share/gc/parallel/psFileBackedVirtualspace.hpp
> >     1 /*
> >     2 * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> > - Wrong alignment. The second/below star should locate under first line
> star.
> >
> >    37   PSFileBackedVirtualSpace(ReservedSpace rs, const char*
> > file_path);
> >    38   bool   expand_by(size_t bytes);
> > - Just recommendation to add new line between 37 and 38.
> >
> >    43 #endif //
> SHARE_VM_GC_PARALLEL_PSFILEBACKEDVIRTUALSPACE_HPP
> > - Last line is not terminated with a newline.
> >
> >
> > Thanks,
> > Sangheon
> >
> >
> > >
> > > I will post G1 GC implementation in a few days.
> > >
> > > Thanks
> > > Kishor.
> > >
> > >> -----Original Message-----
> > >> From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
> > >> Sent: Thursday, August 30, 2018 4:06 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/30/18 11:55 AM, Kharbas, Kishor wrote:
> > >>> Hi Sangheon,
> > >>>
> > >>> So far I don’t see a need to do so. I will post my implementation
> > >>> code
> > >> today or tomorrow, if we see a need or any simplification by
> > >> changing classes in VirtualSpace.hpp, we can discuss that.
> > >> Okay.
> > >>
> > >> Thanks,
> > >> Sangheon
> > >>
> > >>
> > >>> Regards,
> > >>> -Kishor
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: sangheon.kim at oracle.com
> [mailto:sangheon.kim at oracle.com]
> > >>>> Sent: Wednesday, August 29, 2018 10:17 AM
> > >>>> 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/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
> > >>>>>>>>>



More information about the hotspot-gc-dev mailing list