RFR(M): 8211425: Allocation of old generation of java heap on alternate memory devices - G1 GC
Kharbas, Kishor
kishor.kharbas at intel.com
Fri Dec 7 07:34:45 UTC 2018
Hi,
JTreg regression testing shows 1 regression when we use patched JDK without flag
- runtime/appcds/cacheObject/ArchivedIntegerCacheTest.java
I looked at the log, but don't seem to have a handle on this. Any pointers on how appCDS is affected by the patch? Error is,
Exception in thread "main" java.lang.RuntimeException: FAILED. Value expected to be archived: -128 of type java.lang.Byte
at CheckIntegerCacheApp.checkArchivedAsExpected(CheckIntegerCacheApp.java:109)
at CheckIntegerCacheApp.main(CheckIntegerCacheApp.java:73)
]
exitValue = 1
And 3 regressions when additional flag - AllocateOldGenAt is specified. I have an idea what is going on here.
+gc/TestSoftReferencesBehaviorOnOOME.java
+gc/g1/TestGreyReclaimedHumongousObjects.java
+gc/g1/humongousObjects/TestNoAllocationsInHRegions.java
Thanks
Kishor
> -----Original Message-----
> From: Kharbas, Kishor
> Sent: Thursday, December 6, 2018 5:24 PM
> To: Stefan Johansson <stefan.johansson at oracle.com>;
> 'sangheon.kim at oracle.com' <sangheon.kim at oracle.com>
> Cc: 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-dev at openjdk.java.net>;
> 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Kharbas, Kishor
> <kishor.kharbas at intel.com>
> Subject: RE: RFR(M): 8211425: Allocation of old generation of java heap on
> alternate memory devices - G1 GC
>
> Hi Stefan, Sangheon,
>
> My replies are inline.
> Updated webrev -
> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.08/
>
> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.07_to_08
>
> Thank you.
> Kishor
>
> > -----Original Message-----
> > From: Stefan Johansson [mailto:stefan.johansson at oracle.com]
> > Sent: Thursday, December 6, 2018 6:03 AM
> > To: Kharbas, Kishor <kishor.kharbas at intel.com>;
> > 'sangheon.kim at oracle.com' <sangheon.kim at oracle.com>
> > Cc: 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
> dev at openjdk.java.net>;
> > 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> > Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
> > Subject: Re: RFR(M): 8211425: Allocation of old generation of java heap on
> > alternate memory devices - G1 GC
> >
> > Hi Kishor,
> >
> > On 2018-12-06 10:05, Kharbas, Kishor wrote:
> > > Hi,
> > >
> > > I have new webrev (removed Parallel GC part, but couldn't get rid of files
> > showing up with 0 lines changed) -
> > http://cr.openjdk.java.net/~kkharbas/8211425/webrev.07
> > > Incremental webrev -
> > http://cr.openjdk.java.net/~kkharbas/8211425/webrev.06_to_07/
> > One new comment here and then the replies to your fixes are below.
> > src/hotspot/share/gc/g1/heapRegionManager.cpp
> > ---------------------------------------------
> > 71 HeapRegionManager*
> > HeapRegionManager::create_manager(G1CollectedHeap* heap,
> > CollectorPolicy* policy) {
> > 72 if (static_cast<G1CollectorPolicy*>(policy)->is_hetero_heap()) {
> > 73 return new
> > HeterogeneousHeapRegionManager((uint)(policy->max_heap_byte_size() /
> > HeapRegion::GrainBytes) /*heap size as num of regions*/);
> > 74 }
> >
> > If you pass in a G1CollectorPolicy you can avoid the cast.
> >
> [Kharbas, Kishor] Thanks.
> > >
> > > I am replying to Sangheon's comments inline, and have copied below
> > Stefan's comments from previous two emails below with my replies.
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > Thanks for providing a separate webrev for this.
> > >
> > > I'm not sure I understand why we need a special region type. What is the
> > problem of having Old regions in DRAM, if we have an evacuation failure?
> > > I understand that it is not what we want with this feature, but by allowing
> > to go over Xmx during the GC we should avoid most occurrences of it. I can
> > happen during Mixed collections, but if it does we are probably close to run
> > into a Full GC.
> > >
> > >>> With the newer way (borrowing regions) we do not need special region
> > type. I removed it from latest webrev.
> > Great, thanks.
> >
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > Here are my comments on the patch:
> > > src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> > > -------------------------------------------
> > > 4558 if (new_alloc_region == NULL && is_hetero_heap() && !is_survivor)
> {
> > > 4559 // if it is a heterogenous heap, we can always copy objects
> > > from young generation even if
> > > 4560 // old generation is full. We can borrow (or pre-allocate)
> > > regions in nv-dimm to serve as allocation region.
> > > 4561 new_alloc_region =
> > > static_cast<HeterogeneousHeapRegionManager*>(hrm())-
> > >borrow_old_region_for_gc();
> > > 4562 assert(new_alloc_region != NULL, "We should get a non-null
> > > region");
> > > 4563 }
> > >
> > > This logic should live in
> > > HeterogeneousHeapRegionManager::allocate_free_region(). This
> function
> > should return NULL only when we are really out of regions (that is no
> regions
> > can be borrowed).
> > >
> > >>> allocate_free_region() is called by mutators too. Humongous calls this
> > function with 'old' = true. So we won't be able to differentiate whether the
> > call is for gc old region allocation or for humongous.
> > >>> Is it a good idea to add a new hrm method to be called for gc allocation
> > and override it in HeterogeneousHeapRegionManager?
> > I think instead changing G1CollectedHeap::new_region() and
> > HeapRegionManager::allocate_free_region() to take a HeapRegionType
> > instead of a bool is better. Then the different callers can better pass
> > in what they really are.
> >
> > I think the HeapRegionType needs to be extended a bit to handle this,
> > I'm thinking something like:
> > http://cr.openjdk.java.net/~sjohanss/8211425/HeapRegionType-for-
> > new_region/
> >
> [Kharbas, Kishor] Thank you. I liked your idea and used it for the
> implementation. Now there is no call to borrow_old_region_for_gc() from
> outside, allocate_free_region() handles this.
>
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
> > > ------------------------------------------------
> > > 227 // Third clause: if it is a heterogenous heap, we borrow regions
> > > from nv-dimm for old gc allocation.
> > > 228 // This region can only be used to evacate young objects, so we
> > > check the state of the object being copied.
> > > 229 if (_old_gen_is_full && dest_state.is_old() ||
> > > 230 (_g1h->is_hetero_heap() && state.is_old() &&
> > > static_cast<HeterogeneousHeapRegionManager*>(_g1h->hrm())-
> > >is_old_full())
> > > ) {
> > >
> > > Instead of doing this extra clause I think we should make sure
> > _old_gen_is_full is correct even for hetero heaps. I don't really see why it
> > shouldn't be already.
> > >
> > >>> 'this->_old_gen_is_full' is set true in allocate_in_next_plab() which is
> the
> > final attempt for allocation in default code path.
> > >>> However when hetero heap is enabled, allocate_in_next_plab() may
> not
> > be called since we allocate plab from borrowed region when old is full.
> >
> > Ok, I think I see your point now. If "borrowing" regions you only want
> > to use them for young objects. But adding the or clause here will mean
> > we will add extra checks for each evacuated object and I really want to
> > avoid that, since it will affect normal G1 quite a bit potentially.
> >
> [Kharbas, Kishor] I am able to avoid this extra check on fast path by setting
> _old_gen_is_full flag after allocate_direct_or_new_plab().
> After allocate_direct_or_new_plab(), we check whether we
> had to borrow any region for this allocation and set _old_gen_is_full
> accordingly.
> At the top, only when _old_gen_is_full is true, we need
> additional check for whether source is young or old.
>
> > An alternative to this case would be to allow borrowing for all, and if
> > we after the GC realize we can't get down to Xmx number of regions
> > trigger a Full GC to solve this.
> >
> > When talking about the Full GC, I think there might be potential
> > problems there. How do you make sure all objects are moved to old. The
> > normal algorithm move stuff to the bottom of the heap by compacting
> > chains of heap regions and tagging the regions as old after the
> > collection. For the nv-dimm case I can't see how you would handle
> > regions still in dram to move them into nv-dimm.
> >
> [Kharbas, Kishor] Yes, if we allow borrowing for all and complete the GC with
> extra regions, new allocations could increase the live set to more than Xmx.
> Even a Full GC would not help.
>
> > >
> > >
> >
> ================================================================
> > =====================================================
> > >
> > > src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> > > -------------------------------------------
> > > 1493 _is_hetero_heap(AllocateOldGenAt != NULL),
> > >
> > > Does G1CollectedHeap really need to know this, can't we always just as
> the
> > G1CollectorPolicy? As a matter of fact, I think most places where we check
> > is_hetro_heap() a better solution is to use virtual calls or other abstractions
> > to do the right thing.
> > >
> > >>> I moved this to G1CollectorPolicy. I got rid of few places where
> > is_hetero_heap() is needed by using abstractions (specialized G1Policy).
> > Looks good.
> >
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > 1786 // Now we know the target length of young list. So adjust the
> > > heap to provision that many regions on dram.
> > > 1787 if (is_hetero_heap()) {
> > > 1788
> > > static_cast<HeterogeneousHeapRegionManager*>(hrm())-
> > >adjust_dram_regions((uint)g1_policy()->young_list_target_length(),
> > > workers());
> > > 1789 }
> > >
> > > Here we have an example of what I mentioned above. Instead of having
> > the check and the cast, is there a way we can make this happen every time
> > we update the young_list_target_length(). I see that we currently only do
> this
> > at initialization and after full collections, why don't we have to do this after
> > each GC?
> > >
> > > Same comment for lines 2532-2535.
> > >
> > >>> young_list_target_length is updated from other places such as
> > g1YoungRemSetSamplingThread. In which case we have to acquire heap
> lock
> > or check whether it’s the VMThread and lock is owned.
> > >>> I had adjust_dram_regions() call here earlier, but later moved it out
> > based on comments.
> > >>> In latest webrev, I call adjust_dram_regions() from G1Policy->init(),
> > record_collection_pause_end() and record_full_collection_end()
> >
> > I like this approach much better.
> >
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > src/hotspot/share/gc/g1/heapRegionManager.cpp
> > > ---------------------------------------------
> > > 71 HeapRegionManager*
> > > HeapRegionManager::create_manager(G1CollectedHeap* heap,
> > > CollectorPolicy* policy) {
> > > 72 if (heap->is_hetero_heap()) {
> > > 73 return new
> > > HeterogeneousHeapRegionManager((uint)(policy->max_heap_byte_size()
> /
> > > HeapRegion::GrainBytes) /*heap size as num of regions*/);
> > > 74 }
> > > 75 return new HeapRegionManager();
> > > 76 }
> > >
> > > As I mentioned above, I think the is_hetero check should be done on the
> > policy instead. Otherwise this is a place where this check is really needed.
> > >
> > >>> Done.
> > Nice.
> >
> > >
> > >
> >
> ================================================================
> > =====================================================
> > >
> > > src/hotspot/share/gc/g1/heapRegionSet.hpp
> > > -----------------------------------------
> > > 198 uint num_of_regions_in_range(uint start, uint end) const;
> > >
> > > Was looking at the usage of this method and it is only used to get the
> length
> > of the dram free-list. Looking at the HeteroHeapRegionManager wouldn't it
> > be possible to add a second free-list, so we have one for dram and one for
> > nvdimm.
> > >
> > >>> I had given this a thought when I started the implementation, but I
> chose
> > to keep a single list so that I can re-use base class (HeapRegionManager)
> > functions. For example, insert_into_free_list(),
> > allocate_free_regions_starting_at(), etc.
> > > Since the regions in free_list are already sorted, I did not have to
> change
> > a lot in free list management.
> > Ok, I'm fine with this if you think it makes your implementation easier.
> >
> > >
> > >
> >
> ================================================================
> > =====================================================
> > > src/hotspot/share/runtime/arguments.cpp
> > > ---------------------------------------
> > > 1625 if (AllocateOldGenAt != NULL) {
> > > 1626 FLAG_SET_ERGO(bool, UseCompressedOops, false);
> > > 1627 return;
> > > 1628 }
> > >
> > > Any reason this can't be done in GCArguments::initialize()?
> > >
> > >>> I moved it to GCArguments.
> > Perfect.
> >
> > >
> >
> ================================================================
> > =====================================================
> > >
> > > src/hotspot/share/gc/g1/heterogeneousHeapRegionManager.hpp
> > > ----------------------------------------------------------
> > > 85 // Empty constructor, we'll initialize it with the initialize()
> > > method.
> > > 86 HeterogeneousHeapRegionManager(uint num_regions) :
> > > _max_regions(num_regions), _max_dram_regions(0),
> > > 87
> > > _max_nvdimm_regions(0), _start_index_of_nvdimm(0)
> > > 88 {}
> > >
> > > The member _total_commited_before_full_gc is not initialized.
> > >
> > >>> Fixed
> > Thumbs up.
> >
> > >
> >
> ================================================================
> > ======================================================
> > > 98 // Adjust dram_set to provision 'expected_num_regions' regions.
> > > 99 void adjust_dram_regions(uint expected_num_regions,
> WorkGang*
> > > pretouch_workers);
> > >
> > > As mentioned I would turn this into a virtual method if it can't be
> > abstracted in another way. The WorkGang can be obtained by calling
> > > G1CollectedHeap::heap()->workers() so you don't need to pass it in. This
> > way we get a cleaner interface I think.
> > >
> > >>> Thanks.
> > Thanks for addressing my comments,
> > Stefan
> >
> > >
> >
> ================================================================
> > ======================================================
> > >
> > >
> > >> -----Original Message-----
> > >> From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
> > >> Sent: Wednesday, December 5, 2018 3:02 PM
> > >> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Stefan Johansson
> > >> <stefan.johansson at oracle.com>
> > >> Cc: 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
> > dev at openjdk.java.net>;
> > >> 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> > >> Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
> > >> Subject: Re: RFR(M): 8211425: Allocation of old generation of java heap
> on
> > >> alternate memory devices - G1 GC
> > >>
> > >> Hi Kishor,
> > >>
> > >> On 12/5/18 12:51 AM, Kharbas, Kishor wrote:
> > >>> Hi Stefan, Sangheon,
> > >>> I have another webrev update -
> > >>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.06
> > >>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.05_to_06
> > >>>
> > >>> Changes are:
> > >>> 1. While testing the new evacuation handling mechanism, I identified
> > some
> > >> issues and fixed them. The implementation has been tested by tracing
> the
> > >> code path through the debugger.
> > >>> Based on the fix, here is an explanation for evacuation failure
> > handling -
> > >>> 1) The implementation is based on the axiom that for every
> > region
> > >> in dram where is a unavailable (not 'committed') region in nv-dimm. By
> > >> borrowing such regions, we can avoid evacuation failure for young
> objects
> > >> and thus avoid young->old region conversion.
> > >>> 2) We cannot use borrowed regions for old objects. Since in
> > worst
> > >> case for evacuating 'n' young and 'm' old regions we need 'n + m'
> > destination
> > >> regions. And we may need to borrow 'n+m' from nv-dimm. But we can
> > only
> > >> guarantee 'n' borrowed regions.
> > >>> So we use borrowed region only for copying young objects,
> > else
> > >> we cannot avoid young->old conversion and will need a special region
> > type
> > >> as in current implementation anyway.
> > >>> 3) When we cannot allocate a new region for
> OldGCAllocRegion,
> > we
> > >> borrow a region from hrm() to use as allocation region. We release (un-
> > >> commit) that many number of regions in adjust_dram_regions() which is
> > >> called at end of GC.
> > >>> 4) So we may be in situation where there is valid old plab and
> > >> OldGCAllocRegion but we can only use them when source is young. So
> we
> > >> add another clause to the if() statement at the beginning of
> > >> copy_to_survivor_space() to check for this condition.
> > >>> For ease of review I extracted evacuation failure handling
> > >>> changes here -
> > >>>
> > http://cr.openjdk.java.net/~kkharbas/8211425/webrev_EvacuationFailure
> > >> I have some comments.
> > >>
> > >> 1. Have you test enough for evacuation failure handling case? i.e. it
> would
> > be
> > >> better to have tests for these cases.
> > >> e.g. How can we guarantee the total heap will be less than Xmx after
> > GC if
> > >> we followed 'borrowing region path'.
> > >> Couldn't shrinking dram fail after borrowing a region?
> > >> (heterogeneousHeapRegionManager.cpp:65)
> > >>
> > > [Kharbas, Kishor] Since we only copy objects from young generation, we
> > will never borrow more than the length of young generation in the
> collection
> > set. Also we should completely evacuate the young generation, so when
> > collection set is freed, we have that many free regions to shrink.
> > >
> > >> 2. Do we still need pre-mature old type after introducing 'borrowing
> > >> concept'? (Stefan also mentioned about this)
> > >>
> > > [Kharbas, Kishor] Done (I had kept it for removal later)
> > >
> > >> 3. (Again) Please don't merge ParallelGC patch(8211424) into G1
> > >> Patch(8211425) as those should be committed separately. And this email
> > >> thread is to review G1. :) i.e. you should commit identical reviewed
> patch,
> > >> not modifying it (to split Parallel part).
> > >>
> > > [Kharbas, Kishor] Done.
> > >> --------------------------------------
> > >> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> > >> 4561 new_alloc_region =
> > >> static_cast<HeterogeneousHeapRegionManager*>(hrm())-
> > >>> borrow_old_region_for_gc();
> > >> - This can be moved to hetero hrm. Stefan also mentioned this.
> > >>
> > >>
> > > [Kharbas, Kishor] I could not find an existing method in hrm to handle
> this.
> > Two methods to allocate regions are allocate_free_region() and
> > allocate_free_regions_starting_at().
> > > Allocate_free_region() is also called for humongous
> > allocation with 'old' parameter so I can't issue a borrowed region in every
> > call.
> > > Please let me know if you think add a new hrm method
> > for use in this code and abstracting it, seems a good solution.
> > >> --------------------------------------
> > >> src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
> > >> 227 // Third clause: if it is a heterogenous heap, we borrow regions
> > >> from nv-dimm for old gc allocation.
> > >> 228 // This region can only be used to evacate young objects, so we
> > >> check the state of the object being copied.
> > >> - Something like this:
> > >> // Third clause: if it is a heterogenous heap, we can borrow regions
> > >> from nv-dimm for old region allocation. And these regions should be
> only
> > >> used to evacuate young objects.
> > >>
> > > [Kharbas, Kishor] Done.
> > >
> > >> 229 if (_old_gen_is_full && dest_state.is_old() ||
> > >> 230 (_g1h->is_hetero_heap() && state.is_old() &&
> > >> static_cast<HeterogeneousHeapRegionManager*>(_g1h->hrm())-
> > >>> is_old_full())
> > >> ) {
> > >> - It would be better to have another parenthesis at line 229.
> > >>
> > >>
> > > [Kharbas, Kishor] Done.
> > >
> > >> --------------------------------------
> > >> src/hotspot/share/gc/g1/heterogeneousHeapRegionManager.cpp
> > >> 65 uint num = shrink_dram(_no_borrowed_regions);
> > >> 66 assert(num == _no_borrowed_regions, "Should be able to
> > >> uncommit the number of regions borrowed for evacuation failure
> > handling");
> > >> - Couldn't we fail at line 65?
> > >>
> > > [Kharbas, Kishor] Replied above.
> > >>
> > >> --------------------------------------
> > >> src/hotspot/share/gc/g1/heterogeneousHeapRegionManager.hpp
> > >> 45 bool _is_old_full;
> > >> - This name looks mis-leading. It sets true when we temporarily borrow
> a
> > >> region from nvdimm during GC. To align with '_no_borrowed_regions',
> > >> something like 'has_borrowed_regions'?
> > >>
> > > [Kharbas, Kishor] Done.
> > >> 135 bool is_old_full();
> > >> - 'const'?
> > >>
> > > [Kharbas, Kishor] Replace this method with has_borrowed_regions().
> > >>
> > >>> 2. There is a fix for incorrect handling of humongous allocations under
> > >> certain situations.
> > >>> 3. My jtreg tests are running, will provide the results when they finish.
> > >> I guess you are running serviceability as well?
> > >>
> > > [Kharbas, Kishor] Yes I have added them.
> > >
> > >> Thanks,
> > >> Sangheon
> > >>
> > >>
> > >>>
> > >>> Let me know your feedback.
> > >>>
> > >>> Thank you.
> > >>> Kishor
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Kharbas, Kishor
> > >>>> Sent: Monday, December 3, 2018 11:48 PM
> > >>>> To: sangheon.kim at oracle.com; Stefan Johansson
> > >>>> <stefan.johansson at oracle.com>
> > >>>> Cc: 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
> > >> dev at openjdk.java.net>;
> > >>>> 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> > >>>> Viswanathan, Sandhya <sandhya.viswanathan at intel.com>; Kharbas,
> > >> Kishor
> > >>>> <kishor.kharbas at intel.com>
> > >>>> Subject: RE: RFR(M): 8211425: Allocation of old generation of java
> heap
> > >> on
> > >>>> alternate memory devices - G1 GC
> > >>>>
> > >>>> Hi Sangheon and Stefan,
> > >>>>
> > >>>> I have updated webrev for this feature,
> > >>>>
> > >>>> 1. Webrev with Stefan's comments on Parallel GC (abstraction related
> > >>>> comments) :
> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.04
> > >>>>
> > >>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.03_to_04
> > >>>>
> > >>>> 2. Webrev with evacuation failure handling :
> > >>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.05
> > >>>>
> > >>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.04_to_05
> > >>>> Testing underway.
> > >>>>
> > >>>> 3. Explanation of evacuation failure handling:
> > >>>> 1) The implementation is based on the axiom that for every
> > region
> > >> in
> > >>>> dram where is a unavailable (not 'committed') region in nv-dimm. By
> > >>>> borrowing such regions, we can avoid evacuation failure for young
> > >> objects
> > >>>> and thus avoid young->old region conversion.
> > >>>> 2) We cannot use borrowed regions for old objects. Since in
> > worst
> > >>>> case for evacuating 'n' young and 'm' old regions we need 'n + m'
> > >> destination
> > >>>> regions. And we may need to borrow 'n+m' from nv-dimm. But we
> can
> > >> only
> > >>>> guarantee 'n' borrowed regions.
> > >>>> So we use borrowed region only for copying young objects,
> > else
> > >> we
> > >>>> cannot avoid young->old conversion and will need a special region
> type
> > as
> > >> in
> > >>>> current implementation anyway.
> > >>>> 3) When we cannot allocate a new region for
> OldGCAllocRegion,
> > >> we
> > >>>> borrow a region from hrm() to use as allocation region. We sort of
> > >> release
> > >>>> that many number of regions in adjust_dram_regions() which is called
> > at
> > >> end
> > >>>> of GC.
> > >>>> 4) When G1ParScanThreadState: _old_gen_is_full is true, we
> > only
> > >> let
> > >>>> young objects be allocated from the borrowed region. There might be
> a
> > >> race
> > >>>> condition here, when one thread enters plab_allocate() while other
> > thread
> > >>>> determines and set _old_gen_is_full.
> > >>>> But we can tolerate this race condition.
> > >>>>
> > >>>> 4. I am testing all tier1 tests and appCDS tests with the combined
> patch.
> > >> Will
> > >>>> provide an update soon.
> > >>>>
> > >>>> Thanks,
> > >>>> Kishor
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: sangheon.kim at oracle.com
> [mailto:sangheon.kim at oracle.com]
> > >>>>> Sent: Sunday, December 2, 2018 2:49 PM
> > >>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Stefan Johansson
> > >>>>> <stefan.johansson at oracle.com>
> > >>>>> Cc: 'hotspot-gc-dev at openjdk.java.net'
> > >>>>> <hotspot-gc-dev at openjdk.java.net>;
> > >>>>> 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> > >>>>> Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
> > >>>>> Subject: Re: RFR(M): 8211425: Allocation of old generation of java
> > >>>>> heap on alternate memory devices - G1 GC
> > >>>>>
> > >>>>> Hi Kishor,
> > >>>>>
> > >>>>> On 11/29/18 12:19 PM, Kharbas, Kishor wrote:
> > >>>>>> Hi Sangheon,
> > >>>>>>
> > >>>>>> Thank you for reviewing the webrev.
> > >>>>>> 1. Based on your feedback I have newer webrev. There are some
> > >>>>> comments inline about AppCDS (top of previous email).
> > >>>>>> 2. Webrev with your feedback on G1 GC patch :
> > >>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.02/
> > >>>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.01_to_02/
> > >>>>> Webrev.02 looks good.
> > >>>>>
> > >>>>>> 3. I merged the Parallel GC patch with your latest comment,
> > >>>>>> consolidated jtreg tests and combined webrev is :
> > >>>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.03
> > >>>>> I meant to work based on Parallel GC patch so that your G1 patch
> > >>>>> doesn't need to comment out Paralell GC test case at newly added
> > tests.
> > >>>>>
> > >>>>> I only checked commented out parts of Parallel GC are enabled. This
> > >>>>> looks good too.
> > >>>>> I would expect that you completed running whole your patches(G1
> > and
> > >>>>> Parallel GC) together without any failures.
> > >>>>>
> > >>>>>> 4. I moved the CSR to finalized. Thanks for your review. We will
> > >>>>>> wait to
> > >>>>> hear from CSR committee.
> > >>>>> Okay, I saw it is approved now.
> > >>>>>
> > >>>>>> 5. I am working on Stefan's latest feedback on handling of
> > >>>>>> evacuation
> > >>>>> failure.
> > >>>>> OK.
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Sangheon
> > >>>>>
> > >>>>>
> > >>>>>> Regards,
> > >>>>>> Kishor
> > >>>>>>
> > >>>>>>> -----Original Message-----
> > >>>>>>> From: sangheon.kim at oracle.com
> > >> [mailto:sangheon.kim at oracle.com]
> > >>>>>>> Sent: Tuesday, November 27, 2018 9:25 PM
> > >>>>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Stefan
> Johansson
> > >>>>>>> <stefan.johansson at oracle.com>
> > >>>>>>> Cc: 'hotspot-gc-dev at openjdk.java.net' <hotspot-gc-
> > >>>>> dev at openjdk.java.net>;
> > >>>>>>> 'Hotspot dev runtime' <hotspot-runtime-dev at openjdk.java.net>;
> > >>>>>>> Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
> > >>>>>>> Subject: Re: RFR(M): 8211425: Allocation of old generation of java
> > >>>>>>> heap
> > >>>>> on
> > >>>>>>> alternate memory devices - G1 GC
> > >>>>>>>
> > >>>>>>> Hi Kishor,
> > >>>>>>>
> > >>>>>>> On 11/19/18 6:20 PM, Kharbas, Kishor wrote:
> > >>>>>>>> Hi Sangheon,
> > >>>>>>>>
> > >>>>>>>> Here is the incremental webrev -
> > >>>>>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.00_to_01
> > >>>>>>> Thanks for the incremental webrev.
> > >>>>>>> Here are some comments for webrev.01.
> > >>>>>>>
> > >>>>>>> 1. General comments:
> > >>>>>>> - Any update with AppCDS tests? (You mentioned this on your
> > >>>>>>> previous
> > >>>>> email)
> > >>>>>> [Kharbas, Kishor] I completed testing of AppCDS with : 1) original
> > >>>>>> code. 2)
> > >>>>> patch applied, but flag not used. 3) patch applied, flag set.
> > >>>>>> Test results at :
> > >>>>> http://cr.openjdk.java.net/~kkharbas/8211425/JT_AppCDS
> > >>>>>> 1) and 2) both show "Test results:
> > >>>>>> passed: 120; failed: 1;
> > >>>>> error: 2"
> > >>>>>> 3) No tests are run, all are
> > >>>>>> skipped. Do you know if
> > >>>>> appCDS does not work with certain flags. I use jtreg flag
> > >>>>> -javaoptions:"- XX:+UnlockExperimentalVMOptions -
> > >>>> XX:AllocateOldGenAt=/home/kishor".
> > >>>>>>> - Please test with ParallelGC patch added. i.e. Applied both
> > >>>>>>> ParallelGC and
> > >>>>> G1
> > >>>>>>> parts, so that you don't need to comment out ParallelGC on new
> > tests.
> > >>>>>>>
> > >>>>>> [Kharbas, Kishor] The tests pass with merged patch, testing both
> > >>>>>> ParallelGC
> > >>>>> and G1.
> > >>>>>>> 2. New tests related general comments:
> > >>>>>>> - As AllocateOldGenAt feature will not be tested regularly, new
> > >>>>>>> tests
> > >>>>> should
> > >>>>>>> be excluded by default.
> > >>>>>> [Kharbas, Kishor] How do I do this? Should I remove these test from
> > >>>>> TEST.groups?
> > >>>>>>> - New tests need filters for other unsupported GCs.
> > >>>>>>> e.g. @requires vm.gc=="null" | vm.gc.G1 | vm.gc.Parallel
> > >>>>>> [Kharbas, Kishor] Done.
> > >>>>>>> - Are there any reason for printing stdout in new tests? i.e. looks
> > >>>>>>> like we don't need 'System.out.println(output.getStdout());'
> > >>>>>>>
> > >>>>>> [Kharbas, Kishor] not really, I checked that output is dumped to
> > >>>>>> stdout of
> > >>>>> jtreg anyway. So I removed this.
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/g1CollectedHeapHeap.hpp
> > >>>>>>> 45 #include "gc/g1/heapRegionManager.hpp"
> > >>>>>>> 46 #include "gc/g1/heterogeneousHeapRegionManager.hpp"
> > >>>>>>> 47 #include "gc/g1/heapRegionSet.hpp"
> > >>>>>>> - Line 46 should be below line 47.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/g1CollectorPolicy.hpp
> > >>>>>>> 41 virtual size_t heap_reservation_size_bytes();
> > >>>>>>> - const?
> > >>>>>>>
> > >>>>>>> 45 size_t _heap_reservation_size_bytes;
> > >>>>>>> - Do we really need this variable? Just returning
> > >>>>>>> 2*_max_heap_byte_size seems enough. In this case, adding
> virtual
> > >>>>>>> size_t
> > >>>>>>> heap_reservation_size_bytes() seems enough.
> > >>>>>>> - Regarding naming, how about heap_reserved_size_bytes()?
> > >>>>>>>
> > >>>>>>> 51 size_t heap_reservation_size_bytes();
> > >>>>>>> - I would prefer to have 'virtual' for readability. I saw you
> > >>>>>>> reflected this
> > >>>>> kind
> > >>>>>>> of my comment in other places so asking here too.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
> > >>>>>>> 25 #include "logging/log.hpp"
> > >>>>>>> - precompiled.hpp comes first
> > >>>>>>>
> > >>>>>>> 180 return false;
> > >>>>>>> 181 log_error(gc, init)("Could not create file for Old
> > >>>>>>> generation at location %s", AllocateOldGenAt);
> > >>>>>>> - We never reach line 181 because of line 180? :)
> > >>>>>>>
> > >>>>>>> 196
> > >>>>>>>
> > >>>>
> > >>
> > G1RegionToHeteroSpaceMapper::G1RegionToHeteroSpaceMapper(Reserve
> > >>>>>>> dSpace rs,
> > >>>>>>> - My previous comment was about splitting initialization
> part(which
> > >>>>>>> does actual file-heap mapping) from ctor, so that we can avoid
> > >>>>>>> failure during construction. After construction, call initialize
> > >>>>>>> method to do file mapping and then report any failures if have.
> > >>>>>>>
> > >>>>>>> 196
> > >>>>>>>
> > >>>>
> > >>
> > G1RegionToHeteroSpaceMapper::G1RegionToHeteroSpaceMapper(Reserve
> > >>>>>>> dSpace rs,
> > >>>>>>> 197 size_t actual_size,
> > >>>>>>> 198 size_t page_size,
> > >>>>>>> 199 size_t alloc_granularity,
> > >>>>>>> 200 size_t commit_factor,
> > >>>>>>> 201 MemoryType type) :
> > >>>>>>> 202 G1RegionToSpaceMapper(rs, actual_size, page_size,
> > >>>>>>> alloc_granularity, commit_factor, type),
> > >>>>>>> 203 _num_committed_dram(0),
> _num_committed_nvdimm(0),
> > >>>>>>> _success(false) {
> > >>>>>>> - Looks like a bad alignment. I think parameters and initialization
> > >>>>>>> list(parental class as well) should be distinguishable.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> 295 delete mapper;
> > >>>>>>> - Existing issue(no action needed): current mapper code doesn't
> > >>>>>>> have a destructor.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/heapRegionManager.cpp
> > >>>>>>> 29 #include "gc/g1/heapRegionManager.inline.hpp"
> > >>>>>>> 30 #include "gc/g1/heterogeneousHeapRegionManager.hpp"
> > >>>>>>> 31 #include "gc/g1/heapRegionSet.inline.hpp"
> > >>>>>>> - Move line 30 after line 31.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/heapRegionManager.hpp
> > >>>>>>>
> > >>>>>>> 117 FreeRegionList _free_list;
> > >>>>>>> 118 void make_regions_available(uint index, uint num_regions
> =
> > >>>>>>> 1,
> > >>>>>>> WorkGang* pretouch_gang = NULL);
> > >>>>>>> - New line between 117 and 118 please.
> > >>>>>>>
> > >>>>>>> 130 static HeapRegionManager*
> > >> create_manager(G1CollectedHeap*
> > >>>>>>> heap,
> > >>>>>>> CollectorPolicy* policy);
> > >>>>>>> - I see build failure in solaris.
> > >>>>>>> open/src/hotspot/share/gc/g1/heapRegionManager.hpp", line
> 129:
> > >>>> Error:
> > >>>>>>> CollectorPolicy is not defined.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/heapRegionType.cpp
> > >>>>>>> - Copyright update
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/shared/gcArguments.cpp
> > >>>>>>>
> > >>>>>>> 68 "AllocateOldGenAt not supported for selected GC.\n");
> > >>>>>>> - "AllocateOldGenAt *is* not supported ..."
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/prims/whitebox.cpp
> > >>>>>>> 510 return (jlong)(Universe::heap()->base() + start_region
> > >>>>>>> * HeapRegion::GrainBytes);
> > >>>>>>> - g1h->base() is same, isn't it? There are more lines that using
> > >>>>>>> Universe::heap()->base().
> > >>>>>>>
> > >>>>>>> 524 }
> > >>>>>>> 525 else {
> > >>>>>>> and
> > >>>>>>> 551 }
> > >>>>>>> 552 else {
> > >>>>>>> - Make at one line. i.e. '} else {'
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/runtime/arguments.cpp
> > >>>>>>> 1625 if (AllocateOldGenAt != NULL) {
> > >>>>>>> 1626 FLAG_SET_ERGO(bool, UseCompressedOops, false);
> > >>>>>>> 1627 return;
> > >>>>>>> 1628 }
> > >>>>>>> - Wrong alignment. Needs 2 spaces for line 1625~1628.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/runtime/globals.hpp
> > >>>>>>> 2606 experimental(ccstr, AllocateOldGenAt, NULL,
> > >>>>>>> \
> > >>>>>>> 2607 "Directory to use for allocating old
> > >>>>>>> generation")
> > >>>>>>> - How about moving AllocateOldGenAt around AllocateHeapAt
> > option.
> > >>>>>>> - Change the description similar to AllocateHeapAt as it explains
> > >> better.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/heterogeneousHeapRegionManager.cpp
> > >>>>>>> 29 #include "gc/g1/heapRegionManager.inline.hpp"
> > >>>>>>> 30 #include "gc/g1/heterogeneousHeapRegionManager.hpp"
> > >>>>>>> 31 #include "gc/g1/heapRegionSet.inline.hpp"
> > >>>>>>> - Line 30 should be located after line 31.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> src/hotspot/share/gc/g1/heterogeneousHeapRegionManager.hpp
> > >>>>>>>
> > >>>>>>> #ifndef
> > SHARE_VM_GC_G1_HeterogeneousHeapRegionManager_HPP
> > >>>>>>> - Should be all uppercase letters.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> ----------------------
> > >>>>>>> test/hotspot/jtreg/gc/8202286/TestAllocateOldGenAtMultiple.java
> > >>>>>>> 56 "-Xmx4g -Xms4g", // 4.
> > >>>>>>> With larger heap size (UnscaledNarrowOop not possible).
> > >>>>>>> 57 "-Xmx4g -Xms4g -XX:+UseLargePages", // 5.
> > >>>>>>> Set UseLargePages.
> > >>>>>>> 58 "-Xmx4g -Xms4g -XX:+UseNUMA" // 6. Set
> > >> UseNUMA.
> > >>>>>>> - It would be better to use smaller heap for test stability(even
> > >>>>>>> though this will not be tested regularly) unless you have any good
> > >>>> reason for it.
> > >>>>>>>
> > >>>>>>>> I have also filed a CSR at
> > >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-
> > >>>>>>> 8214081
> > >>>>>>> I reviewed this CSR as well.
> > >>>>>>>
> > >>>>>>> Thanks,
> > >>>>>>> Sangheon
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> Thank you,
> > >>>>>>>> Kishor
> > >>>>>>>>
> > >>>>>>>>> -----Original Message-----
> > >>>>>>>>> From: sangheon.kim at oracle.com
> > >>>> [mailto:sangheon.kim at oracle.com]
> > >>>>>>>>> Sent: Sunday, November 18, 2018 11:14 AM
> > >>>>>>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Stefan
> > Johansson
> > >>>>>>>>> <stefan.johansson at oracle.com>; 'hotspot-gc-
> > >> dev at openjdk.java.net'
> > >>>>>>>>> <hotspot-gc-dev at openjdk.java.net>; 'Hotspot dev runtime'
> > >>>>>>>>> <hotspot- runtime-dev at openjdk.java.net>
> > >>>>>>>>> Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
> > >>>>>>>>> Subject: Re: RFR(M): 8211425: Allocation of old generation of
> > >>>>>>>>> java heap
> > >>>>>>> on
> > >>>>>>>>> alternate memory devices - G1 GC
> > >>>>>>>>>
> > >>>>>>>>> Hi Kishor,
> > >>>>>>>>>
> > >>>>>>>>> Could you give us incremental webrev? It will help a lot for
> > >> reviewers.
> > >>>>>>>>>
> > >>>>>>>>> Thanks,
> > >>>>>>>>> Sangheon
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> On 11/16/18 6:35 PM, Kharbas, Kishor wrote:
> > >>>>>>>>>> Hi,
> > >>>>>>>>>>
> > >>>>>>>>>> I have an update to the webrev at :
> > >>>>>>>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.01
> > >>>>>>>>>> 1. Most of the comments from Sangheon and Stefan have
> been
> > >>>>>>> addressed.
> > >>>>>>>>> For ease of reading, I am copy-pasting below the comments not
> > >>>>>>>>> fixed
> > >>>>> or
> > >>>>>>> for
> > >>>>>>>>> which I have follow up.
> > >>>>>>>>>> 2. I also ran jtreg tests, with and without flag using test
> > >>>>>>>>>> group shown
> > >>>>>>> below
> > >>>>>>>>> (removed tests incompatible with this flag. Also, for testing, I
> > >>>>>>>>> made - XX:+AllocateOldGenAt a no-op instead of error since
> many
> > >>>>>>>>> tests have
> > >>>>> sub-
> > >>>>>>>>> tests for all different GCs)
> > >>>>>>>>>> I see same number of failures (12) for original, patch
> > >>>>>>>>>> without flag and with flag (no new failures) 3. Tasks in
> progress
> > >> are :
> > >>>>>>>>>> - AppCDS tests.
> > >>>>>>>>>> - CSR filing.
> > >>>>>>>>>>
> > >>>>>>>>>> tier1_old_nvdimm = \
> > >>>>>>>>>> serviceability/ \
> > >>>>>>>>>> :tier1_gc \
> > >>>>>>>>>> :tier1_runtime \
> > >>>>>>>>>> -gc/serial \
> > >>>>>>>>>> -gc/parallel \
> > >>>>>>>>>> -gc/cms \
> > >>>>>>>>>> -gc/epsilon \
> > >>>>>>>>>> -gc/TestAllocateHeapAt.java \
> > >>>>>>>>>> -gc/TestAllocateHeapAtError.java \
> > >>>>>>>>>> -gc/TestAllocateHeapAtMultiple.java \
> > >>>>>>>>>> -gc/g1/TestFromCardCacheIndex.java \
> > >>>>>>>>>> -gc/metaspace/TestMetaspacePerfCounters.java \
> > >>>>>>>>>> -gc/metaspace/TestPerfCountersAndMemoryPools.java \
> > >>>>>>>>>> -runtime/Metaspace/PrintMetaspaceDcmd.java
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>> ========================================================
> > >>>>>>>>>> Comments from Sangheon:
> > >>>>>>>>>>
> > >>>> ========================================================
> > >>>>>>>>>> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
> > >>>>>>>>>>
> > >>>>>>>>>> 173 void
> > >>>>
> G1RegionToHeteroSpaceMapper::map_nvdimm_space(ReservedSpace
> > >>>>>>>>>> rs) {
> > >>>>>>>>>> - Your previous change of adding AllocateHeapAt created nv
> file
> > >>>>>>>>>> inside
> > >>>>> of
> > >>>>>>>>> ReservedHeapSpace but this-AllocateOldGenAt create inside of
> > >>>>> mapper.
> > >>>>>>>>> Couldn't old gen also create its file near there? I feel like
> > >>>>>>>>> creating here
> > >>>>>>> seems
> > >>>>>>>>> un-natural.
> > >>>>>>>>>> - This is just an idea. Instead of adding AllocateOldGenAt,
> > >>>>>>>>>> couldn't re-
> > >>>>> use
> > >>>>>>>>> AllocateHeapAt and add type option? e.g. 'all' or 'old' or 'off'.
> > >>>>>>>>>>>> If I do this in ReservedHeapSpace, all of the reserved
> memory
> > >>>>>>>>>>>> will
> > >>>>> be
> > >>>>>>>>> mapped to nv-dimm. Since here I want part of reserved
> memory
> > in
> > >>>>>>>>> nv-
> > >>>>>>> dimm,
> > >>>>>>>>> I can't use this directly, or easily modify it for our need.
> > >>>>>>>>>>>> I like the idea of re-using AllocateHeapAt. I however not
> sure
> > >>>>>>>>>>>> how can a '-XX option handle extra option.( I am thinking of
> > >>>>>>>>>>>> something similar to -Xbootclasspath/a(p))
> > >>>> ========================================================
> > >>>>>>>>>>
> > src/hotspot/share/gc/g1/heapRegionManagerForHeteroHeap.cpp
> > >>>>>>>>>> 34 // expand_by() is called to grow the heap. We grow into
> > >>>>>>>>>> nvdimm
> > >>>>>>> now.
> > >>>>>>>>>> 35 // Dram regions are committed later as needed during
> > mutator
> > >>>>> region
> > >>>>>>>>>> allocation or
> > >>>>>>>>>> 36 // when young list target length is determined after gc cycle.
> > >>>>>>>>>> 37 uint HeapRegionManagerForHeteroHeap::expand_by(uint
> > >>>>>>> num_regions,
> > >>>>>>>>>> WorkGang* pretouch_workers) {
> > >>>>>>>>>> 38 uint num_expanded =
> expand_nvdimm(MIN2(num_regions,
> > >>>>>>>>>> max_expandable_length() - total_regions_committed()),
> > >>>>>>>>>> pretouch_workers);
> > >>>>>>>>>> 39 assert(total_regions_committed() <=
> > >>>>>>>>>> max_expandable_length(), "must be");
> > >>>>>>>>>> 40 return num_expanded;
> > >>>>>>>>>> 41 }
> > >>>>>>>>>> - I guess the only reason of not adjusting dram here is that we
> > >>>>>>>>>> don't have information of 'is_old'? Looks a bit weired but I
> > >>>>>>>>>> don't have any suggestion. (see more below at
> > >>>>>>>>>> allocate_free_region)
> > >>>>>>>>>>
> > >>>>>>>>>>>> Yes, expand_by() is used to grow the heap at initialization or
> > >>>>>>>>>>>> after
> > >>>>> full
> > >>>>>>>>> GC. Since young generation target length is decided at a later
> > >>>>>>>>> point, I
> > >>>>>>> expand
> > >>>>>>>>> in nv-dimm space here.
> > >>>>>>>>>>>> Later when young generation target length is determined,
> > >>>>>>>>> adjust_dram_regions() is called to provision the target number
> of
> > >>>>> regions.
> > >>>> ========================================================
> > >>>>>>>>>> Comments from Stefan
> > >>>>>>>>>>
> > >>>> ========================================================
> > >>>>>>>>>> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> > >>>>>>>>>> -------------------------------------------
> > >>>>>>>>>> 1642 if (AllocateOldGenAt != NULL) {
> > >>>>>>>>>> 1643 _is_hetero_heap = true;
> > >>>>>>>>>> 1644 max_byte_size *= 2;
> > >>>>>>>>>> 1645 }
> > >>>>>>>>>>
> > >>>>>>>>>> I would like this to be abstracted out of G1CollectedHeap, not
> > >>>>>>>>>> entirely sure how but I'm thinking something like adding a
> > >>>>>>>>>> G1HeteroCollectorPolicy which answers correctly on how
> much
> > >>>>> memory
> > >>>>>>>>>> needs to be reserved and how big the heap actually is.
> > >>>>>>>>>>
> > >>>>>>>>>>>> I abstracted out this in G1HeteroCollectorPolicy.
> > >>>>>>>>>> 1668
> G1RegionToSpaceMapper::create_heap_mapper(g1_rs,
> > >>>>>>>>>> 1669 g1_rs.size(),
> > >>>>>>>>>> 1670 page_size,
> > >>>>>>>>>> 1671 HeapRegion::GrainBytes,
> > >>>>>>>>>> 1672 1,
> > >>>>>>>>>> 1673 mtJavaHeap);
> > >>>>>>>>>>
> > >>>>>>>>>> This function could then also make use of the new policy,
> > >>>>>>>>>> something
> > >>>>> like:
> > >>>>>>>>>> create_heap_mapper(g1_rs, _collector_policy, page_size);
> > >>>>>>>>>>
> > >>>>>>>>>>>> Since the mapper needs to map the reserved memory, Since
> > >>>>>>>>> create_heap_mapper() does not need anything from
> > >>>>>>>>> collector_policy, I chose to keep the call unchanged.
> > >>>>>>>>>> 3925 if(g1h->is_hetero_heap()) {
> > >>>>>>>>>> 3926 if(!r->is_old()) {
> > >>>>>>>>>> ....
> > >>>>>>>>>> 3929 r->set_premature_old();
> > >>>>>>>>>> 3930 }
> > >>>>>>>>>> 3931 } else {
> > >>>>>>>>>> 3932 r->set_old();
> > >>>>>>>>>> 3933 }
> > >>>>>>>>>>
> > >>>>>>>>>> So if I understand this correctly, premature_old is used when
> we
> > >>>>>>>>>> get evac failures and we use it to force these regions to be
> > >>>>>>>>>> included in the next Mixed collection. I'm not sure this is
> > >>>>>>>>>> needed, in fact I think one of the cool things with nv-dimm is
> > >>>>>>>>>> that we can avoid evac failures. G1 normally needs to stop
> > >>>>>>>>>> handing out regions to promote to when there are no regions
> > >>>>>>>>>> left, but when using nv-dimm the old
> > >>>>> regions
> > >>>>>>>>>> come from another pool and we should be able to avoid this
> > case.
> > >>>>>>>>>>
> > >>>>>>>>>>>> Yes! I had given this a thought. We can always procure a
> free
> > >>>>>>>>>>>> region
> > >>>>> in
> > >>>>>>>>> nv-dimm. However if we do this, during this GC phase there
> could
> > >>>>>>>>> be
> > >>>>> more
> > >>>>>>>>> regions committed in memory than Xmx.
> > >>>>>>>>>>>> This would mean heap memory usage increases to more
> than
> > >>>> 'Xmx'
> > >>>>>>>>> during some gc phases. Will this be ok?
> > >>>>>>>>>>>> But again in current implementation memory usage is more
> > >> than
> > >>>>> Xmx
> > >>>>>>>>>>>> anyway, since I allocate Xmx memory in nv-dimm at start. I
> do
> > >>>>>>>>>>>> this
> > >>>>>>>>> because allocating in nv-dimm involves mapping to a file
> system,
> > >>>>>>>>> which becomes complicated if you expand/shrink file whenever
> > you
> > >>>>>>>>> commit/uncommit regions. But I chose to keep this only in
> > >>>>>>>>> G1regionToSpaceMapper and not design rest of the code based
> > on
> > >>>> this.
> > >>>>>>>>>>>> I had to make an exception for Full GC where I increase the
> > >>>>> committed
> > >>>>>>>>> regions in nv-dimm before start of GC, since we need to move
> all
> > >>>>> objects
> > >>>>>>> to
> > >>>>>>>>> old generation.
> > >>>>>>>>>>>> Let me know what you think, I like your idea since we don't
> > >>>>>>>>>>>> need to
> > >>>>>>> add
> > >>>>>>>>> more complexity for handling premature old regions.
> > >>>>>>>>>> Thanks and regards,
> > >>>>>>>>>> Kishor
> > >>>>>>>>>>
> > >>>>>>>>>>> -----Original Message-----
> > >>>>>>>>>>> From: sangheon.kim at oracle.com
> > >>>>> [mailto:sangheon.kim at oracle.com]
> > >>>>>>>>>>> Sent: Monday, November 5, 2018 9:39 PM
> > >>>>>>>>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; Stefan
> > >>>>>>>>>>> Johansson <stefan.johansson at oracle.com>; 'hotspot-gc-
> > >>>>> dev at openjdk.java.net'
> > >>>>>>>>>>> <hotspot-gc-dev at openjdk.java.net>; 'Hotspot dev runtime'
> > >>>>> <hotspot-
> > >>>>>>>>>>> runtime-dev at openjdk.java.net>
> > >>>>>>>>>>> Cc: Viswanathan, Sandhya
> <sandhya.viswanathan at intel.com>
> > >>>>>>>>>>> Subject: Re: RFR(M): 8211425: Allocation of old generation of
> > >>>>>>>>>>> java heap on alternate memory devices - G1 GC
> > >>>>>>>>>>>
> > >>>>>>>>>>> Hi Kishor,
> > >>>>>>>>>>>
> > >>>>>>>>>>> On 11/2/18 2:48 PM, Kharbas, Kishor wrote:
> > >>>>>>>>>>>> Thanks Stefan and Sangheon for your comments.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> As suggested, I ran jtreg for serviceability tests. I see
> > >>>>>>>>>>>> failures even without
> > >>>>>>>>>>> using the new flag.
> > >>>>>>>>>>>> Looks like things are breaking in mirror classes
> > >>>>>>>>>>> (jdk.hotspot.agent/sun.jvm.hotspot.gc.g1.*) because of new
> > >>>>>>>>>>> fields and/or method changes in JVM classes.
> > >>>>>>>>>>>> Is there a bkm for changing these mirror Java classes when
> we
> > >>>>> change
> > >>>>>>>>>>>> JVM
> > >>>>>>>>>>> classes?
> > >>>>>>>>>>> Sorry I'm not aware of any better known method than fixing
> > >>>>>>>>>>> one-by-
> > >>>>>>> one.
> > >>>>>>>>>>>> I am working on addressing your comments on the patch
> (had
> > >>>>>>>>>>>> been
> > >>>>>>>>>>> dragged into another project, so I am behind on this).
> > >>>>>>>>>>> Thanks for your update.
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks,
> > >>>>>>>>>>> Sangheon
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>>> Thanks
> > >>>>>>>>>>>> Kishor
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>> -----Original Message-----
> > >>>>>>>>>>>>> From: Stefan Johansson
> > >> [mailto:stefan.johansson at oracle.com]
> > >>>>>>>>>>>>> Sent: Friday, October 26, 2018 7:32 AM
> > >>>>>>>>>>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>; 'hotspot-
> > gc-
> > >>>>>>>>>>>>> dev at openjdk.java.net' <hotspot-gc-
> dev at openjdk.java.net>;
> > >>>>>>> 'Hotspot
> > >>>>>>>>>>> dev
> > >>>>>>>>>>>>> runtime' <hotspot-runtime-dev at openjdk.java.net>
> > >>>>>>>>>>>>> Cc: Viswanathan, Sandhya
> > <sandhya.viswanathan at intel.com>
> > >>>>>>>>>>>>> Subject: Re: RFR(M): 8211425: Allocation of old generation
> of
> > >>>>>>>>>>>>> java heap on alternate memory devices - G1 GC
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Hi Kishor,
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I'll start with a few general observations and then there are
> > >>>>>>>>>>>>> some specific code comments below.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I think the general design looks promising, not having to set
> > >>>>>>>>>>>>> a fixed limit for what can end up on NV-dimm is great.
> > Having
> > >>>>>>>>>>>>> a separate HeapRegionManager looks like a good
> abstraction
> > >>>>>>>>>>>>> and
> > >>>>>>> when
> > >>>>>>>>>>>>> adding larger features like this that is really important.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I also agree with Sangheon that you should do more
> testing,
> > >>>>>>>>>>>>> both with and without the featured turned on. I also
> > >>>>>>>>>>>>> recommend you to build with pre- compiled headers
> > disabled,
> > >>>>>>>>>>>>> to find places where includes have been forgotten. To
> > >>>>>>>>>>>>> configure such build add --disable-precompiled-headers to
> > >> your
> > >>>> configure call.
> > >>>>>>>>>>>>> On 2018-10-04 04:08, Kharbas, Kishor wrote:
> > >>>>>>>>>>>>>> Hi all,
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Requesting review of the patch for allocating old
> generation
> > >>>>>>>>>>>>>> of
> > >>>>> g1
> > >>>>>>>>>>>>>> gc on alternate memory devices such nv-dimm.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> The design of this implementation is explained on the bug
> > >>>>>>>>>>>>>> page -
> > >>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8211425
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Please follow the parent issue here :
> > >>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8202286.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Webrev:
> > >>>>> http://cr.openjdk.java.net/~kkharbas/8211425/webrev.00
> > >>>>>>>>>>>>>>
> > >> <http://cr.openjdk.java.net/%7Ekkharbas/8211425/webrev.00>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/g1Allocator.inline.hpp
> > >>>>>>>>>>>>> ----------------------------------------------
> > >>>>>>>>>>>>> 100 size_t length = static_cast
> > >>>>>>>>>>>>> <G1CollectedHeap*>(Universe::heap())-
> > >>>>> max_reserved_capacity()
> > >>>>>>>>>>>>> ;
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> You can avoid the cast by using G1CollectedHeap::heap()
> > >>>>>>>>>>>>> instead
> > >>>>> of
> > >>>>>>>>>>>>> Universe::heap().
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/g1CollectedHeap.cpp
> > >>>>>>>>>>>>> -------------------------------------------
> > >>>>>>>>>>>>> 1642 if (AllocateOldGenAt != NULL) {
> > >>>>>>>>>>>>> 1643 _is_hetero_heap = true;
> > >>>>>>>>>>>>> 1644 max_byte_size *= 2;
> > >>>>>>>>>>>>> 1645 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I would like this to be abstracted out of G1CollectedHeap,
> > >>>>>>>>>>>>> not entirely sure how but I'm thinking something like
> adding
> > >>>>>>>>>>>>> a G1HeteroCollectorPolicy which answers correctly on how
> > >>>> much
> > >>>>>>>>> memory
> > >>>>>>>>>>>>> needs to be reserved and how big the heap actually is.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> 1668
> > >> G1RegionToSpaceMapper::create_heap_mapper(g1_rs,
> > >>>>>>>>>>>>> 1669 g1_rs.size(),
> > >>>>>>>>>>>>> 1670 page_size,
> > >>>>>>>>>>>>> 1671 HeapRegion::GrainBytes,
> > >>>>>>>>>>>>> 1672 1,
> > >>>>>>>>>>>>> 1673 mtJavaHeap);
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> This function could then also make use of the new policy,
> > >>>>> something
> > >>>>>>>>> like:
> > >>>>>>>>>>>>> create_heap_mapper(g1_rs, _collector_policy, page_size);
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> 1704 if (is_hetero_heap()) {
> > >>>>>>>>>>>>> 1705 _hrm = new
> > >>>>>>>>>>>>> HeapRegionManagerForHeteroHeap((uint)((max_byte_size
> > >>>>>>>>>>>>> / 2) / HeapRegion::GrainBytes /*heap size as num of
> > >> regions*/));
> > >>>>>>>>>>>>> 1706 }
> > >>>>>>>>>>>>> 1707 else {
> > >>>>>>>>>>>>> 1708 _hrm = new HeapRegionManager();
> > >>>>>>>>>>>>> 1709 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> This code could then become something like:
> > >>>>>>>>>>>>> _hrm =
> > >>>> HeapRegionManager::create_manager(_collector_policy)
> > >>>>>>>>>>>>> 3925 if(g1h->is_hetero_heap()) {
> > >>>>>>>>>>>>> 3926 if(!r->is_old()) {
> > >>>>>>>>>>>>> ....
> > >>>>>>>>>>>>> 3929 r->set_premature_old();
> > >>>>>>>>>>>>> 3930 }
> > >>>>>>>>>>>>> 3931 } else {
> > >>>>>>>>>>>>> 3932 r->set_old();
> > >>>>>>>>>>>>> 3933 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> So if I understand this correctly, premature_old is used
> when
> > >>>>>>>>>>>>> we get evac failures and we use it to force these regions to
> > >>>>>>>>>>>>> be included in the next Mixed collection. I'm not sure this
> > >>>>>>>>>>>>> is needed, in fact I think one of the cool things with
> > >>>>>>>>>>>>> nv-dimm is that we can avoid evac failures. G1 normally
> > needs
> > >>>>>>>>>>>>> to stop handing out regions to promote to when there are
> > no
> > >>>>>>>>>>>>> regions left, but when using nv-dimm the old regions come
> > >>>>>>>>>>>>> from another pool and we should
> > >>>>> be
> > >>>>>>>>> able to avoid this case.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/g1FullCollector.cpp
> > >>>>>>>>>>>>> -------------------------------------------
> > >>>>>>>>>>>>> 169 if (_heap->is_hetero_heap()) {
> > >>>>>>>>>>>>> 170 static_cast
> > >>>>>>>>>>>>> <HeapRegionManagerForHeteroHeap*>(_heap->_hrm)-
> > >>>>>>>>>>>>>> prepare_for_full_collection_start();
> > >>>>>>>>>>>>> 171 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Move this to:
> > >>>>>>>>>>>>> G1CollectedHeap::prepare_heap_for_full_collection()
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> 185 if (_heap->is_hetero_heap()) {
> > >>>>>>>>>>>>> 186 static_cast
> > >>>>>>>>>>>>> <HeapRegionManagerForHeteroHeap*>(_heap->_hrm)-
> > >>>>>>>>>>>>>> prepare_for_full_collection_end();
> > >>>>>>>>>>>>> 187 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Move this to:
> > >>>>>>>>>>>>> G1CollectedHeap::prepare_heap_for_mutators()
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> And if you make the prepare-functions virtual and not
> doing
> > >>>>>>>>>>>>> anything on HeapRegionManager we can avoid the checks.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/g1Policy.cpp
> > >>>>>>>>>>>>> ------------------------------------
> > >>>>>>>>>>>>> 223 if(_g1h->is_hetero_heap() && (Thread::current()-
> > >>>>>>>>>> is_VM_thread()
> > >>>>>>>>>>>>> || Heap_lock->owned_by_self())) {
> > >>>>>>>>>>>>> 224 static_cast
> > >>>>>>>>>>>>> <HeapRegionManagerForHeteroHeap*>(_g1h->hrm())-
> > >>>>>>>>>>>>>> resize_dram_regions(_young_list_target_length,
> > >>>>>>>>>>>>> _g1h->workers());
> > >>>>>>>>>>>>> 225 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Feels like this code should be part of the
> > >>>>>>>>>>>>> prepare_for_full_collection_end() above, but the
> > >>>>>>>>>>>>> _young_list_target_length isn't updated until right after
> > >>>>>>>>>>>>> prepare_heap_for_mutators() so you might need to
> > >> restructure
> > >>>>> the
> > >>>>>>>>>>>>> code a bit more to make it fit.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/g1RegionToSpaceMapper.cpp
> > >>>>>>>>>>>>> -------------------------------------------------
> > >>>>>>>>>>>>> Had to add these two includes to make it compile.
> > >>>>>>>>>>>>> #include "runtime/java.hpp"
> > >>>>>>>>>>>>> #include "utilities/formatBuffer.hpp"
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Please also clean out all code commented out.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/heapRegionManager.hpp
> > >>>>>>>>>>>>> ---------------------------------------------
> > >>>>>>>>>>>>> I agree with Sangheon, please group the members that
> > should
> > >>>>>>>>>>>>> be protected and remember to update the init-list for the
> > >>>>> constructor.
> > >>>>>>>>>>>>> Also agree that the #else on #ifdef ASSERT should be
> > removed.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/gc/g1/heapRegionSet.cpp
> > >>>>>>>>>>>>> -----------------------------------------
> > >>>>>>>>>>>>> 240 bool started = false;
> > >>>>>>>>>>>>> 241 while (cur != NULL && cur->hrm_index() <= end) {
> > >>>>>>>>>>>>> 242 if (!started && cur->hrm_index() >= start) {
> > >>>>>>>>>>>>> 243 started = true;
> > >>>>>>>>>>>>> 244 }
> > >>>>>>>>>>>>> 245 if(started) {
> > >>>>>>>>>>>>> 246 num++;
> > >>>>>>>>>>>>> 247 }
> > >>>>>>>>>>>>> 248 cur = cur->next();
> > >>>>>>>>>>>>> 249 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> To me this looks more complex than it is because of the
> > >>>>>>>>>>>>> multiple conditions, what do you think about:
> > >>>>>>>>>>>>> while (cur != NULL) {
> > >>>>>>>>>>>>> uint index = cur->hrm_index();
> > >>>>>>>>>>>>> if (index > end) {
> > >>>>>>>>>>>>> break;
> > >>>>>>>>>>>>> } else if (index >= start) {
> > >>>>>>>>>>>>> num++;
> > >>>>>>>>>>>>> }
> > >>>>>>>>>>>>> cur = cur->next();
> > >>>>>>>>>>>>> }
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/runtime/arguments.cpp
> > >>>>>>>>>>>>> ---------------------------------------
> > >>>>>>>>>>>>> 2072 if(!FLAG_IS_DEFAULT(AllocateHeapAt) &&
> > >>>>>>>>>>>>> !FLAG_IS_DEFAULT(AllocateOldGenAt)) {
> > >>>>>>>>>>>>> 2073 jio_fprintf(defaultStream::error_stream(),
> > >>>>>>>>>>>>> 2074 "AllocateHeapAt and AllocateOldGenAt
> cannot
> > >> be
> > >>>>> used
> > >>>>>>>>>>>>> together.\n");
> > >>>>>>>>>>>>> 2075 status = false;
> > >>>>>>>>>>>>> 2076 }
> > >>>>>>>>>>>>> 2077
> > >>>>>>>>>>>>> 2078 if (!FLAG_IS_DEFAULT(AllocateOldGenAt) &&
> > >> (UseSerialGC
> > >>>>> ||
> > >>>>>>>>>>>>> UseConcMarkSweepGC || UseEpsilonGC || UseZGC)) {
> > >>>>>>>>>>>>> 2079 jio_fprintf(defaultStream::error_stream(),
> > >>>>>>>>>>>>> 2080 "AllocateOldGenAt not supported for selected
> > >> GC.\n");
> > >>>>>>>>>>>>> 2081 status = false;
> > >>>>>>>>>>>>> 2082 }
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> This code would fit much better in GCArguments. There is
> > >>>>>>>>>>>>> currently no method handling this case but please add
> > >>>>>>> check_args_consistency().
> > >>>>>>>>>>>>> You can look at CompilerConfig::check_args_consistency
> for
> > >>>>>>>>> inspiration.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> src/hotspot/share/runtime/globals.hpp
> > >>>>>>>>>>>>> -------------------------------------
> > >>>>>>>>>>>>> 2612 experimental(uintx, G1YoungExpansionBufferPerc,
> 10,
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Move to g1_globals.hpp and call it
> > >>>>> G1YoungExpansionBufferPercent.
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>> src/hotspot/share/gc/g1/heapRegionManagerForHeteroHeap.*pp
> > >>>>>>>>>>>>> ----------------------------------------------------------
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Haven't reviewed this code in detail yet but will do in a
> > >>>>>>>>>>>>> later
> > >>>>> review.
> > >>>>>>>>>>>>> One thought I have already is about the name, what do
> you
> > >>>>>>>>>>>>> think
> > >>>>>>>>> about:
> > >>>>>>>>>>>>> HeterogeneousHeapRegionManager
> > >>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>>> Stefan
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Testing : Passed tier1_gc and tier1_runtime jtret tests.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> I added extra options
> > >>>>>>>>>>>>>> "-XX:+UnlockExperimentalVMOptions -
> > >>>>>>>>>>>>> XX:AllocateOldGenAt=/home/Kishor"
> > >>>>>>>>>>>>>> to each test. There are some tests which I had to exclude
> > >>>>>>>>>>>>>> since they won't work with this flag. Example: tests in
> > >>>>>>> ConcMarkSweepGC
> > >>>>>>>>>>>>>> which does not support this flag, tests requiring
> > >>>>>>>>>>>>>> CompressedOops to be enabled,
> > >>>>>>>>>>>>> etc.
> > >>>>>>>>>>>>>> Thanks
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Kishor
> > >>>>>>>>>>>>>>
> > >
More information about the hotspot-gc-dev
mailing list