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
<[email protected]> 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
<[email protected]> 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
