Frequent allocations / promotions of StringCoding$String{Encoder,Decoder} objects

[Tony Printezis]

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.shipilev at 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
tprintezis at twitter.com