[11] RFR: 8197849: Misc improvements to URL parsing

[Daniel Fuchs]

Hi Claes,

Your proposed changes to ParseUtil look reasonable
to me, though I had to carefully compare the characters
in the range (c >= '&' && c <= ':') with the
L_ENCODED / H_ENCODED masks to convince myself
that there was no behavior change to
ParseUtil::firstEncodeIndex.

I wonder whether this would deserve some additional
comment - though I'm not sure how it could be formulated.

Given the sensitivity of the impacted code maybe it would
be prudent to wait for a second review before pushing.

best regards,

-- daniel

On 14/02/2018 15:30, Claes Redestad wrote:
> Hi,
> 
> as a means to improve startup in some applications, please review this 
> set of small improvements to improve both interpreted and compiled 
> performance of creating and handling certain jar URLs. Some of the 
> changes in sun.net.www.ParseUtil::encodePath have a small, positive 
> effect when dealing with other types of path resources.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8197849
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8197849/jdk.00/
> 
> This shaves off a percent or so of the total bytecode execution in a few 
> of our startup tests:
> 
> - ParseUtil::encodePath cost is reduced ~20% during startup, averaging 
> ~10-15% faster for typical inputs after JIT. Weird examples like paths 
> only consisting of slashes and dots can be seen to take a small hit due 
> to not getting special treatment.
> 
> - ParseUtil::canonizeString cost on startup reduced by 50% (~15% 
> improvement after JIT) for typical inputs by adding a test to return 
> directly if there's no need to "canonize" the string (which is typically 
> always the case for well-formed jar files). I added a sanity test to 
> ensure I didn't accidentally change semantics of cases that would lead 
> to canonicalization.
> 
> - Removed a couple of unnecessary allocation in 
> sun.net.www.protocol.jar.Handler. Maybe there are some good reasons not 
> to make ParseUtil a final utility class with only static methods and a 
> private constructor, though...
> 
> Thanks!
> 
> /Claes