From a403da3042335288bf10f58f9d0a16f7585b4557 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 12 Jun 2013 10:59:51 +0200 Subject: [PATCH] Rewrite HTTP encoder to use gathering writes --- .../handler/codec/http/HttpClientCodec.java | 2 +- .../handler/codec/http/HttpObjectEncoder.java | 57 +++++++++++-------- .../WebSocketServerHandshaker00Test.java | 6 +- 3 files changed, 35 insertions(+), 30 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java index 3c82ebd12b..8899c528e0 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java @@ -86,7 +86,7 @@ public final class HttpClientCodec @Override protected void encode( - ChannelHandlerContext ctx, HttpObject msg, ByteBuf out) throws Exception { + ChannelHandlerContext ctx, HttpObject msg, MessageList out) throws Exception { if (msg instanceof HttpRequest && !done) { queue.offer(((HttpRequest) msg).getMethod()); } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java index edcaf7c036..970db26f9e 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java @@ -17,7 +17,8 @@ package io.netty.handler.codec.http; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; -import io.netty.handler.codec.MessageToByteEncoder; +import io.netty.channel.MessageList; +import io.netty.handler.codec.MessageToMessageEncoder; import io.netty.util.CharsetUtil; import java.util.Map; @@ -38,10 +39,12 @@ import static io.netty.handler.codec.http.HttpConstants.*; * To implement the encoder of such a derived protocol, extend this class and * implement all abstract methods properly. */ -public abstract class HttpObjectEncoder extends MessageToByteEncoder { +public abstract class HttpObjectEncoder extends MessageToMessageEncoder { private static final byte[] CRLF = { CR, LF }; private static final byte[] ZERO_CRLF = { '0', CR, LF }; - private static final byte[] HEADER_SEPARATOR = { COLON , SP }; + private static final ByteBuf ZERO_CRLF_CRLF_BUF = + unreleasableBuffer(directBuffer(5, 5).writeBytes(ZERO_CRLF).writeBytes(CRLF)); + private static final byte[] HEADER_SEPARATOR = { COLON, SP}; private static final int ST_INIT = 0; private static final int ST_CONTENT_NON_CHUNK = 1; private static final int ST_CONTENT_CHUNK = 2; @@ -50,7 +53,7 @@ public abstract class HttpObjectEncoder extends MessageTo private int state = ST_INIT; @Override - protected void encode(ChannelHandlerContext ctx, HttpObject msg, ByteBuf out) throws Exception { + protected void encode(ChannelHandlerContext ctx, HttpObject msg, MessageList out) throws Exception { if (msg instanceof HttpMessage) { if (state != ST_INIT) { throw new IllegalStateException("unexpected message type: " + msg.getClass().getSimpleName()); @@ -59,14 +62,14 @@ public abstract class HttpObjectEncoder extends MessageTo @SuppressWarnings({ "unchecked", "CastConflictsWithInstanceof" }) H m = (H) msg; + ByteBuf buf = ctx.alloc().buffer(); // Encode the message. - encodeInitialLine(out, m); - encodeHeaders(out, m); - out.writeBytes(CRLF); - + encodeInitialLine(buf, m); + encodeHeaders(buf, m.headers()); + buf.writeBytes(CRLF); + out.add(buf); state = HttpHeaders.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK; } - if (msg instanceof HttpContent) { if (state == ST_INIT) { throw new IllegalStateException("unexpected message type: " + msg.getClass().getSimpleName()); @@ -78,7 +81,7 @@ public abstract class HttpObjectEncoder extends MessageTo if (state == ST_CONTENT_NON_CHUNK) { if (contentLength > 0) { - out.writeBytes(content, content.readerIndex(), content.readableBytes()); + out.add(content.retain()); } if (chunk instanceof LastHttpContent) { @@ -86,16 +89,26 @@ public abstract class HttpObjectEncoder extends MessageTo } } else if (state == ST_CONTENT_CHUNK) { if (contentLength > 0) { - out.writeBytes(copiedBuffer(Integer.toHexString(contentLength), CharsetUtil.US_ASCII)); - out.writeBytes(CRLF); - out.writeBytes(content, content.readerIndex(), contentLength); - out.writeBytes(CRLF); + byte[] length = Integer.toHexString(contentLength).getBytes(CharsetUtil.US_ASCII); + ByteBuf buf = ctx.alloc().buffer(length.length + 2); + buf.writeBytes(length); + buf.writeBytes(CRLF); + out.add(buf); + out.add(content.retain()); + out.add(wrappedBuffer(CRLF)); } if (chunk instanceof LastHttpContent) { - out.writeBytes(ZERO_CRLF); - encodeTrailingHeaders(out, (LastHttpContent) chunk); - out.writeBytes(CRLF); + HttpHeaders headers = ((LastHttpContent) chunk).trailingHeaders(); + if (headers.isEmpty()) { + out.add(ZERO_CRLF_CRLF_BUF.duplicate()); + } else { + ByteBuf buf = ctx.alloc().buffer(); + buf.writeBytes(ZERO_CRLF); + encodeHeaders(buf, headers); + buf.writeBytes(CRLF); + out.add(buf); + } state = ST_INIT; } @@ -105,14 +118,8 @@ public abstract class HttpObjectEncoder extends MessageTo } } - private static void encodeHeaders(ByteBuf buf, HttpMessage message) { - for (Map.Entry h: message.headers()) { - encodeHeader(buf, h.getKey(), h.getValue()); - } - } - - private static void encodeTrailingHeaders(ByteBuf buf, LastHttpContent trailer) { - for (Map.Entry h: trailer.trailingHeaders()) { + private static void encodeHeaders(ByteBuf buf, HttpHeaders headers) { + for (Map.Entry h: headers) { encodeHeader(buf, h.getKey(), h.getValue()); } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java index aa91646b92..492e5cc877 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java @@ -15,7 +15,6 @@ */ package io.netty.handler.codec.http.websocketx; -import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.http.DefaultFullHttpRequest; @@ -56,10 +55,9 @@ public class WebSocketServerHandshaker00Test { new WebSocketServerHandshaker00( "ws://example.com/chat", "chat", Integer.MAX_VALUE).handshake(ch, req); - ByteBuf resBuf = (ByteBuf) ch.readOutbound(); - EmbeddedChannel ch2 = new EmbeddedChannel(new HttpResponseDecoder()); - ch2.writeInbound(resBuf); + ch2.writeInbound(ch.readOutbound()); + ch2.writeInbound(ch.readOutbound()); HttpResponse res = (HttpResponse) ch2.readInbound(); Assert.assertEquals("ws://example.com/chat", res.headers().get(Names.SEC_WEBSOCKET_LOCATION));