From 4921f62c8ab8205fd222439dcd1811760b05daf1 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Wed, 24 Jan 2018 22:01:52 -0800 Subject: [PATCH] HttpResponseStatus object allocation reduction Motivation: Usages of HttpResponseStatus may result in more object allocation then necessary due to not looking for cached objects and the AsciiString parsing method not being used due to CharSequence method being used instead. Modifications: - HttpResponseDecoder should attempt to get the HttpResponseStatus from cache instead of allocating a new object - HttpResponseStatus#parseLine(CharSequence) should check if the type is AsciiString and redirect to the AsciiString parsing method which may not require an additional toString call - HttpResponseStatus#parseLine(AsciiString) can be optimized and doesn't require and may not require object allocation Result: Less allocations when dealing with HttpResponseStatus. --- .../io/netty/buffer/ByteProcessorTest.java | 2 + .../codec/http/HttpResponseDecoder.java | 2 +- .../codec/http/HttpResponseStatus.java | 134 +++++++----------- .../codec/http/HttpResponseStatusTest.java | 91 ++++++++++++ .../codec/http2/HttpConversionUtil.java | 3 +- .../java/io/netty/util/ByteProcessor.java | 26 ++-- .../io/netty/util/ByteProcessorUtils.java | 25 ++++ 7 files changed, 187 insertions(+), 96 deletions(-) create mode 100644 codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseStatusTest.java create mode 100644 common/src/main/java/io/netty/util/ByteProcessorUtils.java diff --git a/buffer/src/test/java/io/netty/buffer/ByteProcessorTest.java b/buffer/src/test/java/io/netty/buffer/ByteProcessorTest.java index d485955b72..b20f00715d 100644 --- a/buffer/src/test/java/io/netty/buffer/ByteProcessorTest.java +++ b/buffer/src/test/java/io/netty/buffer/ByteProcessorTest.java @@ -37,6 +37,7 @@ public class ByteProcessorTest { assertEquals(16, buf.forEachByte(14, length - 14, ByteProcessor.FIND_NON_LF)); assertEquals(19, buf.forEachByte(16, length - 16, ByteProcessor.FIND_NUL)); assertEquals(21, buf.forEachByte(19, length - 19, ByteProcessor.FIND_NON_NUL)); + assertEquals(24, buf.forEachByte(19, length - 19, ByteProcessor.FIND_ASCII_SPACE)); assertEquals(24, buf.forEachByte(21, length - 21, ByteProcessor.FIND_LINEAR_WHITESPACE)); assertEquals(28, buf.forEachByte(24, length - 24, ByteProcessor.FIND_NON_LINEAR_WHITESPACE)); assertEquals(-1, buf.forEachByte(28, length - 28, ByteProcessor.FIND_LINEAR_WHITESPACE)); @@ -51,6 +52,7 @@ public class ByteProcessorTest { final int length = buf.readableBytes(); assertEquals(27, buf.forEachByteDesc(0, length, ByteProcessor.FIND_LINEAR_WHITESPACE)); + assertEquals(25, buf.forEachByteDesc(0, length, ByteProcessor.FIND_ASCII_SPACE)); assertEquals(23, buf.forEachByteDesc(0, 28, ByteProcessor.FIND_NON_LINEAR_WHITESPACE)); assertEquals(20, buf.forEachByteDesc(0, 24, ByteProcessor.FIND_NUL)); assertEquals(18, buf.forEachByteDesc(0, 21, ByteProcessor.FIND_NON_NUL)); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java index c6351c47bf..21491971ef 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseDecoder.java @@ -116,7 +116,7 @@ public class HttpResponseDecoder extends HttpObjectDecoder { protected HttpMessage createMessage(String[] initialLine) { return new DefaultHttpResponse( HttpVersion.valueOf(initialLine[0]), - new HttpResponseStatus(Integer.parseInt(initialLine[1]), initialLine[2]), validateHeaders); + HttpResponseStatus.valueOf(Integer.parseInt(initialLine[1]), initialLine[2]), validateHeaders); } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java index 026866ebcc..b7e2c10d45 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpResponseStatus.java @@ -15,13 +15,15 @@ */ package io.netty.handler.codec.http; -import static io.netty.handler.codec.http.HttpConstants.SP; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufUtil; import io.netty.util.AsciiString; -import io.netty.util.ByteProcessor; import io.netty.util.CharsetUtil; +import static io.netty.handler.codec.http.HttpConstants.SP; +import static io.netty.util.ByteProcessor.FIND_ASCII_SPACE; +import static java.lang.Integer.parseInt; + /** * The response code and its description of HTTP or its derived protocols, such as * RTSP and @@ -324,10 +326,15 @@ public class HttpResponseStatus implements Comparable { /** * Returns the {@link HttpResponseStatus} represented by the specified code. - * If the specified code is a standard HTTP getStatus code, a cached instance + * If the specified code is a standard HTTP status code, a cached instance * will be returned. Otherwise, a new instance will be returned. */ public static HttpResponseStatus valueOf(int code) { + HttpResponseStatus status = valueOf0(code); + return status != null ? status : new HttpResponseStatus(code); + } + + private static HttpResponseStatus valueOf0(int code) { switch (code) { case 100: return CONTINUE; @@ -442,12 +449,25 @@ public class HttpResponseStatus implements Comparable { case 511: return NETWORK_AUTHENTICATION_REQUIRED; } - - return new HttpResponseStatus(code); + return null; } /** - * Parses the specified HTTP status line into a {@link HttpResponseStatus}. The expected formats of the line are: + * Returns the {@link HttpResponseStatus} represented by the specified {@code code} and {@code reasonPhrase}. + * If the specified code is a standard HTTP status {@code code} and {@code reasonPhrase}, a cached instance + * will be returned. Otherwise, a new instance will be returned. + * @param code The response code value. + * @param reasonPhrase The response code reason phrase. + * @return the {@link HttpResponseStatus} represented by the specified {@code code} and {@code reasonPhrase}. + */ + public static HttpResponseStatus valueOf(int code, String reasonPhrase) { + HttpResponseStatus responseStatus = valueOf0(code); + return responseStatus != null && responseStatus.reasonPhrase().contentEquals(reasonPhrase) ? responseStatus : + new HttpResponseStatus(code, reasonPhrase); + } + + /** + * Parses the specified HTTP status line into a {@link HttpResponseStatus}. The expected formats of the line are: *
    *
  • {@code statusCode} (e.g. 200)
    • *
  • {@code statusCode} {@code reasonPhrase} (e.g. 404 Not Found)
    • @@ -456,79 +476,25 @@ public class HttpResponseStatus implements Comparable { * @throws IllegalArgumentException if the specified status line is malformed */ public static HttpResponseStatus parseLine(CharSequence line) { - String status = line.toString(); - try { - int space = status.indexOf(' '); - if (space == -1) { - return valueOf(Integer.parseInt(status)); - } else { - int code = Integer.parseInt(status.substring(0, space)); - String reasonPhrase = status.substring(space + 1); - HttpResponseStatus responseStatus = valueOf(code); - if (responseStatus.reasonPhrase().contentEquals(reasonPhrase)) { - return responseStatus; - } else { - return new HttpResponseStatus(code, reasonPhrase); - } - } - } catch (Exception e) { - throw new IllegalArgumentException("malformed status line: " + status, e); - } + return (line instanceof AsciiString) ? parseLine((AsciiString) line) : parseLine(line.toString()); } - private static final class HttpStatusLineProcessor implements ByteProcessor { - private static final byte ASCII_SPACE = (byte) ' '; - private final AsciiString string; - private int i; - /** - * 0 = New or havn't seen {@link #ASCII_SPACE}. - * 1 = Last byte was {@link #ASCII_SPACE}. - * 2 = Terminal State. Processed the byte after {@link #ASCII_SPACE}, and parsed the status line. - * 3 = Terminal State. There was no byte after {@link #ASCII_SPACE} but status has been parsed with what we saw. - */ - private int state; - private HttpResponseStatus status; - - public HttpStatusLineProcessor(AsciiString string) { - this.string = string; - } - - @Override - public boolean process(byte value) { - switch (state) { - case 0: - if (value == ASCII_SPACE) { - state = 1; - } - break; - case 1: - parseStatus(i); - state = 2; - return false; - default: - break; - } - ++i; - return true; - } - - private void parseStatus(int codeEnd) { - int code = string.parseInt(0, codeEnd); - status = valueOf(code); - if (codeEnd < string.length()) { - String actualReason = string.toString(codeEnd + 1, string.length()); - if (!status.reasonPhrase().contentEquals(actualReason)) { - status = new HttpResponseStatus(code, actualReason); - } - } - } - - public HttpResponseStatus status() { - if (state <= 1) { - parseStatus(string.length()); - state = 3; - } - return status; + /** + * Parses the specified HTTP status line into a {@link HttpResponseStatus}. The expected formats of the line are: + *
      + *
    • {@code statusCode} (e.g. 200)
      • + *
    • {@code statusCode} {@code reasonPhrase} (e.g. 404 Not Found)
      • + *
    + * + * @throws IllegalArgumentException if the specified status line is malformed + */ + public static HttpResponseStatus parseLine(String line) { + try { + int space = line.indexOf(' '); + return space == -1 ? valueOf(parseInt(line)) : + valueOf(parseInt(line.substring(0, space)), line.substring(space + 1)); + } catch (Exception e) { + throw new IllegalArgumentException("malformed status line: " + line, e); } } @@ -543,13 +509,8 @@ public class HttpResponseStatus implements Comparable { */ public static HttpResponseStatus parseLine(AsciiString line) { try { - HttpStatusLineProcessor processor = new HttpStatusLineProcessor(line); - line.forEachByte(processor); - HttpResponseStatus status = processor.status(); - if (status == null) { - throw new IllegalArgumentException("unable to get status after parsing input"); - } - return status; + int space = line.forEachByte(FIND_ASCII_SPACE); + return space == -1 ? valueOf(line.parseInt()) : valueOf(line.parseInt(0, space), line.toString(space + 1)); } catch (Exception e) { throw new IllegalArgumentException("malformed status line: " + line, e); } @@ -598,10 +559,11 @@ public class HttpResponseStatus implements Comparable { } this.code = code; - codeAsText = new AsciiString(Integer.toString(code)); + String codeString = Integer.toString(code); + codeAsText = new AsciiString(codeString); this.reasonPhrase = reasonPhrase; if (bytes) { - this.bytes = (code + " " + reasonPhrase).getBytes(CharsetUtil.US_ASCII); + this.bytes = (codeString + ' ' + reasonPhrase).getBytes(CharsetUtil.US_ASCII); } else { this.bytes = null; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseStatusTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseStatusTest.java new file mode 100644 index 0000000000..2bc83bac8f --- /dev/null +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseStatusTest.java @@ -0,0 +1,91 @@ +/* + * Copyright 2018 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.handler.codec.http; + +import io.netty.util.AsciiString; +import org.junit.Test; + +import static io.netty.handler.codec.http.HttpResponseStatus.parseLine; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +public class HttpResponseStatusTest { + @Test + public void parseLineStringJustCode() { + assertSame(HttpResponseStatus.OK, parseLine("200")); + } + + @Test + public void parseLineStringCodeAndPhrase() { + assertSame(HttpResponseStatus.OK, parseLine("200 OK")); + } + + @Test + public void parseLineStringCustomCode() { + HttpResponseStatus customStatus = parseLine("612"); + assertEquals(612, customStatus.code()); + } + + @Test + public void parseLineStringCustomCodeAndPhrase() { + HttpResponseStatus customStatus = parseLine("612 FOO"); + assertEquals(612, customStatus.code()); + assertEquals("FOO", customStatus.reasonPhrase()); + } + + @Test(expected = IllegalArgumentException.class) + public void parseLineStringMalformedCode() { + parseLine("200a"); + } + + @Test(expected = IllegalArgumentException.class) + public void parseLineStringMalformedCodeWithPhrase() { + parseLine("200a foo"); + } + + @Test + public void parseLineAsciiStringJustCode() { + assertSame(HttpResponseStatus.OK, parseLine(new AsciiString("200"))); + } + + @Test + public void parseLineAsciiStringCodeAndPhrase() { + assertSame(HttpResponseStatus.OK, parseLine(new AsciiString("200 OK"))); + } + + @Test + public void parseLineAsciiStringCustomCode() { + HttpResponseStatus customStatus = parseLine(new AsciiString("612")); + assertEquals(612, customStatus.code()); + } + + @Test + public void parseLineAsciiStringCustomCodeAndPhrase() { + HttpResponseStatus customStatus = parseLine(new AsciiString("612 FOO")); + assertEquals(612, customStatus.code()); + assertEquals("FOO", customStatus.reasonPhrase()); + } + + @Test(expected = IllegalArgumentException.class) + public void parseLineAsciiStringMalformedCode() { + parseLine(new AsciiString("200a")); + } + + @Test(expected = IllegalArgumentException.class) + public void parseLineAsciiStringMalformedCodeWithPhrase() { + parseLine(new AsciiString("200a foo")); + } +} diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java index 66a9e52845..5f808db8df 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java @@ -44,6 +44,7 @@ import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION; import static io.netty.handler.codec.http.HttpHeaderNames.COOKIE; import static io.netty.handler.codec.http.HttpHeaderNames.TE; import static io.netty.handler.codec.http.HttpHeaderValues.TRAILERS; +import static io.netty.handler.codec.http.HttpResponseStatus.parseLine; import static io.netty.handler.codec.http.HttpScheme.HTTP; import static io.netty.handler.codec.http.HttpScheme.HTTPS; import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm; @@ -183,7 +184,7 @@ public final class HttpConversionUtil { public static HttpResponseStatus parseStatus(CharSequence status) throws Http2Exception { HttpResponseStatus result; try { - result = HttpResponseStatus.parseLine(status); + result = parseLine(status); if (result == HttpResponseStatus.SWITCHING_PROTOCOLS) { throw connectionError(PROTOCOL_ERROR, "Invalid HTTP/2 status code '%d'", result.code()); } diff --git a/common/src/main/java/io/netty/util/ByteProcessor.java b/common/src/main/java/io/netty/util/ByteProcessor.java index 1ca1e58d7e..31cd6ff824 100644 --- a/common/src/main/java/io/netty/util/ByteProcessor.java +++ b/common/src/main/java/io/netty/util/ByteProcessor.java @@ -14,6 +14,11 @@ */ package io.netty.util; +import static io.netty.util.ByteProcessorUtils.CARRIAGE_RETURN; +import static io.netty.util.ByteProcessorUtils.HTAB; +import static io.netty.util.ByteProcessorUtils.LINE_FEED; +import static io.netty.util.ByteProcessorUtils.SPACE; + /** * Provides a mechanism to iterate over a collection of bytes. */ @@ -63,22 +68,22 @@ public interface ByteProcessor { /** * Aborts on a {@code CR ('\r')}. */ - ByteProcessor FIND_CR = new IndexOfProcessor((byte) '\r'); + ByteProcessor FIND_CR = new IndexOfProcessor(CARRIAGE_RETURN); /** * Aborts on a non-{@code CR ('\r')}. */ - ByteProcessor FIND_NON_CR = new IndexNotOfProcessor((byte) '\r'); + ByteProcessor FIND_NON_CR = new IndexNotOfProcessor(CARRIAGE_RETURN); /** * Aborts on a {@code LF ('\n')}. */ - ByteProcessor FIND_LF = new IndexOfProcessor((byte) '\n'); + ByteProcessor FIND_LF = new IndexOfProcessor(LINE_FEED); /** * Aborts on a non-{@code LF ('\n')}. */ - ByteProcessor FIND_NON_LF = new IndexNotOfProcessor((byte) '\n'); + ByteProcessor FIND_NON_LF = new IndexNotOfProcessor(LINE_FEED); /** * Aborts on a semicolon {@code (';')}. @@ -90,13 +95,18 @@ public interface ByteProcessor { */ ByteProcessor FIND_COMMA = new IndexOfProcessor((byte) ','); + /** + * Aborts on a ascii space character ({@code ' '}). + */ + ByteProcessor FIND_ASCII_SPACE = new IndexOfProcessor(SPACE); + /** * Aborts on a {@code CR ('\r')} or a {@code LF ('\n')}. */ ByteProcessor FIND_CRLF = new ByteProcessor() { @Override public boolean process(byte value) { - return value != '\r' && value != '\n'; + return value != CARRIAGE_RETURN && value != LINE_FEED; } }; @@ -106,7 +116,7 @@ public interface ByteProcessor { ByteProcessor FIND_NON_CRLF = new ByteProcessor() { @Override public boolean process(byte value) { - return value == '\r' || value == '\n'; + return value == CARRIAGE_RETURN || value == LINE_FEED; } }; @@ -116,7 +126,7 @@ public interface ByteProcessor { ByteProcessor FIND_LINEAR_WHITESPACE = new ByteProcessor() { @Override public boolean process(byte value) { - return value != ' ' && value != '\t'; + return value != SPACE && value != HTAB; } }; @@ -126,7 +136,7 @@ public interface ByteProcessor { ByteProcessor FIND_NON_LINEAR_WHITESPACE = new ByteProcessor() { @Override public boolean process(byte value) { - return value == ' ' || value == '\t'; + return value == SPACE || value == HTAB; } }; diff --git a/common/src/main/java/io/netty/util/ByteProcessorUtils.java b/common/src/main/java/io/netty/util/ByteProcessorUtils.java new file mode 100644 index 0000000000..c642a0ab12 --- /dev/null +++ b/common/src/main/java/io/netty/util/ByteProcessorUtils.java @@ -0,0 +1,25 @@ +/* + * Copyright 2018 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package io.netty.util; + +final class ByteProcessorUtils { + static final byte SPACE = (byte) ' '; + static final byte HTAB = (byte) '\t'; + static final byte CARRIAGE_RETURN = (byte) '\r'; + static final byte LINE_FEED = (byte) '\n'; + + private ByteProcessorUtils() { + } +}