From 82ac20bec032a1e8e6618637bd24f095aa296b02 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 18 Jul 2019 10:20:34 +0200 Subject: [PATCH] Cache the ChannelHandlerContext used in Http2StreamChannelBootstrap (#9382) Motivation: At the moment we lookup the ChannelHandlerContext used in Http2StreamChannelBootstrap each time the open(...) method is invoked. This is not needed and we can just cache it for later usage. Modifications: Cache ChannelHandlerContext in volatile field. Result: Speed up open(...) method implementation when called multiple times --- .../http2/Http2StreamChannelBootstrap.java | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 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 625ceae0a8..2c71ff89c6 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 @@ -46,6 +46,9 @@ public final class Http2StreamChannelBootstrap { private final Channel channel; private volatile ChannelHandler handler; + // Cache the ChannelHandlerContext to speed up open(...) operations. + private volatile ChannelHandlerContext multiplexCtx; + public Http2StreamChannelBootstrap(Channel channel) { this.channel = requireNonNull(channel, "channel"); } @@ -110,18 +113,8 @@ public final class Http2StreamChannelBootstrap { */ @SuppressWarnings("deprecation") public Future open(final Promise promise) { - ChannelHandlerContext ctx = channel.pipeline().context(Http2MultiplexCodec.class); - if (ctx == null) { - ctx = channel.pipeline().context(Http2MultiplexHandler.class); - } - if (ctx == null) { - if (channel.isActive()) { - promise.setFailure(new IllegalStateException(StringUtil.simpleClassName(Http2MultiplexCodec.class) + - " must be in the ChannelPipeline of Channel " + channel)); - } else { - promise.setFailure(new ClosedChannelException()); - } - } else { + try { + ChannelHandlerContext ctx = findCtx(); EventExecutor executor = ctx.executor(); if (executor.inEventLoop()) { open0(ctx, promise); @@ -129,13 +122,39 @@ public final class Http2StreamChannelBootstrap { final ChannelHandlerContext finalCtx = ctx; executor.execute(() -> open0(finalCtx, promise)); } + } catch (Throwable cause) { + promise.setFailure(cause); } return promise; } + private ChannelHandlerContext findCtx() throws ClosedChannelException { + // First try to use cached context and if this not work lets try to lookup the context. + ChannelHandlerContext ctx = this.multiplexCtx; + if (ctx != null && !ctx.isRemoved()) { + return ctx; + } + ChannelPipeline pipeline = channel.pipeline(); + ctx = pipeline.context(Http2MultiplexCodec.class); + if (ctx == null) { + ctx = pipeline.context(Http2MultiplexHandler.class); + } + if (ctx == null) { + if (channel.isActive()) { + throw new IllegalStateException(StringUtil.simpleClassName(Http2MultiplexCodec.class) + " or " + + StringUtil.simpleClassName(Http2MultiplexHandler.class) + + " must be in the ChannelPipeline of Channel " + channel); + } else { + throw new ClosedChannelException(); + } + } + this.multiplexCtx = ctx; + return ctx; + } + /** * @deprecated should not be used directly. Use {@link #open()} or {@link #open(Promise)} - */ + */ @Deprecated public void open0(ChannelHandlerContext ctx, final Promise promise) { assert ctx.executor().inEventLoop(); @@ -172,7 +191,7 @@ public final class Http2StreamChannelBootstrap { } @SuppressWarnings("unchecked") - private void init(Channel channel) throws Exception { + private void init(Channel channel) { ChannelPipeline p = channel.pipeline(); ChannelHandler handler = this.handler; if (handler != null) {