RFR 8223593 : Refactor code for reallocating storage

[Peter Levart]

Hi Ivan, Roger,

What about "calcNewLength" ?

The word "new" gives enough hint as to what the method does - it 
calculates the length of new array to be allocated instead of old one.

Regards, Peter

On 5/17/19 8:45 PM, Ivan Gerasimov wrote:
> Hi Roger!
>
> I think that not only the name, but also the arguments compose the 
> signature.
> So, calcLength(oldLength, minGrowth, prefGrowth) is meant to be read 
> as "given old length and the amount(s) to grow, calculate the new 
> length".
> And since this method is placed into ArraysSupport, it should be clear 
> that it is an array's length.
>
> I understand that in certain contexts size may sound more natural than 
> length.
> Here, however, using 'length' should be a hint that we're dealing with 
> array.length.
>
> With kind regards,
> Ivan
>
> On 5/17/19 10:35 AM, Roger Riggs wrote:
>> Hi Ivan,
>>
>> The new calcLength method name is too generic, it does not say enough 
>> about its function.
>> There is no indication that the purpose is to resize an array.
>> As for size vs length, sometime size is more evocative of the 
>> function being performed
>> than 'length' and is more natural.  In most cases, size and length 
>> are understood to be synonyms
>> and capacity is a very apt term for how many elements can be held.
>>
>> Can I recommend resizedLength.
>>
>> Regards, Roger
>>
>>
>>
>> On 05/15/2019 10:47 PM, Ivan Gerasimov wrote:
>>> Thank you Pavel and Roger for reviewing!
>>>
>>> I apologize for reiterating this.
>>> After off-line discussion with Stuart Marks, the fix was modified 
>>> once again.
>> !!
>>> The modifications were mostly stylistic:  The used terminology now 
>>> reflects that we work with arrays (thus 'length', not 'size' or 
>>> 'capacity').
>>> Functionally, the fix remains exactly the same.
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8223593
>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8223593/02/webrev/
>>>
>>> With kind regards,
>>> Ivan
>>>
>>> On 5/14/19 7:50 AM, Roger Riggs wrote:
>>>> Hi Ivan,
>>>>
>>>> The updated patch looks fine.
>>>>
>>>> Strictly speaking, the change to Files.readAllBytes is not 
>>>> indicated by the bug report
>>>> so please update or comment on the bug report to mention that change.
>>>>
>>>> Thanks, Roger
>>>>
>>>>
>>>> On 05/13/2019 10:44 AM, Pavel Rappo wrote:
>>>>> Thanks for updating your patch. The updated code seems fine.
>>>>>
>>>>> -Pavel
>>>>>
>>>>>> On 11 May 2019, at 05:01, Ivan 
>>>>>> Gerasimov<ivan.gerasimov at oracle.com>  wrote:
>>>>>>
>>>>>> Hello!
>>>>>>
>>>>>> Please help review the updated fix.
>>>>>>
>>>>>> This new webrev includes changes suggested by Pavel, Peter and 
>>>>>> Roger.
>>>>>>
>>>>>> BUGURL:https://bugs.openjdk.java.net/browse/JDK-8223593
>>>>>> WEBREV:http://cr.openjdk.java.net/~igerasim/8223593/01/webrev/
>>>>>>
>>>>>> Please note that the behavior of j.n.f.Files.readAllBytes() has 
>>>>>> changed slightly, so now it *may* be possible to read a file 
>>>>>> larger than (Integer.MAX_VALUE - 8), if VM is able to allocate 
>>>>>> that large array.
>>>>>>
>>>>>> With kind regards,
>>>>>> Ivan
>>>>>>
>>>>>> On 5/8/19 6:50 PM, Ivan Gerasimov wrote:
>>>>>>> Hello!
>>>>>>>
>>>>>>> Jdk has several places with similar logic:  an array needs to be 
>>>>>>> reallocated (by at least some defined amount), taking into 
>>>>>>> account the maximum allowed size of arrays.
>>>>>>>
>>>>>>> There's clearly an opportunity for refactoring, so it is 
>>>>>>> proposed to introduce a dedicated utility method for calculating 
>>>>>>> the best new size of an array.
>>>>>>>
>>>>>>> Would you please help review this enhancement?
>>>>>>>
>>>>>>> BUGURL:https://bugs.openjdk.java.net/browse/JDK-8223593
>>>>>>> WEBREV:http://cr.openjdk.java.net/~igerasim/8223593/00/webrev/
>>>>>>>
>>>>>>> Mach5 job ran fine.
>>>>>>>
>>>>>>> Thanks in advance!
>>>>>>>
>>>>>> -- 
>>>>>> With kind regards,
>>>>>> Ivan Gerasimov
>>>>>>
>>>>
>>>
>>> -- 
>>> With kind regards,
>>> Ivan Gerasimov
>>
>