Re: [PATCH] Optimization of AbstractStringBuilder.ensureCapacityInternal()

[Roman Leventov]

Hi Claes, indeed, seems that this change breaks the zeroing optimization,
so ensureCapacityInternal() becomes slower when the char count is
comparable with the array length. Thanks.

On 22 February 2018 at 18:35, Claes Redestad 
wrote:

> Hi,
>
> interesting - do you have any numbers showing a benefit from doing this
> (on various sets of input)?
>
> My concerns are that count might typically be close enough to value.length
> to make the difference small in practice, and that there are some (fragile)
> JIT optimizations to try and avoid zeroing out the newly allocated array
> that we have to take care not to break.
>
> /Claes
>
>
> On 2018-02-22 17:51, Roman Leventov wrote:
>
>> AbstractStringBuilder.ensureCapacityInternal() doesn't need to copy the
>> whole underlying array, only the part until the current count.
>>
>> diff --git
>> a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
>> b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
>> --- a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
>> +++ b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
>> @@ -140,11 +140,12 @@
>>* overflow, this method throws {@code OutOfMemoryError}.
>>*/
>>   private void ensureCapacityInternal(int minimumCapacity) {
>> +byte[] oldValue = value;
>>   // overflow-conscious code
>> -int oldCapacity = value.length >> coder;
>> +int oldCapacity = oldValue.length >> coder;
>>   if (minimumCapacity - oldCapacity > 0) {
>> -value = Arrays.copyOf(value,
>> -newCapacity(minimumCapacity) << coder);
>> +value = new byte[newCapacity(minimumCapacity) << coder];
>> +System.arraycopy(oldValue, 0, value, 0, count << coder);
>>   }
>>   }
>>
>
>

Re: [PATCH] Optimization of AbstractStringBuilder.ensureCapacityInternal()

[Claes Redestad]

Hi,

interesting - do you have any numbers showing a benefit from doing this
(on various sets of input)?

My concerns are that count might typically be close enough to value.length
to make the difference small in practice, and that there are some (fragile)
JIT optimizations to try and avoid zeroing out the newly allocated array
that we have to take care not to break.

/Claes

On 2018-02-22 17:51, Roman Leventov wrote:

AbstractStringBuilder.ensureCapacityInternal() doesn't need to copy the
whole underlying array, only the part until the current count.

diff --git
a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
--- a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
+++ b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
@@ -140,11 +140,12 @@
   * overflow, this method throws {@code OutOfMemoryError}.
   */
  private void ensureCapacityInternal(int minimumCapacity) {
+byte[] oldValue = value;
  // overflow-conscious code
-int oldCapacity = value.length >> coder;
+int oldCapacity = oldValue.length >> coder;
  if (minimumCapacity - oldCapacity > 0) {
-value = Arrays.copyOf(value,
-newCapacity(minimumCapacity) << coder);
+value = new byte[newCapacity(minimumCapacity) << coder];
+System.arraycopy(oldValue, 0, value, 0, count << coder);
  }
  }

Re: [PATCH] Optimization of AbstractStringBuilder.ensureCapacityInternal()

[Roman Leventov]

Similar optimizations are also possible in ArrayList and Vector.

On 22 February 2018 at 17:51, Roman Leventov  wrote:

> AbstractStringBuilder.ensureCapacityInternal() doesn't need to copy the
> whole underlying array, only the part until the current count.
>
> diff --git a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
> b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
> --- a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
> +++ b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
> @@ -140,11 +140,12 @@
>   * overflow, this method throws {@code OutOfMemoryError}.
>   */
>  private void ensureCapacityInternal(int minimumCapacity) {
> +byte[] oldValue = value;
>  // overflow-conscious code
> -int oldCapacity = value.length >> coder;
> +int oldCapacity = oldValue.length >> coder;
>  if (minimumCapacity - oldCapacity > 0) {
> -value = Arrays.copyOf(value,
> -newCapacity(minimumCapacity) << coder);
> +value = new byte[newCapacity(minimumCapacity) << coder];
> +System.arraycopy(oldValue, 0, value, 0, count << coder);
>  }
>  }
>

[PATCH] Optimization of AbstractStringBuilder.ensureCapacityInternal()

[Roman Leventov]

AbstractStringBuilder.ensureCapacityInternal() doesn't need to copy the
whole underlying array, only the part until the current count.

diff --git
a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
--- a/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
+++ b/src/java.base/share/classes/java/lang/AbstractStringBuilder.java
@@ -140,11 +140,12 @@
  * overflow, this method throws {@code OutOfMemoryError}.
  */
 private void ensureCapacityInternal(int minimumCapacity) {
+byte[] oldValue = value;
 // overflow-conscious code
-int oldCapacity = value.length >> coder;
+int oldCapacity = oldValue.length >> coder;
 if (minimumCapacity - oldCapacity > 0) {
-value = Arrays.copyOf(value,
-newCapacity(minimumCapacity) << coder);
+value = new byte[newCapacity(minimumCapacity) << coder];
+System.arraycopy(oldValue, 0, value, 0, count << coder);
 }
 }