From 7d971a78a0869171948bb479b3c114d5e772ad45 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 21 Oct 2020 21:09:32 +1100 Subject: [PATCH] Minor performance improvement in websocket upgrade (#10710) Motivation: I noticed WebSocketServerExtensionHandler taking up a non-trivial amount of CPU time for a non-websocket based menchmark. This attempts to speed it up. Modifications: - It is faster to check for a 101 response than to look at headers, so an initial response code check is done - Move all the actual upgrade code into its own method to increase chance of this method being inlined - Add an extra contains() check for the upgrade header, to avoid allocating an iterator if there is no upgrade header Result: A small but noticable performance increase. Signed-off-by: Stuart Douglas --- .../extensions/WebSocketExtensionUtil.java | 5 +- .../WebSocketServerExtensionHandler.java | 78 ++++++++++--------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketExtensionUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketExtensionUtil.java index 86f4731af6..cb4c4cb740 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketExtensionUtil.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketExtensionUtil.java @@ -37,7 +37,10 @@ public final class WebSocketExtensionUtil { private static final Pattern PARAMETER = Pattern.compile("^([^=]+)(=[\\\"]?([^\\\"]+)[\\\"]?)?$"); static boolean isWebsocketUpgrade(HttpHeaders headers) { - return headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true) && + //this contains check does not allocate an iterator, and most requests are not upgrades + //so we do the contains check first before checking for specific values + return headers.contains(HttpHeaderNames.UPGRADE) && + headers.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true) && headers.contains(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET, true); } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java index 6402799de2..f3399e10dc 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/extensions/WebSocketServerExtensionHandler.java @@ -25,6 +25,7 @@ import io.netty.handler.codec.http.HttpHeaderNames; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpRequest; import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; import java.util.ArrayList; import java.util.Arrays; @@ -104,44 +105,51 @@ public class WebSocketServerExtensionHandler implements ChannelHandler { @Override public void write(final ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { if (msg instanceof HttpResponse) { - HttpHeaders headers = ((HttpResponse) msg).headers(); - - if (WebSocketExtensionUtil.isWebsocketUpgrade(headers)) { - - if (validExtensions != null) { - String headerValue = headers.getAsString(HttpHeaderNames.SEC_WEBSOCKET_EXTENSIONS); - - for (WebSocketServerExtension extension : validExtensions) { - WebSocketExtensionData extensionData = extension.newResponseData(); - headerValue = WebSocketExtensionUtil.appendExtension(headerValue, - extensionData.name(), - extensionData.parameters()); - } - promise.addListener((ChannelFutureListener) future -> { - if (future.isSuccess()) { - for (WebSocketServerExtension extension : validExtensions) { - WebSocketExtensionDecoder decoder = extension.newExtensionDecoder(); - WebSocketExtensionEncoder encoder = extension.newExtensionEncoder(); - ctx.pipeline() - .addAfter(ctx.name(), decoder.getClass().getName(), decoder) - .addAfter(ctx.name(), encoder.getClass().getName(), encoder); - } - } - }); - - if (headerValue != null) { - headers.set(HttpHeaderNames.SEC_WEBSOCKET_EXTENSIONS, headerValue); - } - } - - promise.addListener((ChannelFutureListener) future -> { - if (future.isSuccess()) { - ctx.pipeline().remove(WebSocketServerExtensionHandler.this); - } - }); + HttpResponse httpResponse = (HttpResponse) msg; + //checking the status is faster than looking at headers + //so we do this first + if (HttpResponseStatus.SWITCHING_PROTOCOLS.equals(httpResponse.status())) { + handlePotentialUpgrade(ctx, promise, httpResponse); } } ctx.write(msg, promise); } + + private void handlePotentialUpgrade(final ChannelHandlerContext ctx, + ChannelPromise promise, HttpResponse httpResponse) { + HttpHeaders headers = httpResponse.headers(); + if (WebSocketExtensionUtil.isWebsocketUpgrade(headers)) { + if (validExtensions != null) { + String headerValue = headers.getAsString(HttpHeaderNames.SEC_WEBSOCKET_EXTENSIONS); + for (WebSocketServerExtension extension : validExtensions) { + WebSocketExtensionData extensionData = extension.newResponseData(); + headerValue = WebSocketExtensionUtil.appendExtension(headerValue, + extensionData.name(), + extensionData.parameters()); + } + promise.addListener((ChannelFutureListener) future -> { + if (future.isSuccess()) { + for (WebSocketServerExtension extension : validExtensions) { + WebSocketExtensionDecoder decoder = extension.newExtensionDecoder(); + WebSocketExtensionEncoder encoder = extension.newExtensionEncoder(); + String name = ctx.name(); + ctx.pipeline() + .addAfter(name, decoder.getClass().getName(), decoder) + .addAfter(name, encoder.getClass().getName(), encoder); + } + } + }); + + if (headerValue != null) { + headers.set(HttpHeaderNames.SEC_WEBSOCKET_EXTENSIONS, headerValue); + } + } + promise.addListener((ChannelFutureListener) future -> { + if (future.isSuccess()) { + ctx.pipeline().remove(WebSocketServerExtensionHandler.this); + } + }); + } + } }