From a402b80ad0cca9501ec167b3932274691d7ef205 Mon Sep 17 00:00:00 2001
From: Norman Maurer <nmaurer@redhat.com>
Date: Mon, 10 Mar 2014 11:23:56 +0100
Subject: [PATCH] Remove condition in ChannelHandlerAdapter.isSharable() by
 caching the result of the annotation lookup.

Motivation:
Remove the synchronization bottleneck and so speed up things

Modifications:
Introduce a ThreadLocal cache that holds mappings between classes of ChannelHandlerAdapater implementations and the result of checking if the @Sharable annotation is present.
This way we only will need to do the real check one time and server the other calls via the cache. A ThreadLocal and WeakHashMap combo is used to implement the cache
as this way we can minimize the conditions while still be sure we not leak class instances in containers.

Result:
Less conditions during adding ChannelHandlerAdapter to the ChannelPipeline
---
 .../handler/codec/ByteToMessageCodec.java     |  2 +-
 .../handler/codec/ByteToMessageDecoder.java   |  2 +-
 .../netty/channel/ChannelHandlerAdapter.java  | 28 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java
index 6392b16960..80de544472 100644
--- a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java
+++ b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java
@@ -89,7 +89,7 @@ public abstract class ByteToMessageCodec<I> extends ChannelHandlerAdapter {
     }
 
     private void checkForSharableAnnotation() {
-        if (getClass().isAnnotationPresent(Sharable.class)) {
+        if (isSharable()) {
             throw new IllegalStateException("@Sharable annotation is not allowed");
         }
     }
diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java b/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java
index 637f4cd9d0..ff32c4b566 100644
--- a/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java
+++ b/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java
@@ -53,7 +53,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter {
     private boolean first;
 
     protected ByteToMessageDecoder() {
-        if (getClass().isAnnotationPresent(Sharable.class)) {
+        if (isSharable()) {
             throw new IllegalStateException("@Sharable annotation is not allowed");
         }
     }
diff --git a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java
index 8b70bc1ba6..ff1bbc6cc4 100644
--- a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java
+++ b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java
@@ -17,12 +17,31 @@
 package io.netty.channel;
 
 import java.net.SocketAddress;
+import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * Skelton implementation of a {@link ChannelHandler}.
  */
 public class ChannelHandlerAdapter implements ChannelHandler {
 
+    /**
+     * Cache the result of {@link Sharable} annotation detection to workaround a condition. We use a
+     * {@link ThreadLocal} and {@link WeakHashMap} to eliminate the volatile write/reads. Using different
+     * {@link WeakHashMap} instances per {@link Thread} is good enough for us and the number of
+     * {@link Thread}s are quite limited anyway.
+     *
+     * See <a href="See https://github.com/netty/netty/issues/2289">#2289</a>.
+     */
+    private static final ThreadLocal<Map<Class<?>, Boolean>> SHARABLE_CACHE =
+            new ThreadLocal<Map<Class<?>, Boolean>>() {
+                @Override
+                protected Map<Class<?>, Boolean> initialValue() {
+                    // Start with small capacity to keep memory overhead as low as possible.
+                    return new WeakHashMap<Class<?>, Boolean>(4);
+                }
+            };
+
     // Not using volatile because it's used only for a sanity check.
     boolean added;
 
@@ -31,7 +50,14 @@ public class ChannelHandlerAdapter implements ChannelHandler {
      * to different {@link ChannelPipeline}s.
      */
     public boolean isSharable() {
-        return getClass().isAnnotationPresent(Sharable.class);
+        Class<?> clazz = getClass();
+        Map<Class<?>, Boolean> cache = SHARABLE_CACHE.get();
+        Boolean sharable = cache.get(clazz);
+        if (sharable == null) {
+            sharable = clazz.isAnnotationPresent(Sharable.class);
+            cache.put(clazz, sharable);
+        }
+        return sharable;
     }
 
     /**