RFR: 8247373: ArraysSupport.newLength doc, test, and exception message [v5]
Pavel Rappo
prappo at openjdk.java.net
Tue Mar 2 15:52:40 UTC 2021
On Tue, 2 Mar 2021 06:29:07 GMT, Stuart Marks <smarks at 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.
>
> Stuart Marks has updated the pull request incrementally with one additional commit since the last revision:
>
> Update copyright year.
Overall this looks good.
src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 579:
> 577: /**
> 578: * A soft maximum array length imposed by array growth computations.
> 579: * Some JVMs (such as Hotspot) have an implementation limit that will cause
The numbers of spellings "HotSpot" to that of "Hotspot" in the JDK codebase is 10 to 1 respectively. Also, on my machine I can see this:
$ java --version
java 15 2020-09-15
Java(TM) SE Runtime Environment (build 15+36-1562)
Java HotSpot(TM) 64-Bit Server VM (build 15+36-1562, mixed mode, sharing)
test/jdk/jdk/internal/util/ArraysSupport/NewLength.java line 100:
> 98: int r = ArraysSupport.newLength(old, min, pref);
> 99: fail("expected OutOfMemoryError, got normal return value of " + r);
> 100: } catch (OutOfMemoryError success) { }
Consider using `expectThrows` or `assertThrows` from `org.testng.Assert`.
-------------
Marked as reviewed by prappo (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1617
More information about the core-libs-dev
mailing list