Re: Patch: Updated `String +' patch

[Eric Blake <ebb9 at email dot byu dot edu>]

Some comments on gnu.gcj.runtime.StringBuffer:

Notice that java.lang.StringBuffer(Object) and StringBuffer(String) do
not gracefully handle null, so the compiler must use new
StringBuffer().append(o).  But is there any reason that we can't make
gnu.gcj.runtime.StringBuffer(o) work properly? It would be one less
method to call, and one less char[] growth since the constructor can
look at o's initial string length.

I agree with Per; lose the shared flag.

More comments below...

Tom Tromey wrote:
> 
> This patch changes gcj to use a private, unsychronized copy of
> StringBuffer.  This speeds up String `+' operations quite a bit.
> 
> I tested this by rebuilding libgcj with it.
> I also ran performance tests which I posted a while back.
> 
> Ok to commit?
> 
> Tom
> 
> Index: libjava/gnu/gcj/runtime/StringBuffer.java
> ===================================================================
> RCS file: StringBuffer.java
> diff -N StringBuffer.java
> --- /dev/null   Tue May  5 13:32:27 1998
> +++ libjava/gnu/gcj/runtime/StringBuffer.java Mon Dec 10 21:09:09 2001
> @@ -0,0 +1,237 @@
> +// This is a simplified copy of java.lang.StringBuffer with
> +// `synchronized' removed.
> +
> +/* StringBuffer.java -- Growable strings
> +   Copyright (C) 1998, 1999, 2000, 2001 Free Software Foundation, Inc.

I'm no copyright lawyer, but are all those dates necessary for a new
file?

> +  public StringBuffer append (boolean bool)
> +  {
> +    return append (String.valueOf(bool));
> +  }

Would this be more efficient as:
return append(bool ? "true" : "false");

> +  public StringBuffer append (long lnum)
> +  {
> +    return append (String.valueOf(lnum));
> +  }

Similarly, could this call be made as:
return append(Long.toString(lnum));
to save a method call on the stack? (Similar comments for the other
methods). Of course, here the compiler may have already determined that
when optimizing...


> +  public StringBuffer append (String str)
> +  {
> +    if (str == null)
> +      str = "null";
> +    int len = str.length();
> +    ensureCapacity_unsynchronized (count + len);
> +    str.getChars(0, len, value, count);
> +    count += len;
> +    return this;
> +  }

Rather than calling str.length(), couldn't you use str.count, as it is
package visible?


> +  public StringBuffer append (char[] data)
> +  {
> +    return append (data, 0, data.length);
> +  }

Why do you need this? String concatenation never uses it:
void foo()
{
  char[] ca = {'h', 'i'};
  String s = "" + ca;
}

s must contain some string like "[C@abc1240", not "hi"; the compiler
should be emitting StringBuffer.append((Object) ca).

> +  public StringBuffer append (char[] data, int offset, int count)
> +  {
> +    ensureCapacity_unsynchronized (this.count + count);
> +    System.arraycopy(data, offset, value, this.count, count);
> +    this.count += count;
> +    return this;
> +  }

Ditto.


> +
> +  static final long serialVersionUID = 3388685877147921107L;

Does this class really need to be serialized?


-- 
This signature intentionally left boring.

Eric Blake             ebb9@email.byu.edu
  BYU student, free software programmer