RFR: 8259842: Remove Result cache from StringCoding [v5]

[Claes Redestad]

On Sun, 17 Jan 2021 09:21:27 GMT, Peter Levart <plevart at openjdk.org> wrote:

>> This looks much better now. Maybe just one small suggestion: `java.lang.StringCoding#lookupCharset` is used in two places and in both places the same exception handling/rethrowing logic is wrapped around the invocation. So you could move that logic into the lookupCharset method which would simplify call sites. You could even get rid of String.lookupCharset method that way.
>
> When you combine the logic of String.lookupCharset:
> 
>     private static Charset lookupCharset(String charsetName)
>             throws UnsupportedEncodingException {
>         Objects.requireNonNull(charsetName);
>         try {
>             Charset cs = StringCoding.lookupCharset(charsetName);
>             if (cs == null) {
>                 throw new UnsupportedEncodingException(charsetName);
>             }
>             return cs;
>         } catch (IllegalCharsetNameException ics) {
>             throw new UnsupportedEncodingException(charsetName);
>         }
>     }
> 
> ... and StringCoding.lookupCharset:
> 
>     static Charset lookupCharset(String csn) {
>         if (Charset.isSupported(csn)) {
>             try {
>                 return Charset.forName(csn);
>             } catch (UnsupportedCharsetException x) {
>                 throw new Error(x);
>             }
>         }
>         return null;
>     }
> 
> 
> ...you get this:
> 
>     private static Charset lookupCharset(String charsetName)
>             throws UnsupportedEncodingException {
>         Objects.requireNonNull(charsetName);
>         try {
>             Charset cs;
>             inner: {
>                 if (Charset.isSupported(charsetName)) {
>                     try {
>                         cs = Charset.forName(charsetName);
>                         break inner;
>                     } catch (UnsupportedCharsetException x) {
>                         throw new Error(x);
>                     }
>                 }
>                 cs = null;
>             }
>             if (cs == null) {
>                 throw new UnsupportedEncodingException(charsetName);
>             }
>             return cs;
>         } catch (IllegalCharsetNameException ics) {
>             throw new UnsupportedEncodingException(charsetName);
>         }
>     }
> 
> 
> ...and that can be simplified to this:
> 
>     static Charset lookupCharset(String csn) throws UnsupportedEncodingException {
>         Objects.requireNonNull(csn);
>         try {
>             return Charset.forName(csn);
>         } catch (UnsupportedCharsetException | IllegalCharsetNameException x) {
>             throw new UnsupportedEncodingException(csn);
>         }
>     }
> 
> which has an additional benefit that it only performs one lookup (Charset.forName) instead of two (Charset.isSupported & Charset.forName)...

@plevart: I simplified the lookup logic based on your suggestion. Removed some unreachable paths in and simplified  `StringCoding.encode(String, byte, byte[])` as a result.

Simplifying to one lookup speeds up `decodeCharsetName` cases a notch:
before:
decodeCharsetName          UTF-8  avgt   15  370.337 ± 22.199  ns/op
after:
decodeCharsetName          UTF-8  avgt   15  335.971 ± 15.894  ns/op

-------------

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