You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Tim Ellison <t....@gmail.com> on 2009/05/08 11:26:41 UTC

Re: svn commit: r772849 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java

Doesn't this break the spec for ensureCapacity [1]?

It seems to cause a test case failure for me:

expected:<12> but was:<10>

junit.framework.AssertionFailedError: expected:<12> but was:<10>
at
org.apache.harmony.luni.tests.java.lang.StringBuilderTest.test_ensureCapacityI(StringBuilderTest.java:624)

[1]
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/StringBuilder.html#ensureCapacity(int)

Regards,
Tim


regisxu@apache.org wrote:
> Author: regisxu
> Date: Fri May  8 05:54:20 2009
> New Revision: 772849
> 
> URL: http://svn.apache.org/viewvc?rev=772849&view=rev
> Log:
> enlarge half size of buffer to reduce wasted memory
> 
> Modified:
>     harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
> 
> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java?rev=772849&r1=772848&r2=772849&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java (original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java Fri May  8 05:54:20 2009
> @@ -91,8 +91,8 @@
>      }
>  
>      private void enlargeBuffer(int min) {
> -        int twice = (value.length << 1) + 2;
> -        char[] newData = new char[min > twice ? min : twice];
> +        int newSize = (value.length >> 1 + value.length) + 2;
> +        char[] newData = new char[min > newSize ? min : newSize];
>          System.arraycopy(value, 0, newData, 0, count);
>          value = newData;
>          shared = false;
> 
> 
> 

Re: svn commit: r772849 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java

Posted by Regis <xu...@gmail.com>.
Regis wrote:
> Tim Ellison wrote:
>> Doesn't this break the spec for ensureCapacity [1]?
>>
>> It seems to cause a test case failure for me:
>>
>> expected:<12> but was:<10>
>>
>> junit.framework.AssertionFailedError: expected:<12> but was:<10>
>> at
>> org.apache.harmony.luni.tests.java.lang.StringBuilderTest.test_ensureCapacityI(StringBuilderTest.java:624) 
>>
> 
> Weird, it didn't appear in my last ran. How about the patch [1], that 
> will not break spec and save the memory?
> 
> [1]:
> 
> Index: modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
> =====================================================================
> --- modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
> +++ modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
> @@ -258,7 +258,8 @@ abstract class AbstractStringBuilder {
>       */
>      public void ensureCapacity(int min) {
>          if (min > value.length) {
> -            enlargeBuffer(min);
> +            int twice = (value.length << 1) + 2;
> +            enlargeBuffer(twice > min ? twice : min);
>          }
>      }
> 
> 
> 
>>
>> [1]
>> http://java.sun.com/j2se/1.5.0/docs/api/java/lang/StringBuilder.html#ensureCapacity(int) 
>>
>>
>> Regards,
>> Tim
>>
>>
>> regisxu@apache.org wrote:
>>> Author: regisxu
>>> Date: Fri May  8 05:54:20 2009
>>> New Revision: 772849
>>>
>>> URL: http://svn.apache.org/viewvc?rev=772849&view=rev
>>> Log:
>>> enlarge half size of buffer to reduce wasted memory
>>>
>>> Modified:
>>>     
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java 
>>>
>>>
>>> Modified: 
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java 
>>>
>>> URL: 
>>> http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java?rev=772849&r1=772848&r2=772849&view=diff 
>>>
>>> ============================================================================== 
>>>
>>> --- 
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java 
>>> (original)
>>> +++ 
>>> harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java 
>>> Fri May  8 05:54:20 2009
>>> @@ -91,8 +91,8 @@
>>>      }
>>>  
>>>      private void enlargeBuffer(int min) {
>>> -        int twice = (value.length << 1) + 2;
>>> -        char[] newData = new char[min > twice ? min : twice];
>>> +        int newSize = (value.length >> 1 + value.length) + 2;
>>> +        char[] newData = new char[min > newSize ? min : newSize];
>>>          System.arraycopy(value, 0, newData, 0, count);
>>>          value = newData;
>>>          shared = false;
>>>
>>>
>>>
>>
> 
> 

It passed all luni tests, so I applied it to repository at r772937.

-- 
Best Regards,
Regis.

Re: svn commit: r772849 - /harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java

Posted by Regis <xu...@gmail.com>.
Tim Ellison wrote:
> Doesn't this break the spec for ensureCapacity [1]?
> 
> It seems to cause a test case failure for me:
> 
> expected:<12> but was:<10>
> 
> junit.framework.AssertionFailedError: expected:<12> but was:<10>
> at
> org.apache.harmony.luni.tests.java.lang.StringBuilderTest.test_ensureCapacityI(StringBuilderTest.java:624)

Weird, it didn't appear in my last ran. How about the patch [1], that will not 
break spec and save the memory?

[1]:

Index: modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
=====================================================================
--- modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
+++ modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
@@ -258,7 +258,8 @@ abstract class AbstractStringBuilder {
       */
      public void ensureCapacity(int min) {
          if (min > value.length) {
-            enlargeBuffer(min);
+            int twice = (value.length << 1) + 2;
+            enlargeBuffer(twice > min ? twice : min);
          }
      }



> 
> [1]
> http://java.sun.com/j2se/1.5.0/docs/api/java/lang/StringBuilder.html#ensureCapacity(int)
> 
> Regards,
> Tim
> 
> 
> regisxu@apache.org wrote:
>> Author: regisxu
>> Date: Fri May  8 05:54:20 2009
>> New Revision: 772849
>>
>> URL: http://svn.apache.org/viewvc?rev=772849&view=rev
>> Log:
>> enlarge half size of buffer to reduce wasted memory
>>
>> Modified:
>>     harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
>>
>> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java
>> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java?rev=772849&r1=772848&r2=772849&view=diff
>> ==============================================================================
>> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java (original)
>> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/java/lang/AbstractStringBuilder.java Fri May  8 05:54:20 2009
>> @@ -91,8 +91,8 @@
>>      }
>>  
>>      private void enlargeBuffer(int min) {
>> -        int twice = (value.length << 1) + 2;
>> -        char[] newData = new char[min > twice ? min : twice];
>> +        int newSize = (value.length >> 1 + value.length) + 2;
>> +        char[] newData = new char[min > newSize ? min : newSize];
>>          System.arraycopy(value, 0, newData, 0, count);
>>          value = newData;
>>          shared = false;
>>
>>
>>
> 


-- 
Best Regards,
Regis.