On Fri, 15 Jan 2021 22:03:31 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Hi Claes, >> This is quite an undertaking in re-factoring! >> I think that decoding logic that you produced can still be kept in >> StringCoding class though. The private constructor that you created for >> UTF-8 decoding is unnecessary. The logic can be kept in a static method and >> then the String instance constructed in it from the value and coder. The >> only public constructor that remains is then the one taking a Charset >> parameter. I took your code and moved it back to StringCoding while for the >> mentioned constructor I tried the following trick which could work (I >> haven't tested yet with JMH): if you pass an instance of newly constructed >> object down to a method that is inlined into the caller and that method does >> not pass that object to any other methods, JIT will eliminate the heap >> allocation, so I suspect that you can pass results from the called method >> back in that object and still avoid allocation... >> >> https://github.com/plevart/jdk/commit/691600e3789a4c2b52fae921be87d0d7affa6f0f >> >> WDYT? > >> WDYT? > > I get that the approach I took got a bit messy, but I've just spent some time > cleaning it up. Please have a look at the latest, which outlines much of the > logic and consolidates the replace/throw logic in the UTF8 decode paths. I've > checked it does not regress on the micro, and I think the overall state of > the code now isn't much messier than the original code. 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? ------------- PR: https://git.openjdk.java.net/jdk/pull/2102
