Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-13 Thread Alan Bateman
On 12/05/2014 11:55, Paul Sandoz wrote: On May 12, 2014, at 12:42 PM, Alan Bateman wrote: On 12/05/2014 11:03, Paul Sandoz wrote: It covers many areas and i have grouped the patches into such areas to aid reviewing. When commenting please including core-libs. The groupings are a bit odd Ye

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-13 Thread Paul Sandoz
On May 12, 2014, at 4:07 PM, Daniel Fuchs wrote: > Hi Paul, > > I looked at -management and the changes there look good. > > There is just some two spaces vs four space formatting in >

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-13 Thread Paul Sandoz
On May 12, 2014, at 1:00 PM, Ivan Gerasimov wrote: > src/share/classes/sun/misc/UUDecoder.java > 126 StringBuilder x = new StringBuilder(); > Is only filled, but doesn't seem to be used anyhow. > Maybe just delete it? > Thanks, i will take a look at this and your other change once s/S

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Remi Forax
On 05/12/2014 12:42 PM, Alan Bateman wrote: On 12/05/2014 11:03, Paul Sandoz wrote: It covers many areas and i have grouped the patches into such areas to aid reviewing. When commenting please including core-libs. The groupings are a bit odd but I looked through the -core, -io, -management a

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Daniel Fuchs
Hi Paul, I looked at -management and the changes there look good. There is just some two spaces vs four space formatting in line 99

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Xuelei Fan
> - security > http://cr.openjdk.java.net/~psandoz/jdk9/sb/JDK-8041679-buffer-to-builder-security/webrev/ Looks fine to me. Thanks for making this update. Xuelei On 5/12/2014 6:03 PM, Paul Sandoz wrote: > Hi, > > This is a request for review of Otavio's patch replacing StringBuffer > with Stri

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Ivan Gerasimov
src/share/classes/sun/misc/UUDecoder.java 126 StringBuilder x = new StringBuilder(); Is only filled, but doesn't seem to be used anyhow. Maybe just delete it? Sincerely yours, Ivan On 12.05.2014 14:03, Paul Sandoz wrote: Hi, This is a request for review of Otavio's patch replacing

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Paul Sandoz
On May 12, 2014, at 12:42 PM, Alan Bateman wrote: > On 12/05/2014 11:03, Paul Sandoz wrote: >> >> It covers many areas and i have grouped the patches into such areas to aid >> reviewing. When commenting please including core-libs. > The groupings are a bit odd Yeah, definitely idiosyncratic,

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Alan Bateman
On 12/05/2014 11:03, Paul Sandoz wrote: It covers many areas and i have grouped the patches into such areas to aid reviewing. When commenting please including core-libs. The groupings are a bit odd but I looked through the -core, -io, -management and -rmi patches and don't see any issues. Noth

Re: JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Ivan Gerasimov
Wouldn't it be a little bit more efficient to replace a string concatenation with yet another StringBuilder operation? src/share/classes/com/sun/java/util/jar/pack/BandStructure.java 631 StringBuilder sb = new StringBuilder(); ... 636 Utils.log.fine(" meta-c

JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-12 Thread Paul Sandoz
Hi, This is a request for review of Otavio's patch replacing StringBuffer with StringBuilder within OpenJDK. (I also need to review it.) It covers many areas and i have grouped the patches into such areas to aid reviewing. When commenting please including core-libs. Jtreg tests showed no regre