Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-06-16 Thread Paul Sandoz
After some further reflection i have decided to not separate this out and some bits going to client and some bits going to dev, and have pushed all changes to dev. I realise this might cause some friction but believe it the right thing to do. I remain unconvinced that the separate process for p

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-19 Thread Paul Sandoz
On May 19, 2014, at 6:18 PM, Phil Race wrote: > On 5/19/2014 12:50 AM, Alan Bateman wrote: >> On 19/05/2014 07:53, Paul Sandoz wrote: >>> If i don't have permission to push to the client repo (which might be >>> likely) i will need to hand over the patch to yourself or Sergey to commit. >>> An

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-19 Thread Phil Race
On 5/19/2014 12:50 AM, Alan Bateman wrote: On 19/05/2014 07:53, Paul Sandoz wrote: If i don't have permission to push to the client repo (which might be likely) i will need to hand over the patch to yourself or Sergey to commit. And i presume this will have to be a separate issue. If you do dec

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-19 Thread Alan Bateman
On 19/05/2014 07:53, Paul Sandoz wrote: If i don't have permission to push to the client repo (which might be likely) i will need to hand over the patch to yourself or Sergey to commit. And i presume this will have to be a separate issue. If you do decide to split this then it will require crea

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-18 Thread Paul Sandoz
On May 16, 2014, at 8:33 PM, Phil Race wrote: > On 5/16/2014 5:13 AM, Paul Sandoz wrote: >> On May 14, 2014, at 11:05 AM, Alan Bateman wrote: : This code is under src/share/classes, so it's not clear to me what classes i can modify and push in the jdk9-dev/jdk repo or not. I

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-16 Thread Phil Race
On 5/16/2014 5:13 AM, Paul Sandoz wrote: On May 14, 2014, at 11:05 AM, Alan Bateman wrote: : This code is under src/share/classes, so it's not clear to me what classes i can modify and push in the jdk9-dev/jdk repo or not. It's confusing! Is there a list online somewhere? Out of all the cla

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-16 Thread Paul Sandoz
On May 14, 2014, at 11:05 AM, Alan Bateman wrote: > >> : >> >> This code is under src/share/classes, so it's not clear to me what classes i >> can modify and push in the jdk9-dev/jdk repo or not. It's confusing! Is >> there a list online somewhere? >> >> Out of all the classes here: >> >>

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-14 Thread Alan Bateman
On 14/05/2014 09:15, Paul Sandoz wrote: : And here what could have been a 2 line change is 25 .. http://cr.openjdk.java.net/~psandoz/jdk9/sb/JDK-8041679-buffer-to-builder-media/webrev/src/share/classes/sun/font/StandardTextSource.java.sdiff.html So I would say leave the variable names alone unle

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-14 Thread Paul Sandoz
HI Phil, Thanks for looking at this. On May 13, 2014, at 11:15 PM, Phil Race wrote: > Paul, > > I don't see why you changed the variable names in some cases. Note it's not me :-) I am, mostly, the proxy. > See here where one change is only one line since you left it alone and the > other i

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

2014-05-13 Thread Phil Race
Paul, I don't see why you changed the variable names in some cases. See here where one change is only one line since you left it alone and the other is 6 lines since you changed it http://cr.openjdk.java.net/~psandoz/jdk9/sb/JDK-8041679-buffer-to-builder-media/webrev/src/share/classes/javax/pri

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

2014-05-13 Thread Paul Sandoz
On May 13, 2014, at 1:10 PM, Sergey Bylokhov wrote: > Hi, Paul. > adding 2d-dev@ > > media: sound/awt/swing part looks fine. Thanks. > Note that this part of the fix should be pushed to client forest. > Which classes exactly from here: http://cr.openjdk.java.net/~psandoz/jdk9/sb/JDK-80

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

2014-05-13 Thread Sergey Bylokhov
Hi, Paul. adding 2d-dev@ media: sound/awt/swing part looks fine. Note that this part of the fix should be pushed to client forest. On 5/12/14 2:03 PM, Paul Sandoz wrote: Hi, This is a request for review of Otavio's patch replacing StringBuffer with StringBuilder within OpenJDK. (I also need t

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