All, Thanks for the replies!
Aleksey, Thanks for pointing me to the CR (JDK-4806753). I’m a bit confused though. The CR was resolved as fixed but Mark’s patch doesn’t seem to have made it to the StringCoding.java file (I checked the JDK 8 and 9 source repos; also the class sources in src.zip in 1.5.0_1, the Fixed In release, and for good measure last 5 and 6 releases). However, I noticed that the cache in java.nio.cs.ThreadLocalCoders looks very similar to Mark's cache. Was it decided to maybe cache the CharsetEncoders/Decoders instead of the StringCoding$StringEncoders/Decoders (i.e., was that the fix for JDK-4806753)? Unfortunately, this is pre-mercurial so I can’t get the history on this. Any volunteers at Oracle? One more observation: the change that introduced the SoftReferences: - /* The cached coders for each thread - */ - private static ThreadLocal decoder = new ThreadLocal(); - private static ThreadLocal encoder = new ThreadLocal(); + /** The cached coders for each thread */ + private final static ThreadLocal<SoftReference<StringDecoder>> decoder = + new ThreadLocal<SoftReference<StringDecoder>>(); + private final static ThreadLocal<SoftReference<StringEncoder>> encoder = + new ThreadLocal<SoftReference<StringEncoder>>(); was done by this changeset: changeset: 18:b5da6145b050 user: martin date: Sun Mar 09 21:56:42 2008 -0700 files: src/share/classes/java/lang/StringCoding.java description: 6671834: (str) Eliminate StringCoding.java compile warnings Reviewed-by: iris Relevant CR: https://bugs.openjdk.java.net/browse/JDK-6671834 There is no explanation of why SoftReferences were introduced. The only comment from Martin is “A fine things to do.” Eliminating compilation warnings is indeed a fine thing to do. Piggy-backing functional changes on that without any explanation is not (FWIW). I personally propose to remove the use of SoftReferences moving forward. Going back to introducing the caches to StringCoding: I can replicate what’s in java.nio.cs.ThreadLocalCoders or I can re-use the ThreadLocalCoders.Cache class. I’d rather do the latter. Any thoughts on what the best way to do this is (move it to a file by itself, make the class public, etc.)? Tony On February 25, 2016 at 4:16:32 PM, Aleksey Shipilev (aleksey.shipi...@oracle.com) wrote: On 02/25/2016 11:48 PM, Tony Printezis wrote: > Has anyone identified this issue before? Hm, there is a blast from the past: https://bugs.openjdk.java.net/browse/JDK-4806753 In comments there, Mark suggests a patch that apparently handles multiple coders. I wonder where did that go. Also, Martin Buchholz is still an assignee there :) > We believe that caching a small number of coders per thread (instead > of just one) could avoid this unnecessary churn. We’ll be happy to > contribute such a fix if there’s interest in getting it accepted. Yes, this seems important. Thanks, -Aleksey ----- Tony Printezis | JVM/GC Engineer / VM Team | Twitter @TonyPrintezis tprinte...@twitter.com
