Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8247373: ArraysSupport.newLength doc, test, and exception message #1617

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 61 additions & 30 deletions src/java.base/share/classes/jdk/internal/util/ArraysSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -575,52 +575,83 @@ public static int mismatch(double[] a, int aFromIndex,
}

/**
* The maximum length of array to allocate (unless necessary).
* Some VMs reserve some header words in an array.
* Attempts to allocate larger arrays may result in
* {@code OutOfMemoryError: Requested array size exceeds VM limit}
* A soft maximum array length imposed by array growth computations.
* Some JVMs (such as Hotspot) have an implementation limit that will cause
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
*
* OutOfMemoryError("Requested array size exceeds VM limit")
*
* to be thrown if a request is made to allocate an array of some length near
* Integer.MAX_VALUE, even if there is sufficient heap available. The actual
* limit might depend on some JVM implementation-specific characteristics such
* as the object header size. The soft maximum value is chosen conservatively so
* as to be smaller than any implementation limit that is likely to be encountered.
*/
public static final int MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;
public static final int SOFT_MAX_ARRAY_LENGTH = Integer.MAX_VALUE - 8;

/**
* Calculates a new array length given an array's current length, a preferred
* growth value, and a minimum growth value. If the preferred growth value
* is less than the minimum growth value, the minimum growth value is used in
* its place. If the sum of the current length and the preferred growth
* value does not exceed {@link #MAX_ARRAY_LENGTH}, that sum is returned.
* If the sum of the current length and the minimum growth value does not
* exceed {@code MAX_ARRAY_LENGTH}, then {@code MAX_ARRAY_LENGTH} is returned.
* If the sum does not overflow an int, then {@code Integer.MAX_VALUE} is
* returned. Otherwise, {@code OutOfMemoryError} is thrown.
* Computes a new array length given an array's current length, a minimum growth
* amount, and a preferred growth amount. The computation is done in an overflow-safe
* fashion.
*
* This method is used by objects that contain an array that might need to be grown
* in order to fulfill some immediate need (the minimum growth amount) but would also
* like to request more space (the preferred growth amount) in order to accommodate
* potential future needs. The returned length is usually clamped at the soft maximum
* length in order to avoid hitting the JVM implementation limit. However, the soft
* maximum will be exceeded if the minimum growth amount requires it.
*
* If the preferred growth amount is less than the minimum growth amount, the
* minimum growth amount is used as the preferred growth amount.
*
* The preferred length is determined by adding the preferred growth amount to the
* current length. If the preferred length does not exceed the soft maximum length
* (SOFT_MAX_ARRAY_LENGTH) then the preferred length is returned.
*
* @param oldLength current length of the array (must be non negative)
* @param minGrowth minimum required growth of the array length (must be
* positive)
* @param prefGrowth preferred growth of the array length (ignored, if less
* then {@code minGrowth})
* Since 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 is less than the
* soft maximum, the soft maximum is returned. If the minimum required length is
* greater than the soft maximum but does not exceed Integer.MAX_VALUE, the minimum
* required length is returned. Otherwise, the minimum required length exceeds
* Integer.MAX_VALUE, which can never be fulfilled, so this method throws OutOfMemoryError.
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
*
* Note that this method does not do any array allocation itself; it only does array
* length growth computations. However, it will throw OutOfMemoryError as noted above.
*
* Note also that this method cannot detect the JVM's implementation limit, and it
* may compute and return a length value up to and including Integer.MAX_VALUE that
* might exceed the JVM's implementation limit. In that case, the caller will likely
* attempt an array allocation with that length and encounter an OutOfMemoryError.
* Of course, regardless of the length value returned from this method, the caller
* may encounter OutOfMemoryError if there is insufficient heap to fufill the request.
Copy link

@stefan-zobel stefan-zobel Dec 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in line 626: fufill -> fulfill

*
* @param oldLength current length of the array (must be nonnegative)
* @param minGrowth minimum required growth amount (must be positive)
* @param prefGrowth preferred growth amount
* @return the new length of the array
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
* @throws OutOfMemoryError if increasing {@code oldLength} by
* {@code minGrowth} overflows.
* @throws OutOfMemoryError if the new length would exceed Integer.MAX_VALUE
*/
public static int newLength(int oldLength, int minGrowth, int prefGrowth) {
// assert oldLength >= 0
// assert minGrowth > 0

int newLength = Math.max(minGrowth, prefGrowth) + oldLength;
if (newLength - MAX_ARRAY_LENGTH <= 0) {
return newLength;
int prefLength = oldLength + Math.max(minGrowth, prefGrowth); // might overflow
if (0 < prefLength && prefLength <= SOFT_MAX_ARRAY_LENGTH) {
return prefLength;
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
} else {
return hugeLength(oldLength, minGrowth);
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
}
return hugeLength(oldLength, minGrowth);
}

private static int hugeLength(int oldLength, int minGrowth) {
int minLength = oldLength + minGrowth;
if (minLength < 0) { // overflow
throw new OutOfMemoryError("Required array length too large");
}
if (minLength <= MAX_ARRAY_LENGTH) {
return MAX_ARRAY_LENGTH;
throw new OutOfMemoryError(
"Required array length " + oldLength + " + " + minGrowth + " is too large");
} else if (minLength <= SOFT_MAX_ARRAY_LENGTH) {
return SOFT_MAX_ARRAY_LENGTH;
} else {
return minLength;
stuart-marks marked this conversation as resolved.
Show resolved Hide resolved
}
return Integer.MAX_VALUE;
}
}
4 changes: 2 additions & 2 deletions test/jdk/java/util/StringJoiner/MergeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import java.util.StringJoiner;
import java.util.stream.Stream;
import org.testng.annotations.Test;
import static jdk.internal.util.ArraysSupport.MAX_ARRAY_LENGTH;
import static jdk.internal.util.ArraysSupport.SOFT_MAX_ARRAY_LENGTH;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;

Expand Down Expand Up @@ -172,7 +172,7 @@ public void testMergeSelf() {
}

public void OOM() {
String maxString = "*".repeat(MAX_ARRAY_LENGTH);
String maxString = "*".repeat(SOFT_MAX_ARRAY_LENGTH);

try {
StringJoiner sj1 = new StringJoiner("", "", "");
Expand Down
4 changes: 2 additions & 2 deletions test/jdk/java/util/StringJoiner/StringJoinerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import java.util.ArrayList;
import java.util.StringJoiner;
import org.testng.annotations.Test;
import static jdk.internal.util.ArraysSupport.MAX_ARRAY_LENGTH;
import static jdk.internal.util.ArraysSupport.SOFT_MAX_ARRAY_LENGTH;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;

Expand All @@ -49,7 +49,7 @@ public class StringJoinerTest {
private static final String FOUR = "Four";
private static final String FIVE = "Five";
private static final String DASH = "-";
private static final String MAX_STRING = "*".repeat(MAX_ARRAY_LENGTH);
private static final String MAX_STRING = "*".repeat(SOFT_MAX_ARRAY_LENGTH);

public void addAddAll() {
StringJoiner sj = new StringJoiner(DASH, "{", "}");
Expand Down
103 changes: 103 additions & 0 deletions test/jdk/jdk/internal/util/ArraysSupport/NewLength.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/*
* @test
* @bug 8247373
* @modules java.base/jdk.internal.util
* @run testng NewLength
* @summary Test edge cases of ArraysSupport.newLength
*/

import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.fail;

import jdk.internal.util.ArraysSupport;

public class NewLength {
static final int IMAX = Integer.MAX_VALUE;
static final int SOFT = ArraysSupport.SOFT_MAX_ARRAY_LENGTH;

// Data that is expected to return a valid value.

@DataProvider(name = "valid")
public Object[][] validProvider() {
return new Object[][] {
// old min pref expected
{ 0, 1, 2, 2 },
{ 0, 2, 1, 2 },
{ 0, 1, SOFT-1, SOFT-1 },
{ 0, 1, SOFT, SOFT },
{ 0, 1, SOFT+1, SOFT },
{ 0, 1, IMAX, SOFT },
{ 0, SOFT-1, IMAX, SOFT },
{ 0, SOFT, IMAX, SOFT },
{ 0, SOFT+1, IMAX, SOFT+1 },
{ SOFT-2, 1, 2, SOFT },
{ SOFT-1, 1, 2, SOFT },
{ SOFT, 1, 2, SOFT+1 },
{ SOFT+1, 1, 2, SOFT+2 },
{ IMAX-2, 1, 2, IMAX-1 },
{ IMAX-1, 1, 2, IMAX },
{ SOFT-2, 1, IMAX, SOFT },
{ SOFT-1, 1, IMAX, SOFT },
{ SOFT, 1, IMAX, SOFT+1 },
{ SOFT+1, 1, IMAX, SOFT+2 },
{ IMAX-2, 1, IMAX, IMAX-1 },
{ IMAX-1, 1, IMAX, IMAX }
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding test cases for zero (0) for the min and preferred would be good to include and show any unpredictable behavior.

Copy link
Member Author

@stuart-marks stuart-marks Dec 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't want to add test cases that violate the preconditions. I guess I could add a prefGrowth==0 case though.

}

// Data that should provoke an OutOfMemoryError

@DataProvider(name = "error")
public Object[][] errorProvider() {
return new Object[][] {
// old min pref
{ 1, IMAX, IMAX },
{ SOFT, IMAX, 0 },
{ SOFT, IMAX, IMAX },
{ IMAX-1, 2, 0 },
{ IMAX, 1, 0 },
{ IMAX, IMAX, 0 },
{ IMAX, IMAX, IMAX }
};
}

@Test(dataProvider = "valid")
public void valid(int old, int min, int pref, int expected) {
assertEquals(ArraysSupport.newLength(old, min, pref), expected);
}

@Test(dataProvider = "error")
public void error(int old, int min, int pref) {
try {
int r = ArraysSupport.newLength(old, min, pref);
fail("expected OutOfMemoryError, got normal return value of " + r);
} catch (OutOfMemoryError oome) {
// ok
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of an //ok comment, I like to give the exception a name that makes the intent clear.

} catch (IllegalArgumentException success) {}

}
}
}