On Tue, 8 Dec 2020 06:14:35 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: > > fix typo, clarify asserts disabled, test prefGrowth==0 Many years ago, when I wrote the unfactored MAX_ARRAY_LENGTH code, I considered refactoring it, but didn't follow through because various implementations had too many small differences . I'm glad you made it work. I'm happy to see good tests added. Testability is a benefit of refactoring. I'm happy to see the name change to SOFT_MAX_ARRAY_LENGTH - that's a better name. ------------- PR: https://git.openjdk.java.net/jdk/pull/1617
