Thanks, Ivan. I see that the capacity growth x -> 2*x + 2 is mandated by the spec, so we can't change that. Growing by more than a factor of two may mean that the "overflow-conscious" code I originally wrote may need more checks than ArrayList, where growing by 3/2 provides some sanity.
--- Looking at the compact string code for the first time, I wonder that replacing a char[] with a byte[] has effectively reduced the capacity by 2, when non-LATIN1 is stored. If you wanted to preserve the maximum capacity, you would need to use a char[] for storage as before? ---- Anyways, AbstractStringBuilder looks even trickier to get right than ArrayList or Vector. On Mon, Feb 22, 2016 at 3:36 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote: > > > On 22.02.2016 22:49, Martin Buchholz wrote: >> >> Can you make the code look more like the recently optimized code in >> ArrayList and Vector? > > > Sure. Please find the updated webrev below. > Of course, there are nuances, as we need to deal with the compact strings, > so the code isn't quite the same as in ArrayList. > > http://cr.openjdk.java.net/~igerasim/8149330/01/webrev/ > > Sincerely yours, > Ivan > > >> On Mon, Feb 22, 2016 at 11:20 AM, Ivan Gerasimov >> <ivan.gerasi...@oracle.com> wrote: >>> >>> Hello! >>> >>> When the capacity of a StringBuilder needs to be increased (either due to >>> append() or due to explicit call to ensureCapacity()), the new capacity >>> is >>> calculated as "twice the old capacity, plus 2", rounded down to >>> Integer.MAX_VALUE: >>> >>> http://docs.oracle.com/javase/8/docs/api/java/lang/StringBuilder.html#ensureCapacity-int- >>> Because of that, StringBuilder throws OOM early, even though there may be >>> room to grow. >>> >>> The proposed solution is to reserve a few bytes at the top of the range >>> and >>> only try to allocate them if absolutely required. >>> >>> The regression test is @ignored by default, as it is too greedy for >>> memory. >>> >>> Would you please help review the fix? >>> >>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8149330 >>> WEBREV: http://cr.openjdk.java.net/~igerasim/8149330/00/webrev/ >>> >>> Sincerely yours, >>> Ivan > >
