Re: Request for Review : CR#6924259: Remove String.count/String.offset

2013-04-12 Thread Krystal Mok
Hi Nazario, That's a typical substring leak case. Matcher.group(n) eventually calls String.substring() to return the result. Before removing the count and offset fields in String, that'll return a String sharing the value char array, which may cause memory leaks if you hold on to a small substring

Request for Review : CR#6924259: Remove String.count/String.offset

2013-04-12 Thread wangsheng.0376
hi, all, I agree with you to remove offset, today when I run following code in jdk7(sorry I forget the detail version), my code is like this: Pattern pattern = Pattern.compile(regex); Matcher matcher = pattern.match(content); while (matcher.find()) { String link = matcher.group(1);

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-29 Thread Mike Duigou
I have posted a new webrev to: > Removing String(int offset, int count, char value[]) will create trouble, >> I've seen several libraries that use it by reflection to avoid to create >> a copy of the array. I think this method should not dis

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Ulf Zibis
Am 25.05.2012 17:05, schrieb Jim Gish: On 05/25/2012 09:48 AM, Mike Duigou wrote: I too mostly prefer casts without a space. The code was reformatted with an IDE because I'm actually terrible about being consistent in my formatting. Mechanically formatted code, like mechanically separated meat,

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Rémi Forax
On 05/26/2012 12:09 AM, Mike Duigou wrote: On May 25 2012, at 06:57 , Rémi Forax wrote: Hi Mike, Hi Alan, Hi all, in my opinion, EMPTY_STRING_VALUE is a premature optimization, the idea is, I think, to avoid to create an empty char but if you want to do that, it's better to do that in StringBu

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Mike Duigou
On May 25 2012, at 06:57 , Rémi Forax wrote: > Hi Mike, Hi Alan, Hi all, > in my opinion, EMPTY_STRING_VALUE is a premature optimization, > the idea is, I think, to avoid to create an empty char but if you want to do > that, > it's better to do that in StringBuilder.toString() to avoid to create

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread David Schlosnagle
>> On 24/05/2012 21:45, Mike Duigou wrote: >>> >>> Hello all; >>> >>> For a long time preparations and planing have been underway to remove the >>> offset and count fields from java.lang.String. These two fields enable >>> multiple String instances to share the same backing character buffer. Shared

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Jim Gish
On 05/25/2012 09:48 AM, Mike Duigou wrote: A non-material comment is that there are a couple of style changes that I found annoying. Everyone is an expert on such matters, I'm not, but the one that bugged me a bit was adding the space after the cast, eg: (toffset> (long) value.length - len) I

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Rémi Forax
Hi Mike, Hi Alan, Hi all, in my opinion, EMPTY_STRING_VALUE is a premature optimization, the idea is, I think, to avoid to create an empty char but if you want to do that, it's better to do that in StringBuilder.toString() to avoid to create the String at all. I'm worried about one pathologica

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Mike Duigou
> A non-material comment is that there are a couple of style changes that I > found annoying. Everyone is an expert on such matters, I'm not, but the one > that bugged me a bit was adding the space after the cast, eg: > (toffset > (long) value.length - len) > I found myself needing to re-read it

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Alan Bateman
On 24/05/2012 21:45, Mike Duigou wrote: Hello all; For a long time preparations and planing have been underway to remove the offset and count fields from java.lang.String. These two fields enable multiple String instances to share the same backing character buffer. Shared character buffers wer

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-25 Thread Ulf Zibis
Am 25.05.2012 01:11, schrieb Mike Duigou: On May 24 2012, at 15:56 , Ulf Zibis wrote: Am 24.05.2012 22:45, schrieb Mike Duigou: A patch of the changes to java.lang.String for JDK 8 are at. The changes for JDK 7 have only superficial diff

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-24 Thread Mike Duigou
On May 24 2012, at 15:56 , Ulf Zibis wrote: > Am 24.05.2012 22:45, schrieb Mike Duigou: >> A patch of the changes to java.lang.String for JDK 8 are >> at. The changes for >> JDK 7 have only superficial differences (line offsets in the patch

Re: Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-24 Thread Ulf Zibis
Am 24.05.2012 22:45, schrieb Mike Duigou: A patch of the changes to java.lang.String for JDK 8 are at. The changes for JDK 7 have only superficial differences (line offsets in the patch). Correct Copyright date. 157 public String(Str

Request for Review : CR#6924259: Remove String.count/String.offset

2012-05-24 Thread Mike Duigou
Hello all; For a long time preparations and planing have been underway to remove the offset and count fields from java.lang.String. These two fields enable multiple String instances to share the same backing character buffer. Shared character buffers were an important optimization for old bench