Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 18:41:04 GMT, Claes Redestad wrote: > Yes, while I don't know exactly which changes resolved JDK-6764325, it's > clear from the microbenchmarks added for #2102 that it's no longer an issue - > at least not in the mainline. Thanks for checking and for closing that issue. - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. +1 - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. >From my pure developer's perspective the proposed changes look good. If the >performance concerns are removed I'd say it looks good to me. - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. Yes, while I don't know exactly which changes resolved JDK-6764325, it's clear from the microbenchmarks added for #2102 that it's no longer an issue - at least not in the mainline. - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 09:18:39 GMT, Alan Bateman wrote: > It would be useful to get up to date performance data on using Charset vs. > charset name. At least historically, the charset name versions were faster > (see [JDK-6764325](https://bugs.openjdk.java.net/browse/JDK-6764325)). The code in question was changed a few times since then, the last change was done by the https://github.com/openjdk/jdk/pull/2102. So currently the code for string.getBytes String/Charset uses the same code paths, except that the string version has an additional call to lookup the charset. The string version: https://github.com/openjdk/jdk/blob/66d1faa7847b645f20ab2e966adf0a523e3ffeb2/src/java.base/share/classes/java/lang/String.java#L1753 The charset version: https://github.com/openjdk/jdk/blob/66d1faa7847b645f20ab2e966adf0a523e3ffeb2/src/java.base/share/classes/java/lang/String.java#L1777 I checked the performance and the charset is always faster, depending on the string size, up to x20. @cl4es please confirm my observation since you did it already for https://github.com/openjdk/jdk/pull/2102 - PR: https://git.openjdk.java.net/jdk/pull/5063
Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules
On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov wrote: > This is the continuation of JDK-8233884 and JDK-8271456. This change affects > fewer cases so I fix all "java." modules at once. > > In many places standard charsets are looked up via their names, for example: > absolutePath.getBytes("UTF-8"); > > This could be done more efficiently(up to x20 time faster) with use of > java.nio.charset.StandardCharsets: > absolutePath.getBytes(StandardCharsets.UTF_8); > > The later variant also makes the code cleaner, as it is known not to throw > UnsupportedEncodingException in contrary to the former variant. > > tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. It would be useful to get up to date performance data on using Charset vs. charset name. At least historically, the charset name versions were faster (see [JDK-6764325](https://bugs.openjdk.java.net/browse/JDK-6764325)). - PR: https://git.openjdk.java.net/jdk/pull/5063
RFR: 8272120: Avoid looking for standard encodings in "java." modules
This is the continuation of JDK-8233884 and JDK-8271456. This change affects fewer cases so I fix all "java." modules at once. In many places standard charsets are looked up via their names, for example: absolutePath.getBytes("UTF-8"); This could be done more efficiently(up to x20 time faster) with use of java.nio.charset.StandardCharsets: absolutePath.getBytes(StandardCharsets.UTF_8); The later variant also makes the code cleaner, as it is known not to throw UnsupportedEncodingException in contrary to the former variant. tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS. - Commit messages: - Initial fix of JDK-8272120 Changes: https://git.openjdk.java.net/jdk/pull/5063/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5063&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272120 Stats: 127 lines in 15 files changed: 24 ins; 53 del; 50 mod Patch: https://git.openjdk.java.net/jdk/pull/5063.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5063/head:pull/5063 PR: https://git.openjdk.java.net/jdk/pull/5063