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
This commit is contained in:
Norman Maurer 2014-03-10 11:23:56 +01:00
parent e57cf9d201
commit adcc2d4b4b
3 changed files with 30 additions and 3 deletions

View File

@ -89,7 +89,7 @@ public abstract class ByteToMessageCodec<I> extends ChannelDuplexHandler {
}
private void checkForSharableAnnotation() {
if (getClass().isAnnotationPresent(Sharable.class)) {
if (isSharable()) {
throw new IllegalStateException("@Sharable annotation is not allowed");
}
}

View File

@ -52,7 +52,7 @@ public abstract class ByteToMessageDecoder extends ChannelInboundHandlerAdapter
private boolean first;
protected ByteToMessageDecoder() {
if (getClass().isAnnotationPresent(Sharable.class)) {
if (isSharable()) {
throw new IllegalStateException("@Sharable annotation is not allowed");
}
}

View File

@ -16,11 +16,31 @@
package io.netty.channel;
import java.util.Map;
import java.util.WeakHashMap;
/**
* Skelton implementation of a {@link ChannelHandler}.
*/
public abstract 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;
@ -29,7 +49,14 @@ public abstract 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;
}
/**