Regarding the wording, I don't think either "implementation limit" or "VM limit"
is correct, at least not in all the cases.
If we were using ArraysSupport.newLength, the error messages are as follows:
1) Actual shortage of heap space. Hotspot generates this:
OutOfMemoryError: Java heap space
2) Requested array size exceeds implementation limit. Hotspot
generates this:
OutOfMemoryError: Requested array size exceeds VM limit
3) Integer wraparound/overflow during size computation.
ArraysSupport.newLength() generates this:
OutOfMemoryError: Required array length too large
For this changeset, we're leaving several places as they are, either using the
*exact arithmetic methods or doing their own checks. OK, we'll save refactoring
to ArraysSupport.newLength for another time. But the resulting OOME message is I
think misleading.
For example, in String.java line 2190, we catch an exception from Math.addExact
or Math.multiplyExact. This occurs if the arithmetic result exceeds the range of
an int. It's less about some implementation or VM limit, but instead the
requested length simply cannot be represented by an int.
Also, while I'm not as allergic to talking about arrays as Martin, in this case
the subsequent code allocates a StringBuilder and not an array. So it is
somewhat odd for this message to refer to an array.
Anyway, instead of
Requested array size exceeds VM limit
I'd suggest this:
Requested size too large
(Or "length" instead of "size". "Length" is more consistent for arrays and
strings, but oddly enough, Hotspot uses "array size".)
I think this wording could apply to String.java, StringLatin1.java,
StringUTF16.java, Pattern.java, UnsyncByteArrayOutputStream.java, and
ByteArrayChannel.java. I guess we could try to provide a customized message for
each area, but that wouldn't seem to me to add much value.
I'm not entirely sure what's going on in AbstractStringBuilder.java. It uses
ArraysSupport.newLength(), which should either return an in-range value or
throw. I don't understand why the code in ASB.newCapacity() is checking length
against Integer.MAX_VALUE and throwing again. This shouldn't happen by the spec
of AS.newLength(). I think that check (lines 258-259) can simply be removed.
s'marks
On 6/10/20 10:22 AM, Roger Riggs wrote:
Hi Jim,
In Unsafe.java: 632: the "Unable to allocate 98494837395: doesn't read very
well.
Please add " bytes" to the end.
I would probably drop "array" from all the messages but at least in the String*
cases.
As Martin notes, the array is an implementation detail.
(And i still prefer "implementation limit" over "VM limit".
Thanks, Roger
On 6/10/20 12:33 PM, Jim Laskey wrote:
On Jun 130, 2020, at 1:15 PM, Martin Buchholz <marti...@google.com> wrote:
I took a look at PriorityBlockingQueue.
Part of converting to ArraysSupport involves deleting the local orphan
MAX_ARRAY_SIZE; that needs to be done.
Removed.
---
It looks like
newCap > oldCap
is always true, so we should delete the test?
if (newCap > oldCap && queue == array)
---
If oldCap == MAX_ARRAY_SIZE wouldn't newCap == oldCap
In Pattern.java I see
+ throw new OutOfMemoryError("Requested array size exceeds
VM limit");
That wording doesn't seem useful to me - the use of an array is an
implementation detail, and the user didn't __request__ it.
Better seems the wording in ArraysSupport
throw new OutOfMemoryError("Required array length too large");
but if we're going to the trouble of composing a custom detail
message, I'd try harder to find one meaningful to the user of the API,
something like "pattern too large"
Done and done.
Thank you.
On Wed, Jun 10, 2020 at 5:15 AM Jim Laskey <james.las...@oracle.com> wrote:
Will push if no comments by EOB.
On Jun 8, 2020, at 2:22 PM, Jim Laskey <james.las...@oracle.com> wrote:
Revised to use a consistent error message. Modified AbstractStringBuilder
and PriorityBlockingQueue to use ArraysSupport.newLength(). Didn't modify
ByteArrayChannel and UnsyncByteArrayOutputStream since those changes would
require changes to module exporting.
webrev: http://cr.openjdk.java.net/~jlaskey/8230744/webrev-03/index.html
<http://cr.openjdk.java.net/~jlaskey/8230744/webrev-03/index.html>
On Jun 3, 2020, at 11:24 AM, Jim Laskey <james.las...@oracle.com> wrote:
It's not the goal or role of this bug to fix the wrongs of the past,
merely add error messages to the exceptions. I raised the discussion as an
issue. Clearly there is a correct path to follow. If you think more effort
is required then file a bug. :-)
Cheers,
-- Jim
On Jun 2, 2020, at 7:13 PM, Stuart Marks <stuart.ma...@oracle.com> wrote:
On 6/2/20 6:52 AM, Jim Laskey wrote:
Revised to reflect requested changes.
http://cr.openjdk.java.net/~jlaskey/8230744/webrev-01/index.html
<http://cr.openjdk.java.net/~jlaskey/8230744/webrev-01/index.html>
On this, if all you're doing is changing exception messages, then I don't
care very much, modulo concerns about wording from others. If you start
to get into changing the growth logic (like the Math.addExact stuff) then
please see my message on the related thread, "Sometimes constraints are
questionable."
Thanks,
s'marks
