On Tue, 2 Mar 2021 06:29:07 GMT, Stuart Marks <sma...@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