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

[Tom Benson]

Looks OK to me.
Thanks,
Tom

On 12/4/2015 3:17 PM, sangheon.kim wrote:
> 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
>>>>
>>>>
>>
>