RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL

2021-01-20 Thread eirbjo
By moving string splitting and concatenation into the canonizeString utility, 
we can defer allocation until we determine that canonization is required. This 
saves two string allocations and a string concat for the common case where 
canonization is not required.

As a refactoring, move ParseUtil.canonizeString/doCanonize into Handler since 
that's the only call site.

Finally, let's rename the method to canonizalizeString, since canonization is 
an rather unrelated term.

-

Commit messages:
 - Move utilities from ParseUtil into Handler, rename to canonicalizeString, 
call file.length() only once.
 - Reduce allocation by deferring substring calls until canonizeString 
determines it is needed

Changes: https://git.openjdk.java.net/jdk/pull/2167/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2167&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260043
  Stats: 95 lines in 2 files changed: 47 ins; 47 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2167.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2167/head:pull/2167

PR: https://git.openjdk.java.net/jdk/pull/2167


Re: RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL [v2]

2021-01-20 Thread eirbjo
> By moving string splitting and concatenation into the canonizeString utility, 
> we can defer allocation until we determine that canonization is required. 
> This saves two string allocations and a string concat for the common case 
> where canonization is not required.
> 
> As a refactoring, move ParseUtil.canonizeString/doCanonize into Handler since 
> that's the only call site.
> 
> Finally, let's rename the method to canonizalizeString, since canonization is 
> an rather unrelated term.

eirbjo has updated the pull request incrementally with one additional commit 
since the last revision:

  As a part of the rename from "canonize" to "canonicalize", doCanonize should 
also be renamed. This was an oversight in the original PR.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2167/files
  - new: https://git.openjdk.java.net/jdk/pull/2167/files/6e169aa0..f4b543d6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2167&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2167&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2167.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2167/head:pull/2167

PR: https://git.openjdk.java.net/jdk/pull/2167


Re: RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL [v2]

2021-01-20 Thread eirbjo
On Wed, 20 Jan 2021 15:32:13 GMT, Claes Redestad  wrote:

>> eirbjo has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   As a part of the rename from "canonize" to "canonicalize", doCanonize 
>> should also be renamed. This was an oversight in the original PR.
>
> src/java.base/share/classes/sun/net/www/protocol/jar/Handler.java line 232:
> 
>> 230: }
>> 231: 
>> 232: private static String doCanonize(String file) {
> 
> nit: doCanonicalize

Good catch! Not enough coffee :-)

-

PR: https://git.openjdk.java.net/jdk/pull/2167


Re: RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL [v2]

2021-01-20 Thread eirbjo
On Wed, 20 Jan 2021 15:34:21 GMT, Claes Redestad  wrote:

>> eirbjo has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   As a part of the rename from "canonize" to "canonicalize", doCanonize 
>> should also be renamed. This was an oversight in the original PR.
>
> LGTM - just a small nit that you seem to have forgotten to change canonize to 
> canonicalize in some places.

Newbie question: Is a copyright year updates required when code is simply moved 
around like this?

More specifically, is it required when strictly deleting code from a file, like 
ParseUtil in this case?

-

PR: https://git.openjdk.java.net/jdk/pull/2167