RFR: 8149330: Capacity of StringBuilder should not get close to Integer.MAX_VALUE unless necessary

Martin Buchholz martinrb at google.com
Tue Feb 23 06:23:31 UTC 2016


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.gerasimov at 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.gerasimov at 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
>
>



More information about the core-libs-dev mailing list