That looks better. One suggestion is to use StringBuilder.append(s, 0, i) to avoid temp substring.
Thanks Sent from my phone On Nov 4, 2013 8:35 AM, "Michael McMahon" <michael.x.mcma...@oracle.com> wrote: > Vitaly, > > Good points, I agree. I think the optimisation is worthwhile > and not costly. I also found a small casting issue with > StringBuilder.append(char) > being interpreted as StringBuilder.append(int) > > Here is the updated webrev: > > http://cr.openjdk.java.net/~michaelm/8027687/webrev.2/ > > Thanks, > Michael > > > On 02/11/13 01:03, Vitaly Davidovich wrote: > > The size to use is the length of the argument, which you're already using > for the loop. > > On a separate note, is toLowerCase in a perf sensitive area? It makes an > assumption that the lowering will need to happen (by always allocating the > stringbuilder) but is that a common case? If this isn't perf sensitive then > disregard. > > Thanks > > Sent from my phone > On Nov 1, 2013 4:28 PM, "Michael McMahon" <michael.x.mcma...@oracle.com> > wrote: > >> On 01/11/13 18:06, Mike Duigou wrote: >> >>> A couple minor quibbles >>> >>> - Since the length is know the StringBuildiler can be created with a >>> size. >>> >> >> Right, 255 is probably a good size to use. >> >> - sb.toString() is probably more efficient than new String(sb) >>> >> >> Since Chris also suggests it, I'm curious why this is. Is there some >> clever sharing going on between >> StringBuilder and String? >> >> - I would like to see some IDN URL cases in the tests. >>> >> >> The first version of this class doesn't support Unicode strings in the >> hostname labels. >> So, I'm guessing you mean cases of IDNs that have been already converted >> into the ascii encoded form (eg xn--blahblah.xn-blah.com). Something >> I'd like to do >> for JDK 9 will be to allow transparent Unicode in classes like >> URLPermission with >> automatic IDN conversions taking place in the http protocol handler. >> So, I can add some cases of encoded IDNs in the test okay. >> >> Thanks! >> >> Michael >> >> Mike >>> >>> On Nov 1 2013, at 07:46 , Michael McMahon <michael.x.mcma...@oracle.com> >>> wrote: >>> >>> Simple bug fix to new URLPermission class, caused by insufficient >>>> parameter checking >>>> of the constructor. >>>> >>>> webrev: http://cr.openjdk.java.net/~michaelm/8027687/webrev.1/ >>>> >>>> Thanks, >>>> Michael >>>> >>> >> >
