On Fri, 4 Dec 2020 15:47:51 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> This rewrites the doc of ArraysSupport.newLength, adds detail to the 
>> exception message, and adds a test. In addition to some renaming and a bit 
>> of refactoring of the actual code, I also made two changes of substance to 
>> the code:
>> 
>> 1. I fixed a problem with overflow checking. In the original code, if 
>> oldLength and prefGrowth were both very large (say, Integer.MAX_VALUE), this 
>> method could return a negative value. It turns out that writing tests helps 
>> find bugs!
>> 
>> 2. Under the old policy, if oldLength and minGrowth required a length above 
>> SOFT_MAX_ARRAY_LENGTH but not above Integer.MAX_VALUE, this method would 
>> return Integer.MAX_VALUE. That doesn't make any sense, because attempting to 
>> allocate an array of that length will almost certainly cause the Hotspot to 
>> throw OOME because its implementation limit was exceeded. Instead, if the 
>> required length is in this range, this method returns that required length.
>> 
>> Separately, I'll work on retrofitting various call sites around the JDK to 
>> use this method.
>
> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 654:
> 
>> 652:             return SOFT_MAX_ARRAY_LENGTH;
>> 653:         } else {
>> 654:             return minLength;
> 
> Isn't this last `else if... then.. else` the same as:
> `return Math.max(minLength, SOFT_MAX_ARRAY_LENGTH)`

It is, and I considered replacing it, but I felt that it obscured what was 
going on.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1617

Reply via email to