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=5063=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
