RFR 8223593 : Refactor code for reallocating storage

Ivan Gerasimov ivan.gerasimov at oracle.com
Fri May 10 21:36:26 UTC 2019

Hi Peter!

Thank you for reviewing!

On 5/10/19 1:52 AM, Peter Levart wrote:
> Hi Ivan,
> On 5/9/19 8:07 PM, Ivan Gerasimov wrote:
>>> 3. I know that this is not new and has been copied from the old 
>>> code. However,
>>> I'm not sure I understand the meaning of "unless necessary" here:
>>>      /**
>>>       * The maximum size of array to allocate (unless necessary).
>> It means that if the minimum requested new capacity (oldCapacity + 
>> growAtLeastBy) is greater than MAX_ARRAY_SIZE (though still not 
>> greater than Integer.MAX_VALUE) then the result *will* be greater 
>> Current implementation returns Integer.MAX_VALUE in this case. I was 
>> thinking about changing it to returning the actual sum (oldCapacity + 
>> growAtLeastBy), but decided not to do that to preserve the behavior.
>> Practically, it shouldn't matter much, as both variants would likely 
>> lead to OOM anyway.
>> With kind regards,
>> Ivan 
> Is there a case where returning > MAX_ARRAY_SIZE will not lead to OOME?
Yes.  I just tried (successfully) and could allocate up to 
byte[Integer.MAX_INTEGER - 2].
Greater size result in OutOfMemoryError: Requested array size exceeds VM 

> If this utility method is meant for re-sizing arrays only (currently 
> it is only used for that), then perhaps the method could throw OOME 
> explicitly in this case. You already throw OOME for the overflow case, 
> so currently the method is not uniform in returning exceptional values 
> (i.e. values that lead to exceptions).
> Unless you expect some VMs to tolerate arrays as large as 
> Integer.MAX_VALUE ?
I prefer to go conservative with this refactoring, and preserve the 
behavior as close as possible to the current one.

Later, I think, it could be reconsidered.
For example, as I wrote earlier, it may be desirable to make 
hugeCapacity(minCapacity) return minCapacity instead of Integer.MAX_VALUE.

For now, I'd rather keep the implementation to minimize risk of regressions.

> These lines:
>  607         int newCapacity = oldCapacity + preferredGrowBy;
>  608         if (preferredGrowBy < growAtLeastBy) {
>  609             newCapacity = oldCapacity + growAtLeastBy;
>  610         }
> ...could perhaps be more easily grasped as:
> int newCapacity = oldCapacity + Math.max(preferredGrowBy, growAtLeastBy);

Okay, let's do it with Math.max().  I'll update the webrev shortly.

Thank you!


> Regards, Peter

With kind regards,
Ivan Gerasimov

More information about the core-libs-dev mailing list