On Tue, 8 Dec 2020 06:14:35 GMT, Stuart Marks <[email protected]> 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.
>
> Stuart Marks has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fix typo, clarify asserts disabled, test prefGrowth==0
Marked as reviewed by psandoz (Reviewer).
src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 616:
> 614: * greater than the soft maximum but does not exceed
> Integer.MAX_VALUE, the minimum
> 615: * required length is returned. Otherwise, the minimum required
> length exceeds
> 616: * Integer.MAX_VALUE, which can never be fulfilled, so this method
> throws OutOfMemoryError.
I think you can simplify with:
Suggestion:
* If the preferred length exceeds the soft maximum, we use the minimum
growth
* amount. The minimum required length is determined by adding the minimum
growth
* amount to the current length.
* If the minimum required length exceeds Integer.MAX_VALUE, then this
method
* throws OutOfMemoryError. Otherwise, this method returns the soft maximum
or
* minimum required length, which ever is greater.
Then i think it follows that `Math.max` can be used in the implementation.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1617
