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

[Xueming Shen]

On 2/22/16 10:23 PM, Martin Buchholz wrote:
> 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?

 From certain perspective it's a kinda of "regression" that the maximum 
capacity for a non-latin1
buffer/builder is reduced by 2 . But arguably it's really an 
implementation detail that how big a
StringBuffer/Builder can really go, as the spec and the implementation 
don't/can't guarantee
you can really have a buffer/build with a Integer.MAX_VALUE capacity.  
On the other hand on
certain system you might be able to have a bigger buffer/builder for 
latin-1 only characters, as
it only requires half the space with the compact string implementation. 
That said, I was debating
whether or not the constructor (with the capacity parameter) should 
check the capacity, with the
assumption that the buffer/builder might be for non-latin1 input. But it 
doesn't like the check
will bring in any benefit...

Sherman

>
> ----
>
> 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
>>