Potential StringBuffer to StringBuilder clean-up (where warranted)

[Mike Duigou]

Hi Martijn;


Stefan as really dived into this project! Thank you for helping him make progress and assisting him with getting his changes ready to commit.

As with the warnings changes, because the changes span multiple projects, it will be necessary to break up the webrevs a bit and submit them to the various jdk sub-mailing lists (awt, net, security, corelibs, langtools, etc.)

StringBuffer to StringBuilder conversions are an excellent contribution.

On May 4 2012, at 07:17 , Martijn Verburg wrote:

> Hi all,
> 
> Stefan Reich has submitted a patch to the Adopt OpenJDK program which
> we're looking to pre-review that converts (in theory, all easily and
> automatically convertible) uses of StringBuffer into StringBuilder.
> The motivation is to bring some performance benefits where
> synchronization is not required.

In almost all cases the StringBuffer objects would have been uncontended and hotspot will elid the locking. In theory there won't be significant performance differences but it's still worth doing. 

> It covers all files in
> src/share/classes, but could be extended to cover tools and platform
> behaviour as well.
> 
> * The refactoring deals with instances of StringBuffers that are
> created in a method, and which don't escape out of their scope.

This should make the conversions entirely safe.

> * There are a few instances where fields are converted, but they are
> in classes that are not serializable, for example in LogStream.
> * A lot of the changes are in the toString() methods, but there are
> more frequently used methods in RandomAccessFile (for example,
> readLine()) and Double that use StringBuffer.

The uses I looked at in Double and RandomAccessFile don't seem to have any special reason to be using StringBuffer. I can't think of any non-escaping uses of StringBuffer where conversion to StringBuilder wouldn't be appropriate.

> * We're going to check that there really is no synchronization that is
> going on (to the best of our ability)

If the StringBuffer instances don't escape their creating method then this shouldn't be a concern.

> Quick Q:  Is moving from StringBuffer to StringBuilder generally
> desirable?

Yes.

> I'm concerned it might be too dangerous given that whether
> or not it's being used in a synchronized context may not be easily
> detectable.
> 
> If it is generally desirable, then any extra advice on what we should
> look out for in pre-reviewing this patch is most welcome.

I think that for the most part you have it covered by ensuring that the StringBuffer instances are all ones that don't escape the method in which they are created.

Mike