RFR(S): 8142341: GC: current flags need ranges to be implemented

sangheon.kim sangheon.kim at oracle.com
Fri Dec 4 20:17:18 UTC 2015


Hi Tom,

On 12/04/2015 12:12 PM, Tom Benson wrote:
> Hi Sangheon,
>
> On 12/4/2015 3:01 PM, sangheon.kim wrote:
>> Hi Tom,
>>
>> On 12/04/2015 11:17 AM, Tom Benson wrote:
>>> Hi Sangheon,
>>> I'm also also OK with the change, but I found the wording of this 
>>> comment a little confusing, because 'value' is also the name of the 
>>> argument which is the HeapBaseMinAddress being checked:
>>>
>>>   554  // But below check is okay as the wrong value means bigger 
>>> than the value should be.
>>>
>>> How about something like: If an overflow happened in 
>>> Arguments::set_heap_size(), MaxHeapSize will have too large a value. 
>>> Check for this by ensuring that MaxHeapSize plus the requested min 
>>> base address still fit within max_uintx.
>>> ?
>> Your suggestion seems much cleaner. I will use this!
>> Do you need a new webrev for this?
>>
> No, I'm OK with it.  (Assuming you're also going to remove the 
> existing 553. 8^)  )
Ooops! :)

To make sure, here is the diff:

  Flag::Error HeapBaseMinAddressConstraintFunc(size_t value, bool verbose) {
-  // Current MaxHeapSize would have wrong value if an overflow happened 
at Arguments::set_heap_size().
-  // But below check is okay as the wrong value means bigger than the 
value should be.
+  // If an overflow happened in Arguments::set_heap_size(), MaxHeapSize 
will have too large a value.
+  // Check for this by ensuring that MaxHeapSize plus the requested min 
base address still fit within max_uintx.
    if (UseCompressedOops && FLAG_IS_ERGO(MaxHeapSize) && (value > 
(max_uintx - MaxHeapSize))) {

Thanks,
Sangheon


> Thanks,
> Tom
>
>
>> Thanks,
>> Sangheon
>>
>>
>>> Tnx,
>>> Tom
>>>
>>>
>



More information about the hotspot-dev mailing list