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 <mailto: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
<http://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
<mailto: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/
<http://cr.openjdk.java.net/%7Emichaelm/8027687/webrev.1/>
Thanks,
Michael
