RFR(M): 8211424: Allocation of old generation of java heap on alternate memory devices - ParallelOld
Stefan Johansson
stefan.johansson at oracle.com
Fri Dec 14 10:30:57 UTC 2018
Hi Kishor,
Thanks for addressing my comments, but I have one more :)
On 2018-12-14 08:59, Kharbas, Kishor wrote:
> New webrev with small incremental change to make young sizing logs similar to G1.
>
> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.11
src/hotspot/share/gc/parallel/heterogeneousGenerationSizer.cpp
--------------------------------------------------------------
37 char calc_str[100];
38 calc_str[0] = 0;
Instead of using a raw char buffer we have the FormatBuffer class that
can be used. If you look in
G1CollectedHeap::do_collection_pause_at_safepoint there is an example on
how you can use it. I think such change would both help readability and
robustness.
Thanks,
Stefan
> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.10_to_11
>
> Thanks
> Kishor
>
>> -----Original Message-----
>> From: Kharbas, Kishor
>> Sent: Thursday, December 13, 2018 1:01 PM
>> To: 'sangheon.kim at oracle.com' <sangheon.kim at oracle.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>; Kharbas,
>> Kishor <kishor.kharbas at intel.com>
>> Subject: RE: RFR(M): 8211424: Allocation of old generation of java heap on
>> alternate memory devices - ParallelOld
>>
>> Hi Sagheon,
>>
>> I have a new webrev at -
>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.10/
>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.09_to_10/
>>
>> Comments inline.
>>
>>> -----Original Message-----
>>> From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
>>> Sent: Thursday, December 13, 2018 11:04 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): 8211424: Allocation of old generation of java
>>> heap on alternate memory devices - ParallelOld
>>>
>>> Hi Kishor,
>>>
>>> On 12/12/18 7:59 PM, Kharbas, Kishor wrote:
>>>> Hi Sangheon, Stefan,
>>>>
>>>> Here is an updated webrev -
>>>>
>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.07_to_08
>>>> / http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.08/
>>>
>>> ----------------------------------------------------------
>>> src/hotspot/share/gc/parallel/heterogeneousGenerationSizer.cpp
>>> 36 julong young_size_limit = os::physical_memory();
>>> - This variable is not used.
>> [Kharbas, Kishor] Fixed.
>>>
>>> 58 log_info(gc, ergo)("Setting MaxNewSize to " SIZE_FORMAT "
>>> which is 80% of dram memory. Dram usage can be lowered by setting
>>> MaxNewSize to a lower value", (size_t)reasonable_max);
>>> - Later I saw you fixed this compile error on webrev.09, good.
>>>
>> [Kharbas, Kishor] Fixed. And I changed log message according to Stefan's
>> comment.
>>
>>> Again, hope to have more manual testing with young gen size related
>>> options added.
>>>
>> [Kharbas, Kishor] I tested different cases with different combinations of
>> values for MaxNewSize, NewSize, MaxRAM, MaxRAMPercentage/Fraction,
>> etc.
>>
>> Thank you
>> Kishor
>>
>>> Thanks,
>>> Sangheon
>>>
>>>>
>>>> Thanks
>>>> Kishor
>>>>
>>>>> -----Original Message-----
>>>>> From: sangheon.kim at oracle.com [mailto:sangheon.kim at oracle.com]
>>>>> Sent: Wednesday, December 12, 2018 4:34 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): 8211424: Allocation of old generation of java
>>>>> heap on alternate memory devices - ParallelOld
>>>>>
>>>>> One more!
>>>>>
>>>>> ----------------------------------------------------------
>>>>> src/hotspot/share/gc/parallel/heterogeneousGenerationSizer.cpp
>>>>> 56 log_warning(gc, ergo)("Setting MaxNewSize to " SIZE_FORMAT "
>>>>> based on dram memory available", reasonable_max);
>>>>> 62 log_warning(gc, ergo)("Setting NewSize to " SIZE_FORMAT "
>>>>> based on dram memory available", reasonable_max);
>>>>> - You need to type-cast 'reasonable_max' to 'size_t' for both lines.
>>>>> Fails compiling on Mac.
>>>>>
>>>>> Thanks,
>>>>> Sangheon
>>>>>
>>>>>
>>>>> On 12/12/18 3:12 PM, sangheon.kim at oracle.com wrote:
>>>>>> Hi Kishor,
>>>>>>
>>>>>> On 12/12/18 12:51 PM, Kharbas, Kishor wrote:
>>>>>>> New webrev and comments inline.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.07/
>>>>>>>
>>>>>
>>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.06_to_07/
>>>>>> I have some comments.
>>>>>>
>>>>>> ----------------------------------------------------------
>>>>>> src/hotspot/share/gc/parallel/generationSizer.hpp
>>>>>> 46 bool is_hetero_heap() const;
>>>>>> - 'virtual'?
>>>>>>
>>>>>> ----------------------------------------------------------
>>>>>> src/hotspot/share/gc/parallel/parallelArguments.cpp
>>>>>> 100 return create_heap_with_policy<ParallelScavengeHeap,
>>>>>> GenerationSizer>();
>>>>>> - Missing 2 spaces at line 100.
>>>>>>
>>>>>> ----------------------------------------------------------
>>>>>> src/hotspot/share/gc/parallel/heterogeneousGenerationSizer.cpp
>>>>>> 2 * Copyright (c) 2018, Oracle and/or its affiliates. All
>>>>>> rights reserved.
>>>>>> ...
>>>>>> 23 */
>>>>>> - Missing 1 space for line 2 ~ 23.
>>>>>>
>>>>>> 39 julong phys_mem = FLAG_IS_DEFAULT(MaxRAM) ?
>>>>>> os::physical_memory() : (julong)MaxRAM;
>>>>>> - I saw your reasoning, but why would you like to allow MaxRAM via
>>>>>> command-line? Accidentally it can be larger than physical RAM.
>>>>>>
>>>>>> ----------------------------------------------------------
>>>>>> src/hotspot/share/gc/parallel/heterogeneousGenerationSizer.hpp
>>>>>> - Missing 1 space for line 2 ~ 23.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Sangheon
>>>>>>
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Stefan Johansson [mailto:stefan.johansson at oracle.com]
>>>>>>>> Sent: Wednesday, December 12, 2018 1:38 AM
>>>>>>>> To: Kharbas, Kishor <kishor.kharbas at intel.com>;
>>>>>>>> sangheon.kim 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): 8211424: Allocation of old generation of
>>>>>>>> java heap on alternate memory devices - ParallelOld
>>>>>>>>
>>>>>>>> Hi Kishor,
>>>>>>>>
>>>>>>>> On 2018-12-12 09:16, Kharbas, Kishor wrote:
>>>>>>>>> Hi Sangheon, Stefan,
>>>>>>>>>
>>>>>>>>> Thank you for your reviews.
>>>>>>>>> I have an updated webrev (against g1 patch) at :
>>>>>>>>>
>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.06/
>>>>>>>> src/hotspot/share/gc/parallel/generationSizer.cpp
>>>>>>>> -------------------------------------------------
>>>>>>>> Instead of adding set_ergo_flags_for_hetero_heap() to the
>>>>>>>> generic GenerationSizer I think instead having a
>>>>>>>> HeterogeneousGenerationSizer which does this at the correct
>>>>>>>> point during initialization is a better approach.
>>>>>>>>
>>>>>>>> Also I don't think you should use the whole physical ram as
>>>>>>>> limit, the JVM will use memory and you probably don't want to
>>>>>>>> try to expand the young gen all the way to the max. The normal
>>>>>>>> heap sizing code used when nothing is set uses MaxRamPercentage
>>>>>>>> to limit
>>> the heap.
>>>>>>>> There is also the flag MaxRam which can be used to limit the
>>>>>>>> amount of DRAM used by the heap so I think limiting MaxNewSize
>>>>>>>> to MIX(MaxRam, physical mem) * MaxRamPercentage /
>>>>>>>> 100 would be a good idea. Something similar to what is done in
>>>>>>>> Arguments::set_heap_size().
>>>>>>>>
>>>>>>> [Kharbas, Kishor] I added HeterogeneousGenerationSizer which
>>>>>>> would limit NewSize and MaxNewSize. My reasoning is as follows,
>>>>>>> - If MaxRAM is set on command line, use that as available
>>>>>>> dram memory
>>>>>>> - If MaxRAMPercentage or MaxRamFraction is set on cmd line,
>>>>>>> we will limit young size accordingly.
>>>>>>> - Otherwise, I use my own ' MaxRamFractionForYoung' to limit
>>>>>>> young size. Reason is - since MaxRAMPercentage is very low (20%)
>>>>>>> since its meant to size heap when MaxHeap is not specified.
>>>>>>>
>>>>>>>> src/hotspot/share/runtime/arguments.cpp
>>>>>>>> ---------------------------------------
>>>>>>>> 2960 // When AllocateOldGenAt is set, we cannot use largepages
>>>>>>>> for entire heap memory.
>>>>>>>> 2961 // Only young gen which is allocated in dram can use
>>>>>>>> large pages, but we currently don't support that.
>>>>>>>> 2962 if (!FLAG_IS_DEFAULT(AllocateOldGenAt)) {
>>>>>>>> 2963 FLAG_SET_DEFAULT(UseLargePages, false);
>>>>>>>> 2964 }
>>>>>>>>
>>>>>>>> From what I can see you have not addressed this or explained
>>>>>>>> why this have to be done here instead of in GCArguments.
>>>>>>>>
>>>>>>> [Kharbas, Kishor] I moved this to GCArguments. But since this is
>>>>>>> also part of G1 patch, so it's not seen here.
>>>>>>>
>>>>>>> Thank you
>>>>>>> Kishor
>>>>>
>> http://cr.openjdk.java.net/~kkharbas/8211424/8211424.webrev.05_to_0
>>>>>>>>> 6
>>>>>>>>>
>>>>>>>>> Jtreg tests (hotspot_runtime, compiler+hotspot, serviceability,
>>>>>>>> vmTestbase_vm_gc) on my system gave me following results.
>>>>>>>>> (1) OpenJDK tip : passed: 1,806; failed: 24; error: 189
>>>>>>>>> (2) Parallel patch, no flag : passed: 1,590; failed: 15; error:
>>>>>>>>> 242
>>>>>>>>> (3) Parallel patch with flag : passed 1,816; failed: 15; error:
>>>>>>>>> 188
>>>>>>>>>
>>>>>>>>> New failure in (1) -> (2)
>>>>>>>>> -
>>>>>>>> compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test
>>>>>>>> /T
>>>>>>>> es
>>>>>>>> tResol
>>>>>>>>
>>>>>>>> vedJavaType.java
>>>>>>>>> I looked at the report, but could not figure out why it
>>>>>>>>> fails. Most likely it's
>>>>>>>> my environment not being setup correctly.
>>>>>>>> I think this was an issue in OpenJDK pushed during the weekend
>>>>>>>> and fixed (test excluded) Monday afternoon.
>>>>>>>>> New failures in (1) -> (3)
>>>>>>>>> -
>>>>>>>>> compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/tes
>>>>>>>>> t/
>>>>>>>>> Te
>>>>>>>>> stR
>>>>>>>>> esolvedJavaType.java
>>>>>>>>> - compiler/aot/verification/vmflags/TrackedFlagTest.java
>>>>>>>>> Fails because we disable UseCompressedOops.
>>>>>>>>>
>>>>>>>>> @Stefan, I will do performance regression test with SPECjvm2008
>>>>>>>> benchmarks.
>>>>>>>> Good, SPECjvm2008 should not show any problems, but good to run
>>>>>>>> anyways.
>>>>>>>> I've run SPECjbb2015 without seeing any regressions so that is
>>>>>>>> also good.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Stefan
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Kishor
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: sangheon.kim at oracle.com
>>> [mailto:sangheon.kim at oracle.com]
>>>>>>>>>> Sent: Tuesday, December 11, 2018 2:17 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): 8211424: Allocation of old generation of
>>>>>>>>>> java heap on alternate memory devices - ParallelOld
>>>>>>>>>>
>>>>>>>>>> Hi Kishor,
>>>>>>>>>>
>>>>>>>>>> On 12/11/18 1:00 AM, Kharbas, Kishor wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Here is an updated webrev which fixes the comments below.
>>>>>>>>>>> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.05
>>>>>>>>>>>
>> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.04_to_05/
>>>>>>>>>> Here are some general comments:
>>>>>>>>>> - As I told in review thread of 8211425-G1 part review thread,
>>>>>>>>>> I would like to see separate patch for Parallel GC part.
>>>>>>>>>> - A way not to use more than DRAM, same as G1.
>>>>>>>>>> - Testing results with/without this feature.
>>>>>>>>>>
>>>>>>>>>> And I have some minor comments here.
>>>>>>>>>>
>>>>>>>>>> -----------------------------------
>>>>>>>>>> src/hotspot/share/gc/parallel/adjoiningGenerations.cpp
>>>>>>>>>> 287 AdjoiningGenerations*
>>>>>>>>>>
>>> AdjoiningGenerations::create_adjoining_generations(ReservedSpace
>>>>>>>>>> old_young_rs,
>>>>>>>>>> 288 GenerationSizer* policy,
>>>>>>>>>> 289 size_t alignment) {
>>>>>>>>>> - Wrong alignment at line 288/289.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----------------------------------
>>>>>>>>>>
>> src/hotspot/share/gc/parallel/adjoiningGenerationsForHeteroHeap.
>>>>>>>>>> cp
>>>>>>>>>> p
>>>>>>>>>> 47 _total_size_limit = policy->max_heap_byte_size();
>>>>>>>>>> - Please move back to initialization list unless there's any
>>>>>>>>>> good reason.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----------------------------------
>>>>>>>>>> src/hotspot/share/gc/parallel/generationSizer.cpp
>>>>>>>>>> - Copyright update
>>>>>>>>>>
>>>>>>>>>> 80 if(is_hetero_heap() && UseAdaptiveGCBoundary) {
>>>>>>>>>> 81 // This is the size that young gen can grow to,
>>>>>>>>>> when AdaptiveGCBoundary is true.
>>>>>>>>>> 83 // This is the size that old gen can grow to,
>>>>>>>>>> when AdaptiveGCBoundary is true.
>>>>>>>>>> - Line 81 and 83, 'AdaptiveGCBoundary' ->
>>> 'UseAdaptiveGCBoundary'.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----------------------------------
>>>>>>>>>> src/hotspot/share/gc/parallel/generationSizer.hpp
>>>>>>>>>> - Copyright update
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----------------------------------
>>>>>>>>>> src/hotspot/share/gc/parallel/psParallelCompact.cpp
>>>>>>>>>> 1998 // We also return when its a heterogenous heap because
>>>>>>>>>> old generation cannot absorb data from eden
>>>>>>>>>> - 'also return' -> 'also return false'.
>>>>>>>>>> - 'its' -> 'it's' or 'it is'
>>>>>>>>>>
>>>>>>>>>> 2000 if (!(UseAdaptiveSizePolicy && UseAdaptiveGCBoundary)
>>>>>>>>>> ||
>>>>>>>>>> ParallelScavengeHeap::heap()->ps_collector_policy()->is_hetero
>>>>>>>>>> _h
>>>>>>>>>> ea
>>>>>>>>>> p()
>>>>>>>>>> ) {
>>>>>>>>>> - Add new line for the new condition?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Sangheon
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> I have create separate webrev for tests since they are common
>>>>>>>>>>> this issue and 8211425 -
>>>>>>>>>>> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.test.00/
>>>>>>>>>>>
>>>>>>>>>>> I am running jtreg tests for all hotspot tests to check the
>>>>>>>>>>> stability.
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>> Kishor
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Stefan Johansson [mailto:stefan.johansson at oracle.com]
>>>>>>>>>>>> Sent: Friday, November 30, 2018 6:12 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>;
>>>>>>>>>>>> sangheon.kim at oracle.com
>>>>>>>>>>>> Cc: Viswanathan, Sandhya <sandhya.viswanathan at intel.com>
>>>>>>>>>>>> Subject: Re: RFR(M): 8211424: Allocation of old generation
>>>>>>>>>>>> of java heap on alternate memory devices - ParallelOld
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Kishor,
>>>>>>>>>>>>
>>>>>>>>>>>> On 2018-11-22 08:17, Kharbas, Kishor wrote:
>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Requesting review of the patch for allocating old
>>>>>>>>>>>>> generation of parallelold 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-8211424.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This is subtask of
>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8202286
>>>>>>>>>>>>> which is implementation for parallel old gc.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please follow the parent issue here : 64 if
>>>>>>>>>>>>> (AllocateOldGenAt != NULL
>>>>>>>>>> &&
>>>>>>>>>>>> UseAdaptiveGCBoundary) {
>>>>>>>>>>>> 65 heap_size =
>>>>>>>>>>>>
>>>>>
>>> AdjoiningGenerationsForHeteroHeap::required_reserved_memory(_collect
>>>>>>>> o
>>>>>>>>>>>> r
>>>>>>>>>>>> _policy);
>>>>>>>>>>>> 66 }
>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8202286.
>>>>>>>>>>>>>
>>>>>>>>>>>>> (PS: this is continuation of old thread 'RFR(M): 8204908:
>>>>>>>>>>>>> NVDIMM for POGC and G1GC - ReserveSpace.cpp changes are
>>>>> mostly
>>>>>>>>>>>>> eliminated/no collector specific code.')
>>>>>>>>>>>>>
>>>>>>>>>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.04
>>>>>>>>>>>> First a general comment which goes a long the same lines as
>>>>>>>>>>>> many of my comments for the G1 part. I would like to see
>>>>>>>>>>>> some more abstraction around the use of AllocateOldGenAt, I
>>>>>>>>>>>> think we should consider it during argument parsing to turn
>>>>>>>>>>>> off unsupported things like compressed oops and once during
>>>>>>>>>>>> initialization of the GC to know if we are using the feature
>>>>>>>>>>>> or not. After that there should only be calls like
>>>>>>>>>>>> policy->is_hetero_heap() if we really need to decide on this.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/gc/parallel/adjoiningGenerations.cpp
>>>>>>>>>>>> ------------------------------------------------------
>>>>>>>>>>>> 97
>>>>>>>>>>>> virtual_spaces()->reserved_space().first_part(max_low_byte_s
>>>>>>>>>>>> iz e, AllocateOldGenAt != NULL /* split */);
>>>>>>>>>>>> One example of what I mention above, I think you
>>>>>>>>>>>> should consider doing the same as you did for G1 and create
>>>>>>>>>>>> the ParallelScavengeHeap with at CollectorPolicy that is
>>>>>>>>>>>> nvdimm
>>> aware.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/gc/parallel/asPSOldGen.hpp
>>>>>>>>>>>> --------------------------------------------
>>>>>>>>>>>> 32 #include "gc/parallel/psFileBackedVirtualspace.hpp"
>>>>>>>>>>>>
>>>>>>>>>>>> I see no use of this in this header.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
>>>>>>>>>>>> ------------------------------------------------------
>>>>>>>>>>>> 65 // is the heap backed by two different memories?
>>>>>>>>>>>> 66 bool _is_hetero_heap;
>>>>>>>>>>>>
>>>>>>>>>>>> As for G1 I think this doesn't need to be stored in the heap
>>>>>>>>>>>> but the CollectorPolicy (GenerationSizer) knows this and can
>>>>>>>>>>>> be queried when needed. Keeping the getter is fine, but just
>>>>>>>>>>>> have it call the policy.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/gc/parallel/psOldGen.cpp
>>>>>>>>>>>> ------------------------------------------
>>>>>>>>>>>> 75 if(AllocateOldGenAt != NULL) {
>>>>>>>>>>>> 76 _virtual_space = new
>>>>>>>>>>>> PSFileBackedVirtualSpace(rs, alignment, AllocateOldGenAt);
>>>>>>>>>>>> 77 if (!(static_cast
>>>>>>>>>>>> <PSFileBackedVirtualSpace*>(_virtual_space))->initialize())
>>>>>>>>>>>> {
>>>>>>>>>>>> 78 vm_exit_during_initialization("Could not map
>>>>>>>>>>>> space for PSOldGen at given AllocateOldGenAt path");
>>>>>>>>>>>> 79 }
>>>>>>>>>>>>
>>>>>>>>>>>> Either pass in if this is a hetero heap or ask the heap, it
>>>>>>>>>>>> can be obtained by ParallelScavengeHeap::heap().
>>>>>>>>>>>>
>>>>>>>>>>>> I also think it would be better to have the
>>>>>>>>>>>> PSFileBackedVirtualSpace constructor call initialize
>>>>>>>>>>>> directly and let it fail rather than returning false, since
>>>>>>>>>>>> we will abort the VM in this
>>>>>>>> case anyway.
>>>>>>>>>>>> src/hotspot/share/memory/universe.cpp
>>>>>>>>>>>> -------------------------------------
>>>>>>>>>>>> 829 // When AllocateOldGenAt is set, we cannot use
>>>>>>>>>>>> largepages for entire heap memory.
>>>>>>>>>>>> 830 // Only young gen which is allocated in dram can
>>>>>>>>>>>> use large pages, but we currently don't support that.
>>>>>>>>>>>> 831 use_large_pages = (AllocateOldGenAt != NULL) ? false :
>>>>>>>>>>>> use_large_pages;
>>>>>>>>>>>>
>>>>>>>>>>>> Move this to argument parsing and do
>>>>>>>>>> FLAG_SET_DEFAULT(UseLargePages,
>>>>>>>>>>>> false) when nvdimm is used.
>>>>>>>>>>>>
>>>>>>>>>>>> src/hotspot/share/runtime/arguments.cpp
>>>>>>>>>>>> ---------------------------------------
>>>>>>>>>>>> 1627 if(AllocateOldGenAt != NULL) {
>>>>>>>>>>>> 1628 FLAG_SET_ERGO(bool, UseCompressedOops, false);
>>>>>>>>>>>> 1629 return;
>>>>>>>>>>>> 1630 }
>>>>>>>>>>>>
>>>>>>>>>>>> As for G1 I can't see no reason why this can't be moved to
>>>>>>>>>> GCArguments::init.
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Stefan
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>> http://cr.openjdk.java.net/~kkharbas/8211424/webrev.03_to_04
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>
>>>>>>>>>>>>> Kishor
>>>>>>>>>>>>>
>
More information about the hotspot-gc-dev
mailing list