RFR: 8055949: ByteArrayOutputStream capacity should be maximal array size permitted by VM
Hi friends of ByteArrayOutputStream, I'm trying to clean up an apparent oversight when I tried to fix huge array resizing back in 6933217: Huge arrays handled poorly in core libraries https://bugs.openjdk.java.net/browse/JDK-8055949 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MA... The 2x capacity gap was noticed by real users!
On 25/08/2014 18:37, Martin Buchholz wrote:
Hi friends of ByteArrayOutputStream,
I'm trying to clean up an apparent oversight when I tried to fix huge array resizing back in 6933217: Huge arrays handled poorly in core libraries
https://bugs.openjdk.java.net/browse/JDK-8055949 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MA... <http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/ByteArrayOutputStream-MAX_ARRAY_SIZE/>
The 2x capacity gap was noticed by real users! The change to BAOS looks okay, I don't recall why it wasn't updated with the previous changes.
I'm not sure about adding a test with @ignore though, maybe the @test should just be dropped to avoid the temptation to "fix" the test before there is support for selecting tests based on the resources available. -Alan
Thanks, Alan. On Mon, Aug 25, 2014 at 1:28 PM, Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 25/08/2014 18:37, Martin Buchholz wrote:
Hi friends of ByteArrayOutputStream,
I'm trying to clean up an apparent oversight when I tried to fix huge array resizing back in 6933217: Huge arrays handled poorly in core libraries
https://bugs.openjdk.java.net/browse/JDK-8055949
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MA...
The 2x capacity gap was noticed by real users!
The change to BAOS looks okay, I don't recall why it wasn't updated with the previous changes.
I'm not sure about adding a test with @ignore though, maybe the @test should just be dropped to avoid the temptation to "fix" the test before there is support for selecting tests based on the resources available.
jtreg tests are run by default with -ignore:quiet and there is precedent for other tests with such @ignore statements, and I actually used jtreg -ignore:run to really test this. What I would really like someday is to be able to run expensive tests (like this one) only if a user expressly requested them. Anyways, I'm intending to submit as is unless you feel strongly.
On 25/08/2014 21:45, Martin Buchholz wrote:
:
jtreg tests are run by default with -ignore:quiet and there is precedent for other tests with such @ignore statements, and I actually used jtreg -ignore:run to really test this.
What I would really like someday is to be able to run expensive tests (like this one) only if a user expressly requested them.
Anyways, I'm intending to submit as is unless you feel strongly. The issue here is that we still lack a means to select tests based on resources and other criteria. I hope this come in time so it will be possible to write tests like this properly.
In the mean-time then my only concern is that pushing a test with @ignore is just adding to the technical debt. Such tests are unlikely to be run or maintained. If you do push it with @test then a minor comment is to maybe push in the second line of @summary so that it is more distinguishable from the @ lines. -Alan.
I'll submit soonish. @summary second line indented. I added some gratuitous assertions to the test: // check data integrity while we're here byte[] bytes = baos.toByteArray(); if (bytes.length != n) throw new AssertionError("wrong length"); if (bytes[0] != 'x' || bytes[bytes.length - 1] != 'x') throw new AssertionError("wrong contents"); On Tue, Aug 26, 2014 at 12:07 AM, Alan Bateman <Alan.Bateman@oracle.com> wrote:
In the mean-time then my only concern is that pushing a test with @ignore is just adding to the technical debt. Such tests are unlikely to be run or maintained. If you do push it with @test then a minor comment is to maybe push in the second line of @summary so that it is more distinguishable from the @ lines.
I don't think it particularly adds to the technical debt because there's an existing pile of @ignored tests that share the same reason, that will all be handled together ... in the fullness of time.
This looks fine to me as well. I am fine with the @ignore as I don't suspect anyone would be able to sneak in a change which removed the @ignore without anyone noticing and the comment for why it is marked @ignore seems adequate. Mike On Aug 25 2014, at 13:28 , Alan Bateman <Alan.Bateman@oracle.com> wrote:
On 25/08/2014 18:37, Martin Buchholz wrote:
Hi friends of ByteArrayOutputStream,
I'm trying to clean up an apparent oversight when I tried to fix huge array resizing back in 6933217: Huge arrays handled poorly in core libraries
https://bugs.openjdk.java.net/browse/JDK-8055949 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MA... <http://cr.openjdk.java.net/%7Emartin/webrevs/openjdk9/ByteArrayOutputStream-MAX_ARRAY_SIZE/>
The 2x capacity gap was noticed by real users! The change to BAOS looks okay, I don't recall why it wasn't updated with the previous changes.
I'm not sure about adding a test with @ignore though, maybe the @test should just be dropped to avoid the temptation to "fix" the test before there is support for selecting tests based on the resources available.
-Alan
Am 25.08.2014 um 19:37 schrieb Martin Buchholz:
https://bugs.openjdk.java.net/browse/JDK-8055949 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ByteArrayOutputStream-MA...
The 2x capacity gap was noticed by real users!
Hi Martin, the MAX_ARRAY_SIZE code now is copied to many places in JDK. Couldn't you better centralize the code to one place, e.g. j.u.Arrays or some hidden class of sun.java...? Isn't there a property to retrieve MAX_ARRAY_SIZE from the running VM? Imagine, some VM throws OOME above Integer.MAX_VALUE-4 and minCapacity is Integer.MAX_VALUE-4. With this code a OOME will happen: 124 return (minCapacity > MAX_ARRAY_SIZE) ? 125 Integer.MAX_VALUE : 126 MAX_ARRAY_SIZE; With this code we would avoid the OOME: 124 return (minCapacity > MAX_ARRAY_SIZE) ? 125 minCapacity : 126 MAX_ARRAY_SIZE; -Ulf
I decided to care just enough about the last 2x of scalability, but not about the last 8 elements. Your code would resize the 2g elements 8 times before finally reaching MAX_VALUE... Now I'm back in not-caring-anymore mode. At least about MAX_ARRAY_SIZE. On Wed, Aug 27, 2014 at 3:41 AM, Ulf Zibis <Ulf.Zibis@cosoco.de> wrote:
Am 25.08.2014 um 19:37 schrieb Martin Buchholz:
https://bugs.openjdk.java.net/browse/JDK-8055949
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/ ByteArrayOutputStream-MAX_ARRAY_SIZE/
The 2x capacity gap was noticed by real users!
Hi Martin,
the MAX_ARRAY_SIZE code now is copied to many places in JDK. Couldn't you better centralize the code to one place, e.g. j.u.Arrays or some hidden class of sun.java...? Isn't there a property to retrieve MAX_ARRAY_SIZE from the running VM?
Imagine, some VM throws OOME above Integer.MAX_VALUE-4 and minCapacity is Integer.MAX_VALUE-4. With this code a OOME will happen: 124 return (minCapacity > MAX_ARRAY_SIZE) ? 125 Integer.MAX_VALUE : 126 MAX_ARRAY_SIZE; With this code we would avoid the OOME: 124 return (minCapacity > MAX_ARRAY_SIZE) ? 125 minCapacity : 126 MAX_ARRAY_SIZE;
-Ulf
participants (4)
-
Alan Bateman
-
Martin Buchholz
-
Mike Duigou
-
Ulf Zibis