I'm the author of most of the MAX_ARRAY_SIZE code in the jdk. We should be as consistent as we can given the history. It all looks pretty similar. --- + if (minCapacity < 0) // overflow + throw new OutOfMemoryError("Memory capacity overflow: " + + minCapacity); + I deliberately tried to push this cold code into hugeCapacity. (yeah, not worth much ...) --- I deliberately tried to avoid a hard limit within the grow code itself, instead letting the VM fail with its OOME. Perhaps the VM will surprise us by allowing the creation of the array of size Integer.MAX_VALUE, as I think it should. + * @implNote + * The maximum size permitted by this implementation is + * {@code Integer.MAX_VALUE - 8}. --- So unsurprisingly I prefer the status quo. We don't know for sure that """ ByteArrayOutputStream hugeCapacity method can return invalid capacity""" On Mon, Jul 23, 2018 at 2:49 PM, Brian Burkhalter < brian.burkhalter@oracle.com> wrote:

Hi Roger,

Updated version: http://cr.openjdk.java.net/~bpb/8206403/webrev.01/

On Jul 23, 2018, at 2:10 PM, Roger Riggs <Roger.Riggs@Oracle.com> wrote:

...

You might want to add an @requires of 8Gb or whatever so the test only runs on a system it can succeed on.

Re-tested and changed to @requires 2g and dropped the @ignore.

...

I don't see the @randomness in the test. (Other than perhaps available heap).

That was vestigial from copying the header from elsewhere.

...

ByteArrayOutputStream:121: A message indicating the nature of the error would be useful.

There was no message in the original file but I concur that one is better so I added one.

...

In the test, I don't think you need to fill the array, writing 0 is just as good as 0xff.

Changed.

Thanks for the review.

Brian

Latest hotspot code still has the limit. It's dependent on element type and VM specifics like header size, so we can't just try to see what the limit is empirically and adjust 8 to 2. // Return the maximum length of an array of BasicType. The length can passed // to typeArrayOop::object_size(scale, length, header_size) without causing an // overflow. We also need to make sure that this will not overflow a size_t on // 32 bit platforms when we convert it to a byte size. static int32_t max_array_length(BasicType type) { assert(type >= 0 && type < T_CONFLICT, "wrong type"); assert(type2aelembytes(type) != 0, "wrong type"); const size_t max_element_words_per_size_t = align_down((SIZE_MAX/HeapWordSize - header_size(type)), MinObjAlignment); const size_t max_elements_per_size_t = HeapWordSize * max_element_words_per_size_t / type2aelembytes(type); if ((size_t)max_jint < max_elements_per_size_t) { // It should be ok to return max_jint here, but parts of the code // (CollectedHeap, Klass::oop_oop_iterate(), and more) uses an int for // passing around the size (in words) of an object. So, we need to avoid // overflowing an int when we add the header. See CRs 4718400 and 7110613. return align_down(max_jint - header_size(type), MinObjAlignment); } return (int32_t)max_elements_per_size_t; } On Mon, Jul 23, 2018 at 5:40 PM, Brian Burkhalter < brian.burkhalter@oracle.com> wrote:

On Jul 23, 2018, at 5:26 PM, Martin Buchholz <martinrb@google.com> wrote:

I'm the author of most of the MAX_ARRAY_SIZE code in the jdk. We should be as consistent as we can given the history.

Thanks for the history lesson.

It all looks pretty similar.

There are in fact a number of them in java.base:

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/io/ByteArrayOutputStream.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/lang/AbstractStringBuilder.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/AbstractCollection.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/ArrayDeque.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/ArrayList.java

static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/concurrent/ConcurrentHashMap.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/concurrent/ PriorityBlockingQueue.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/Hashtable.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/PriorityQueue.java

static final long MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/stream/Nodes.java

private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; src/java.base/share/classes/java/util/Vector.java

So unsurprisingly I prefer the status quo.

I am wondering whether MAX_ARRAY_SIZE is even still necessary in current VMs? I don’t know the low level details enough to comment on that.

Thanks,

Brian

Hi Brian, A followup on the issues raised by Martin. The original issue[1] was that the resize by doubling approach failed to take advantage of nearly 1G of potential buffer space. The new issue is raised against getting the last additional 2-6 bytes of buffer space before the hitting the VM's implementation limit. I don't think its worth the effort to try to ensure those last few bytes are available before throwing OOM. Reconsidering, I would close the issue as WillNotFix for the reason that the application is encountering an implementation limit. Regards, Roger [1] https://bugs.openjdk.java.net/browse/JDK-8055949 On 7/24/18 10:26 AM, Roger Riggs wrote:

Hi Brian,

The update looks fine.

Roger

On 7/23/18 5:49 PM, Brian Burkhalter wrote:

...

Hi Roger,

Updated version: http://cr.openjdk.java.net/~bpb/8206403/webrev.01/ <http://cr.openjdk.java.net/%7Ebpb/8206403/webrev.01/>

On Jul 23, 2018, at 2:10 PM, Roger Riggs <Roger.Riggs@Oracle.com <mailto:Roger.Riggs@Oracle.com>> wrote:

...

You might want to add an @requires of 8Gb or whatever so the test only runs on a system it can succeed on.

Re-tested and changed to @requires 2g and dropped the @ignore.

...

I don't see the @randomness in the test.  (Other than perhaps available heap).

That was vestigial from copying the header from elsewhere.

...

ByteArrayOutputStream:121: A message indicating the nature of the error would be useful.

There was no message in the original file but I concur that one is better so I added one.

...

In the test, I don't think you need to fill the array, writing 0 is just as good as 0xff.

Changed.

Thanks for the review.

Brian