RFR(S): 8142341: GC: current flags need ranges to be implemented
Tom Benson
tom.benson at oracle.com
Tue Dec 1 18:51:25 UTC 2015
Hi Sangheon,
On 11/30/2015 6:51 PM, sangheon.kim wrote:
> Hi Tom,
>
> Thank you for reviewing this!
>
> On 11/30/2015 02:46 PM, Tom Benson wrote:
>> Hi Sangheon,
>>
>> I think there's a problem with using the same routine to check
>> HeapBaseMinAddress as is used for checking heap size in
>> commandLineFlagConstraintsGC.cpp. I tried running this, to make sure
>> I understood what it was trying to do. When I specified something too
>> large for HeapBaseMinAddress, the check reported that the value must
>> be greater than or equal to maximum value N.
> Sorry Tom, I can't understand.
> Currently we print the error message if its value is too large like,
> "must be less than or equal to aligned maximum value (xxx)".
> Do you mean it should be 'address' instead of 'value'?
What I meant was that I got an internal error when I used the value the
message recommended, so perhaps the value should be further constrained.
> I thought it is okay.
>
>> Re-running with that (still huge) value, I get a fatal internal error
>> out of virtualspace.cpp. (In the debug version, an assertion in
>> universe.cpp triggers before reaching this point). EG:
> I am trying to reproduce this assert but I can't. Do you have more
> information to reproduce this?
> I did -version and running GCOld to trigger GC.
>
> java -XX:HeapBaseMinAddress=18446744073642442752 -version
> java -XX:HeapBaseMinAddress=18446744073642442752 GCOld 20 200 10 100 5000
>
You probably didn't hit the problem because you're running on a smaller
memory system than I am. On the system I'm on, the (computed) max heap
size is 32G. And if you specify an Xmx, you don't go through the code
path that causes it.
Here's the root of the problem. In Arguments::set_heap_size(), we have
this code:
if (FLAG_IS_DEFAULT(MaxHeapSize)) {
. . .
if (UseCompressedOops) {
// Limit the heap size to the maximum possible when using
compressed oops
julong max_coop_heap = (julong)max_heap_for_compressed_oops();
. . .
if (HeapBaseMinAddress + MaxHeapSize < max_coop_heap) { <========
*** PROBLEM IS HERE ***
// Heap should be above HeapBaseMinAddress to get zero based
compressed oops
// but it should be not less than default MaxHeapSize.
max_coop_heap -= HeapBaseMinAddress;
}
reasonable_max = MIN2(reasonable_max, max_coop_heap);
}
HeapBaseMinAddress is 64-bit unsigned, and in this case is
0xfffffffffc000000 (the 18446744073642442752 from the command line).
MaxHeapSize is 0x7CCCCC8 and max_coop_heap is 0x7FE000000. The addition
of HeapBaseMinAddress and MaxHeapSize overflows to 0x3ccccc8. Since
this is less than 0x7FE000000, we subtract the 0xfffffffffc000000 from
max_coop_heap, getting the result 0x802000000. reasonable_max is
0x800000000, so it stays at that value after the MIN2, rather than being
reduced as it should be.
Changing the conditional to also check for overflow avoids the
problem. But of course you don't get the requested HeapBaseMinAddress
anyway, since it's too high.
So another approach would be to change your constraint to ensure that
(HeapBaseMinAddress plus MaxHeapSize) won't overflow. I don't think
there's any point in allowing a HeapBaseMinAddress so high that it would.
Tom
> Thanks,
> Sangheon
>
>
>>
>> $ $JAVA_HOME/bin/java -XX:HeapBaseMinAddress=0xffffffffffffffff -version
>> HeapBaseMinAddress (18446744073709551615) must be less than or equal
>> to aligned maximum value (18446744073642442752)
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>>
>> $ $JAVA_HOME/bin/java -XX:HeapBaseMinAddress=18446744073642442752
>> -version
>> #
>> # A fatal error has been detected by the Java Runtime Environment:
>> #
>> # Internal Error (virtualspace.cpp:456), pid=10359, tid=10360
>> # guarantee(size + noaccess_prefix_size(alignment) <=
>> OopEncodingHeapMax) failed: can not allocate compressed oop heap for
>> this size
>> #
>>
>> Perhaps you should check only the alignment in the constraint code,
>> without checking the range, because I'm not sure you have the final
>> heap size at that point. Then maybe the heap reservation code should
>> report this as a non-internal error, at the point where the assertion
>> occurs, if the user specified a base address.
>>
>> There's also an extraneous single quote in
>> commandLineFlagConstraintsGC.cpp in the comment at line 510.
>>
>> Tom
>>
>>
>> Date: Tue, 24 Nov 2015 06:37:41 -0800
>> From: "sangheon.kim"<sangheon.kim at oracle.com>
>> To: Dmitry Dmitriev<dmitry.dmitriev at oracle.com>,
>> "hotspot-dev at openjdk.java.net Developers"
>> <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR(S): 8142341: GC: current flags need ranges to be
>> implemented
>> Message-ID:<56547635.9060808 at oracle.com>
>> Content-Type: text/plain; charset=utf-8; format=flowed
>>
>> Hi Dmitry,
>>
>> Thank you for the review!
>> Sure I will update my patch related to your enhancement.
>>
>> Thanks,
>> Sangheon
>>
>>
>> On 11/24/2015 06:30 AM, Dmitry Dmitriev wrote:
>>
>>> Hi Sangheon,
>>>
>>> Looks good to me! Thank you for fixing that. I hope that enhancement
>>> will be pushed today(currently in JPRT queue), so please update your
>>> patch after that!
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 21.11.2015 0:04, sangheon.kim wrote:
>>>> Hi all,
>>>>
>>>> Could I have a couple of reviews for this change to add explicit
>>>> 'range' for flags?
>>>>
>>>> Previously, we added 'range' when it is needed to avoid assert/crash
>>>> but now explicit 'range' are needed for all flags.
>>>> All flags should have 'range' or 'constraint' at least.
>>>>
>>>> CR:https://bugs.openjdk.java.net/browse/JDK-8142341
>>>> Webrev:http://cr.openjdk.java.net/~sangheki/8142341/webrev.00
>>>> Testing: JPRT, RBT
>>>> (hotspot/test/:hotspot_all,testlist,noncolo.testlist
>>>> --add-tonga-keyword quick), hotspot/test/runtime/CommandLine for
>>>> embedded
>>>>
>>>> * Summary
>>>> 1. Added 3 new constraint functions.
>>>> 1) HeapBaseMinAddress: Added to avoid an overflow after align up
>>>> and this flag makes hang up without constraint function. (newly added
>>>> as a part of GC work)
>>>> 2) TLABWasteIncrement: Without this constraint function we don't
>>>> have problems (even many GCs happen). But added to avoid an overflow
>>>> at ThreadLocalAllocBuffer::record_slow_allocation(). There are also
>>>> separate CR for this overflow ( JDK-8143352 ).
>>>> 3) NUMAInterleaveGranularity: Added to avoid an overflow after
>>>> align up.
>>>>
>>>> 2. Some flags have narrower range than their type.
>>>> 1) Here's the reason why some flags should have different range.
>>>> (same order from globals.hpp)
>>>> {flag type} {flag name}: (range), {comment}
>>>> size_t NUMAInterleaveGranularity: (os::vm_allocation_granularity(),
>>>> max_uintx), there is a comment at numa_interleaving_init() that this
>>>> flag should be larger than vm_allocation_granularity().
>>>> uintx CMSOldPLABReactivityFactor: (1,max_uintx), couldn't be zero as
>>>> used for multiplication
>>>> uintx CMS_FLSPadding: (0, max_juint), used to set a function which
>>>> has 'unsigned int' type input parameter.
>>>> uintx CMS_SweepPadding: (0, max_juint), used to set a function which
>>>> has 'unsigned int' type input parameter.
>>>> intx CMSWaitDuration: (min_jint, max_jint), used to set a function
>>>> which has 'long' type input parameter.
>>>> uintx PausePadding: (0, max_juint), used to set a function which has
>>>> 'unsigned int' type input parameter.
>>>> uintx PromotedPadding: (0, max_juint), used to set a function which
>>>> has 'unsigned int' type input parameter.
>>>> uintx SurvivorPadding: (0, max_juint), used to set a function which
>>>> has 'unsigned int' type input parameter.
>>>> uintx AdaptiveSizePolicyCollectionCostMargin: (0, 100), as this flag
>>>> is divided by 100 I assume this is percentage.
>>>> uintx GCTimeRatio: (0, max_juint), used to set a function which has
>>>> 'unsigned int' type input parameter.
>>>> intx PrefetchCopyIntervalInBytes: (-1, max_jint)
>>>> intx PrefetchScanIntervalInBytes: (-1, max_jint)
>>>> intx PrefetchFieldsAhead: (-1, max_jint), I think these 3 Prefetch*
>>>> flags should have same upper limit and looking at their related codes
>>>> 'max_jint' seems appropriate ( no problem with 'max_jint' during
>>>> testing). However, as Prefetch::read/write() needs 'intx', 'intx'
>>>> also seems good but we have to fix some codes (maybe including
>>>> generated codes).
>>>> uintx CPUForCMSThread: (0, max_juint), used to set a function which
>>>> has 'unsigned int' type input parameter.
>>>> uintx ProcessDistributionStride: (0, max_juint), used to set uint
>>>> variable and used 'for loop' which has uint increment. i.e. for (uint
>>>> i=0; i<ProcessDistributionStride; i++)
>>>> uintx CMSCoordinatorYieldSleepCount: (0, max_juint), used at
>>>> 'increment for loop()' as max value and the increment is uint.
>>>> uintx CMSYieldSleepCount: (0, max_juint), used at 'increment for
>>>> loop()' as max value and the increment is uint.
>>>> uintx TLABRefillWasteFraction: (1, max_juint), used as a return value
>>>> of uint type function and for division. i.e. uint
>>>> GCTLABConfiguration::tlab_refill_waste_limit()() { return
>>>> TLABRefillWasteFraction; }
>>>> uintx TLABWasteIncrement: (0, max_jint), type cast to (int)
>>>> intx PrintCMSStatistics: (0,2), flag to choose print mode and we only
>>>> use " if (a !=0, >0, >1)".
>>>> intx PrintFLSStatistics: (0,2), flag to choose print mode and we only
>>>> use " if (a !=0, >0, >1)".
>>>> intx PrintFLSCensus: (0,1), flag to choose print mode and we only use
>>>> " if (a !=0, >0)".
>>>> uintx GCDrainStackTargetSize: (0, max_juint), type cast to 'unsigned
>>>> int'
>>>>
>>>> (g1_globals.hpp)
>>>> intx G1ConcRefinementThresholdStep: (0, max_jint), used to set (int)
>>>> type variable.
>>>>
>>>> 3. Excluded flags from TestOptionsWithRanges.java
>>>> 1) Memory and thread related flags: tests for these flags will
>>>> consume too many resources from the system.
>>>> 2) VMThreadStackSize: range work for this flag is not included in
>>>> this patch but I faced OOME several times, so excluded.
>>>>
>>>> * I will reflect Dmitry's enhancement(JDK-8143038: [TESTBUG]
>>>> TestOptionsWithRanges: allow excluding only a subset of tested values
>>>> specified for a flag) next time.
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>>
>>>>
>
More information about the hotspot-dev
mailing list