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

Peter Levart plevart at openjdk.java.net
Sun Jan 17 09:06:49 UTC 2021


On Sat, 16 Jan 2021 01:02:20 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Some common logic is now extracted into methods. That's good. But much of the decoding logic still remains in String. I still think all of static methods can be moved to StringCoding directly as they are now while the private UTF-8 decoding constructor can be replaced with static method and moved to StringCoding. The only problem might be the public String constructor taking Charset parameter. But even here, passing a newly constructed mutable object to the method can be used to return multiple values from the method while relying on JIT to eliminate object allocation.
>> Do you think this code belongs more to String than to StringCoding?
>
>> Do you think this code belongs more to String than to StringCoding?
> 
> I consider StringCoding an implementation detail of String, so I'm not sure there's (much) value in maintaining the separation of concern if it is cause for a performance loss. While encapsulating and separating concerns is a fine purpose I think the main purpose served by StringCoding is to resolve some bootstrap issues: String is one of the first classes to be loaded and eagerly pulling in dependencies like ThreadLocal and Charsets before bootstrapping leads to all manner of hard to decipher issues (yes - I've tried :-)).

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.

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

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


More information about the core-libs-dev mailing list