On Monday, Sep 22, 2003, at 20:45 Pacific/Auckland, Ralph Loader wrote:
My interpretation of the original code was that it was meant to prevent the possibility that a small substring of a large StringBuffer would prevent the large array being gc'd.
The patch you checked in breaks this. Consider:
String Foo() { StringBuffer b = new StringBuffer(); ... put a megabyte of stuff into b ... String ignored = b.toString(); // sets b.shared return b.substring (0, 1); }
The 1 character String returned from Foo now has it's contents stored in
a megabyte char[], and that array has no other references.
Passing ((len << 2) >= value.length) rather than StringBuffer.shared to the String constructor prevented this.
Its hard to say what is optimal here, as you can also argue that if the string is already shared then its reasonable to share even if the substring is small. However, having to copy a small string unnecessarily doesn't seem as bad as potentially wasting a lot of memory in the scenario above. So, I'm checking in the following patch, which reverts to the behaviour of Ralph's original patch.
2003-09-24 Bryce McKinlay <[EMAIL PROTECTED]>
* java/lang/StringBuffer.java (substring): Don't set `shared' on
small Strings, even if buffer is already shared.
Index: StringBuffer.java
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/StringBuffer.java,v
retrieving revision 1.16
diff -u -r1.16 StringBuffer.java
--- StringBuffer.java 22 Sep 2003 08:17:48 -0000 1.16
+++ StringBuffer.java 24 Sep 2003 06:02:00 -0000
@@ -564,11 +564,12 @@
throw new StringIndexOutOfBoundsException();
if (len == 0)
return "";
- // Share unless substring is smaller than 1/4 of the buffer.
- if ((len << 2) >= value.length)
- shared = true;
+ // Don't copy unless substring is smaller than 1/4 of the buffer.
+ boolean share_buffer = ((len << 2) >= value.length);
+ if (share_buffer)
+ this.shared = true;
// Package constructor avoids an array copy.
- return new String(value, beginIndex, len, shared);
+ return new String(value, beginIndex, len, share_buffer);
}
/**
Regards
Bryce.
_______________________________________________ Classpath mailing list [EMAIL PROTECTED] http://mail.gnu.org/mailman/listinfo/classpath
