Potential StringBuffer to StringBuilder clean-up (where warranted)

[Martijn Verburg]

Hi Mike,

Thanks for the detailled reply, we'll get going on the pre-reviews!

Cheers,
Martijn

On 4 May 2012 17:05, Mike Duigou <mike.duigou at oracle.com> wrote:
> 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
>