From 254732b43cf42eb2731a22a07545a489b720e4a1 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 6 Dec 2019 10:59:55 +0100 Subject: [PATCH] Replace synchronized with ConcurrentHashMap in Http2StreamChannelBootstrap (#9848) Motivation: 97361fa2c89da57e88762aaca9e2b186e8c148f5 replace synchronized with ConcurrentHashMap in *Bootstrap classes but missed to do the same for the Http2 variant. Modifications: - Use ConcurrentHashMap - Simplify code in *Bootstrap classes Result: Less contention --- .../http2/Http2StreamChannelBootstrap.java | 56 +++++++++---------- .../io/netty/bootstrap/AbstractBootstrap.java | 14 ++--- .../java/io/netty/bootstrap/Bootstrap.java | 4 +- .../io/netty/bootstrap/ServerBootstrap.java | 8 +-- 4 files changed, 37 insertions(+), 45 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java index 0147ced137..8679fc6719 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2StreamChannelBootstrap.java @@ -34,15 +34,19 @@ import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; import java.nio.channels.ClosedChannelException; -import java.util.LinkedHashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; @UnstableApi public final class Http2StreamChannelBootstrap { private static final InternalLogger logger = InternalLoggerFactory.getInstance(Http2StreamChannelBootstrap.class); + @SuppressWarnings("unchecked") + private static final Map.Entry, Object>[] EMPTY_OPTION_ARRAY = new Map.Entry[0]; + @SuppressWarnings("unchecked") + private static final Map.Entry, Object>[] EMPTY_ATTRIBUTE_ARRAY = new Map.Entry[0]; - private final Map, Object> options = new LinkedHashMap<>(); - private final Map, Object> attrs = new LinkedHashMap<>(); + private final Map, Object> options = new ConcurrentHashMap<>(); + private final Map, Object> attrs = new ConcurrentHashMap<>(); private final Channel channel; private volatile ChannelHandler handler; @@ -61,13 +65,9 @@ public final class Http2StreamChannelBootstrap { public Http2StreamChannelBootstrap option(ChannelOption option, T value) { requireNonNull(option, "option"); if (value == null) { - synchronized (options) { - options.remove(option); - } + options.remove(option); } else { - synchronized (options) { - options.put(option, value); - } + options.put(option, value); } return this; } @@ -80,13 +80,9 @@ public final class Http2StreamChannelBootstrap { public Http2StreamChannelBootstrap attr(AttributeKey key, T value) { requireNonNull(key, "key"); if (value == null) { - synchronized (attrs) { - attrs.remove(key); - } + attrs.remove(key); } else { - synchronized (attrs) { - attrs.put(key, value); - } + attrs.put(key, value); } return this; } @@ -193,36 +189,29 @@ public final class Http2StreamChannelBootstrap { }); } - @SuppressWarnings("unchecked") private void init(Channel channel) { ChannelPipeline p = channel.pipeline(); ChannelHandler handler = this.handler; if (handler != null) { p.addLast(handler); } - synchronized (options) { - setChannelOptions(channel, options); - } - - synchronized (attrs) { - for (Map.Entry, Object> e: attrs.entrySet()) { - channel.attr((AttributeKey) e.getKey()).set(e.getValue()); - } - } + setChannelOptions(channel, options.entrySet().toArray(EMPTY_OPTION_ARRAY)); + setAttributes(channel, attrs.entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY)); } private static void setChannelOptions( - Channel channel, Map, Object> options) { - for (Map.Entry, Object> e: options.entrySet()) { + Channel channel, Map.Entry, Object>[] options) { + for (Map.Entry, Object> e: options) { setChannelOption(channel, e.getKey(), e.getValue()); } } - @SuppressWarnings("unchecked") private static void setChannelOption( Channel channel, ChannelOption option, Object value) { try { - if (!channel.config().setOption((ChannelOption) option, value)) { + @SuppressWarnings("unchecked") + ChannelOption opt = (ChannelOption) option; + if (!channel.config().setOption(opt, value)) { logger.warn("Unknown channel option '{}' for channel '{}'", option, channel); } } catch (Throwable t) { @@ -230,4 +219,13 @@ public final class Http2StreamChannelBootstrap { "Failed to set channel option '{}' with value '{}' for channel '{}'", option, value, channel, t); } } + + private static void setAttributes( + Channel channel, Map.Entry, Object>[] options) { + for (Map.Entry, Object> e: options) { + @SuppressWarnings("unchecked") + AttributeKey key = (AttributeKey) e.getKey(); + channel.attr(key).set(e.getValue()); + } + } } diff --git a/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java b/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java index 0273767475..4d33cce168 100644 --- a/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java +++ b/transport/src/main/java/io/netty/bootstrap/AbstractBootstrap.java @@ -48,6 +48,10 @@ import java.util.concurrent.ConcurrentHashMap; */ public abstract class AbstractBootstrap, C extends Channel, F> implements Cloneable { + @SuppressWarnings("unchecked") + static final Map.Entry, Object>[] EMPTY_OPTION_ARRAY = new Map.Entry[0]; + @SuppressWarnings("unchecked") + static final Map.Entry, Object>[] EMPTY_ATTRIBUTE_ARRAY = new Map.Entry[0]; volatile EventLoopGroup group; private volatile SocketAddress localAddress; @@ -353,16 +357,6 @@ public abstract class AbstractBootstrap, C } } - @SuppressWarnings("unchecked") - static Map.Entry, Object>[] newAttrArray(int size) { - return new Map.Entry[size]; - } - - @SuppressWarnings("unchecked") - static Map.Entry, Object>[] newOptionArray(int size) { - return new Map.Entry[size]; - } - @SuppressWarnings("unchecked") private static void setChannelOption( Channel channel, ChannelOption option, Object value, InternalLogger logger) { diff --git a/transport/src/main/java/io/netty/bootstrap/Bootstrap.java b/transport/src/main/java/io/netty/bootstrap/Bootstrap.java index 6cb9184248..f4eea20bbf 100644 --- a/transport/src/main/java/io/netty/bootstrap/Bootstrap.java +++ b/transport/src/main/java/io/netty/bootstrap/Bootstrap.java @@ -277,8 +277,8 @@ public class Bootstrap extends AbstractBootstrap, Object>[] currentChildOptions = - childOptions.entrySet().toArray(newOptionArray(0)); - final Entry, Object>[] currentChildAttrs = childAttrs.entrySet().toArray(newAttrArray(0)); + childOptions.entrySet().toArray(EMPTY_OPTION_ARRAY); + final Entry, Object>[] currentChildAttrs = childAttrs.entrySet().toArray(EMPTY_ATTRIBUTE_ARRAY); p.addLast(new ChannelInitializer() { @Override