Simplify InternalThreadLocalMap (netty#10872)

Motivation: I did not see any tangible advantage to the padding. The only other field that was guarded was a rarely changed object reference to a BitSet. Without the padding, there is also no longer any use of the inheritance hierarchy. The padding was also using `long`, which would not necessarily prevent the JVM from fitting the aforementioned object reference in an alignment gap. Modification: Move all the fields into the InternalThreadLocalMap and deprecate the stuff we'd like to remove. Result: Simpler code. This resolves the discussion in netty#9284
raidyue · Jul 8, 2022 · ef54081 · ef54081
1 parent e1a3063
commit ef54081
Showing 2 changed files with 30 additions and 46 deletions.
diff --git a/common/src/main/java/io/netty/util/internal/InternalThreadLocalMap.java b/common/src/main/java/io/netty/util/internal/InternalThreadLocalMap.java
@@ -30,6 +30,7 @@
 import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.WeakHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * The internal data structure that stores the thread-local variables for Netty and all {@link FastThreadLocal}s.
@@ -39,6 +40,9 @@
 public final class InternalThreadLocalMap extends UnpaddedInternalThreadLocalMap {
 
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(InternalThreadLocalMap.class);
+    private static final ThreadLocal<InternalThreadLocalMap> slowThreadLocalMap =
+            new ThreadLocal<InternalThreadLocalMap>();
+    private static final AtomicInteger nextIndex = new AtomicInteger();
 
     private static final int DEFAULT_ARRAY_LIST_INITIAL_CAPACITY = 8;
     private static final int STRING_BUILDER_INITIAL_SIZE;
@@ -48,8 +52,31 @@ public final class InternalThreadLocalMap extends UnpaddedInternalThreadLocalMap
 
     public static final Object UNSET = new Object();
 
+    /** Used by {@link FastThreadLocal} */
+    private Object[] indexedVariables;
+
+    // Core thread-locals
+    private int futureListenerStackDepth;
+    private int localChannelReaderStackDepth;
+    private Map<Class<?>, Boolean> handlerSharableCache;
+    private IntegerHolder counterHashCode;
+    private ThreadLocalRandom random;
+    private Map<Class<?>, TypeParameterMatcher> typeParameterMatcherGetCache;
+    private Map<Class<?>, Map<String, TypeParameterMatcher>> typeParameterMatcherFindCache;
+
+    // String-related thread-locals
+    private StringBuilder stringBuilder;
+    private Map<Charset, CharsetEncoder> charsetEncoderCache;
+    private Map<Charset, CharsetDecoder> charsetDecoderCache;
+
+    // ArrayList-related thread-locals
+    private ArrayList<Object> arrayList;
+
     private BitSet cleanerFlags;
 
+    /** @deprecated These padding fields will be removed in the future. */
+    public long rp1, rp2, rp3, rp4, rp5, rp6, rp7, rp8, rp9;
+
     static {
         STRING_BUILDER_INITIAL_SIZE =
                 SystemPropertyUtil.getInt("io.netty.threadLocalMap.stringBuilder.initialSize", 1024);
@@ -85,7 +112,6 @@ private static InternalThreadLocalMap fastGet(FastThreadLocalThread thread) {
     }
 
     private static InternalThreadLocalMap slowGet() {
-        ThreadLocal<InternalThreadLocalMap> slowThreadLocalMap = UnpaddedInternalThreadLocalMap.slowThreadLocalMap;
         InternalThreadLocalMap ret = slowThreadLocalMap.get();
         if (ret == null) {
             ret = new InternalThreadLocalMap();
@@ -120,12 +146,8 @@ public static int lastVariableIndex() {
         return nextIndex.get() - 1;
     }
 
-    // Cache line padding (must be public)
-    // With CompressedOops enabled, an instance of this class should occupy at least 128 bytes.
-    public long rp1, rp2, rp3, rp4, rp5, rp6, rp7, rp8, rp9;
-
     private InternalThreadLocalMap() {
-        super(newIndexedVariableTable());
+        indexedVariables = newIndexedVariableTable();
     }
 
     private static Object[] newIndexedVariableTable() {

diff --git a/common/src/main/java/io/netty/util/internal/UnpaddedInternalThreadLocalMap.java b/common/src/main/java/io/netty/util/internal/UnpaddedInternalThreadLocalMap.java
@@ -13,49 +13,11 @@
  * License for the specific language governing permissions and limitations
  * under the License.
  */
-
 package io.netty.util.internal;
 
-import io.netty.util.concurrent.FastThreadLocal;
-
-import java.nio.charset.Charset;
-import java.nio.charset.CharsetDecoder;
-import java.nio.charset.CharsetEncoder;
-import java.util.ArrayList;
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicInteger;
-
 /**
- * The internal data structure that stores the thread-local variables for Netty and all {@link FastThreadLocal}s.
- * Note that this class is for internal use only and is subject to change at any time.  Use {@link FastThreadLocal}
- * unless you know what you are doing.
+ * @deprecated This class will be removed in the future.
  */
 class UnpaddedInternalThreadLocalMap {
-
-    static final ThreadLocal<InternalThreadLocalMap> slowThreadLocalMap = new ThreadLocal<InternalThreadLocalMap>();
-    static final AtomicInteger nextIndex = new AtomicInteger();
-
-    /** Used by {@link FastThreadLocal} */
-    Object[] indexedVariables;
-
-    // Core thread-locals
-    int futureListenerStackDepth;
-    int localChannelReaderStackDepth;
-    Map<Class<?>, Boolean> handlerSharableCache;
-    IntegerHolder counterHashCode;
-    ThreadLocalRandom random;
-    Map<Class<?>, TypeParameterMatcher> typeParameterMatcherGetCache;
-    Map<Class<?>, Map<String, TypeParameterMatcher>> typeParameterMatcherFindCache;
-
-    // String-related thread-locals
-    StringBuilder stringBuilder;
-    Map<Charset, CharsetEncoder> charsetEncoderCache;
-    Map<Charset, CharsetDecoder> charsetDecoderCache;
-
-    // ArrayList-related thread-locals
-    ArrayList<Object> arrayList;
-
-    UnpaddedInternalThreadLocalMap(Object[] indexedVariables) {
-        this.indexedVariables = indexedVariables;
-    }
+    // We cannot remove this in 4.1 because it could break compatibility.
 }