From dfa5a889789526273504024c2bd9cf9683a095de Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 4 Jul 2014 11:48:23 +0200 Subject: [PATCH] Reuse previous created HttpHeaders by HttpObjectAggregator Motivation: HttpObjectAggregator currently creates a new FullHttpResponse / FullHttpRequest for each message it needs to aggregate. While doing so it also creates 2 DefaultHttpHeader instances (one for the headers and one for the trailing headers). This is bad for two reasons: - More objects are created then needed and also populate the headers is not for free - Headers may get validated even if the validation was disabled in the decoder Modification: - Wrap the previous created HttpResponse / HttpRequest and so reuse the original HttpHeaders - Reuse the previous created trailing HttpHeader. - Fix a bug where the trailing HttpHeader was incorrectly mixed in the headers. Result: - Less GC - Faster HttpObjectAggregator implementation --- .../codec/http/HttpObjectAggregator.java | 236 +++++++++++++++++- .../codec/http/HttpObjectAggregatorTest.java | 2 +- 2 files changed, 225 insertions(+), 13 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java index 73cd089936..cd69394a5b 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java @@ -16,12 +16,14 @@ package io.netty.handler.codec.http; import io.netty.buffer.ByteBuf; +import io.netty.buffer.DefaultByteBufHolder; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelPipeline; +import io.netty.handler.codec.DecoderResult; import io.netty.handler.codec.MessageAggregator; import io.netty.handler.codec.TooLongFrameException; import io.netty.util.internal.logging.InternalLogger; @@ -50,7 +52,6 @@ public class HttpObjectAggregator extends MessageAggregator { private static final InternalLogger logger = InternalLoggerFactory.getInstance(HttpObjectAggregator.class); - private static final FullHttpResponse CONTINUE = new DefaultFullHttpResponse( HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, Unpooled.EMPTY_BUFFER); private static final FullHttpResponse TOO_LARGE = new DefaultFullHttpResponse( @@ -118,21 +119,14 @@ public class HttpObjectAggregator HttpHeaderUtil.setTransferEncodingChunked(start, false); - FullHttpMessage ret; + AggregatedFullHttpMessage ret; if (start instanceof HttpRequest) { - HttpRequest req = (HttpRequest) start; - ret = new DefaultFullHttpRequest(req.protocolVersion(), - req.method(), req.uri(), content); + ret = new AggregatedFullHttpRequest((HttpRequest) start, content, null); } else if (start instanceof HttpResponse) { - HttpResponse res = (HttpResponse) start; - ret = new DefaultFullHttpResponse( - res.protocolVersion(), res.status(), content); + ret = new AggregatedFullHttpResponse((HttpResponse) start, content, null); } else { throw new Error(); } - - ret.headers().set(start.headers()); - ret.setDecoderResult(start.decoderResult()); return ret; } @@ -140,7 +134,7 @@ public class HttpObjectAggregator protected void aggregate(FullHttpMessage aggregated, HttpContent content) throws Exception { if (content instanceof LastHttpContent) { // Merge trailing headers into the message. - aggregated.headers().add(((LastHttpContent) content).trailingHeaders()); + ((AggregatedFullHttpMessage) aggregated).setTrailingHeaders(((LastHttpContent) content).trailingHeaders()); } } @@ -187,4 +181,222 @@ public class HttpObjectAggregator throw new IllegalStateException(); } } + + private abstract static class AggregatedFullHttpMessage extends DefaultByteBufHolder implements FullHttpMessage { + protected final HttpMessage message; + private HttpHeaders trailingHeaders; + + private AggregatedFullHttpMessage(HttpMessage message, ByteBuf content, HttpHeaders trailingHeaders) { + super(content); + this.message = message; + this.trailingHeaders = trailingHeaders; + } + @Override + public HttpHeaders trailingHeaders() { + return trailingHeaders; + } + + public void setTrailingHeaders(HttpHeaders trailingHeaders) { + this.trailingHeaders = trailingHeaders; + } + + @Override + public HttpVersion protocolVersion() { + return message.protocolVersion(); + } + + @Override + public FullHttpMessage setProtocolVersion(HttpVersion version) { + message.setProtocolVersion(version); + return this; + } + + @Override + public HttpHeaders headers() { + return message.headers(); + } + + @Override + public DecoderResult decoderResult() { + return message.decoderResult(); + } + + @Override + public void setDecoderResult(DecoderResult result) { + message.setDecoderResult(result); + } + + @Override + public FullHttpMessage retain(int increment) { + super.retain(increment); + return this; + } + + @Override + public FullHttpMessage retain() { + super.retain(); + return this; + } + + @Override + public FullHttpMessage touch(Object hint) { + super.touch(hint); + return this; + } + + @Override + public FullHttpMessage touch() { + super.touch(); + return this; + } + + @Override + public abstract FullHttpMessage copy(); + + @Override + public abstract FullHttpMessage duplicate(); + } + + private static final class AggregatedFullHttpRequest extends AggregatedFullHttpMessage implements FullHttpRequest { + + private AggregatedFullHttpRequest(HttpRequest request, ByteBuf content, HttpHeaders trailingHeaders) { + super(request, content, trailingHeaders); + } + + @Override + public FullHttpRequest copy() { + DefaultFullHttpRequest copy = new DefaultFullHttpRequest( + protocolVersion(), method(), uri(), content().copy()); + copy.headers().set(headers()); + copy.trailingHeaders().set(trailingHeaders()); + return copy; + } + + @Override + public FullHttpRequest duplicate() { + DefaultFullHttpRequest duplicate = new DefaultFullHttpRequest( + protocolVersion(), method(), uri(), content().duplicate()); + duplicate.headers().set(headers()); + duplicate.trailingHeaders().set(trailingHeaders()); + return duplicate; + } + + @Override + public FullHttpRequest retain(int increment) { + super.retain(increment); + return this; + } + + @Override + public FullHttpRequest retain() { + super.retain(); + return this; + } + + @Override + public FullHttpRequest touch() { + super.touch(); + return this; + } + + @Override + public FullHttpRequest touch(Object hint) { + super.touch(hint); + return this; + } + + @Override + public FullHttpRequest setMethod(HttpMethod method) { + ((HttpRequest) message).setMethod(method); + return this; + } + + @Override + public FullHttpRequest setUri(String uri) { + ((HttpRequest) message).setUri(uri); + return this; + } + + @Override + public HttpMethod method() { + return ((HttpRequest) message).method(); + } + + @Override + public String uri() { + return ((HttpRequest) message).uri(); + } + + @Override + public FullHttpRequest setProtocolVersion(HttpVersion version) { + super.setProtocolVersion(version); + return this; + } + } + + private static final class AggregatedFullHttpResponse extends AggregatedFullHttpMessage + implements FullHttpResponse { + private AggregatedFullHttpResponse(HttpResponse message, ByteBuf content, HttpHeaders trailingHeaders) { + super(message, content, trailingHeaders); + } + + @Override + public FullHttpResponse copy() { + DefaultFullHttpResponse copy = new DefaultFullHttpResponse( + protocolVersion(), status(), content().copy()); + copy.headers().set(headers()); + copy.trailingHeaders().set(trailingHeaders()); + return copy; + } + + @Override + public FullHttpResponse duplicate() { + DefaultFullHttpResponse duplicate = new DefaultFullHttpResponse(protocolVersion(), status(), + content().duplicate()); + duplicate.headers().set(headers()); + duplicate.trailingHeaders().set(trailingHeaders()); + return duplicate; + } + + @Override + public FullHttpResponse setStatus(HttpResponseStatus status) { + ((HttpResponse) message).setStatus(status); + return this; + } + + @Override + public HttpResponseStatus status() { + return ((HttpResponse) message).status(); + } + + @Override + public FullHttpResponse setProtocolVersion(HttpVersion version) { + super.setProtocolVersion(version); + return this; + } + + @Override + public FullHttpResponse retain(int increment) { + super.retain(increment); + return this; + } + + @Override + public FullHttpResponse retain() { + super.retain(); + return this; + } + + @Override + public FullHttpResponse touch(Object hint) { + super.touch(hint); + return this; + } + + @Override + public FullHttpResponse touch() { + super.touch(); + return this; + } + } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java index 7a83bef5b6..32c7a0c0cd 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpObjectAggregatorTest.java @@ -100,7 +100,7 @@ public class HttpObjectAggregatorTest { assertEquals(chunk1.content().readableBytes() + chunk2.content().readableBytes(), HttpHeaderUtil.getContentLength(aggratedMessage)); assertEquals(aggratedMessage.headers().get("X-Test"), Boolean.TRUE.toString()); - assertEquals(aggratedMessage.headers().get("X-Trailer"), Boolean.TRUE.toString()); + assertEquals(aggratedMessage.trailingHeaders().get("X-Trailer"), Boolean.TRUE.toString()); checkContentBuffer(aggratedMessage); assertNull(embedder.readInbound()); }