RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL
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]
> 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]
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]
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