From acb28e3ac8d04feffa15cdceaeda1284b3338b7e Mon Sep 17 00:00:00 2001
From: Norman Maurer <nmaurer@redhat.com>
Date: Tue, 30 Jul 2013 15:46:53 +0200
Subject: [PATCH] Multiple optimizations in the HttpObjectDecoder

* Minimize allocation of StringBuilder and also minimize char array copy
* Try to detect HttpVersion without calling toUpperCase() for performance reasons
---
 .../handler/codec/http/HttpObjectDecoder.java | 59 ++++++++++++-------
 .../netty/handler/codec/http/HttpVersion.java | 36 +++++++++--
 2 files changed, 69 insertions(+), 26 deletions(-)

diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
index 524412dc8a..eb18298256 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -98,10 +98,25 @@ import java.util.List;
  */
 public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecoder.State> {
 
+    private static final ThreadLocal<StringBuilder> BUILDERS = new ThreadLocal<StringBuilder>() {
+        @Override
+        protected StringBuilder initialValue() {
+            return new StringBuilder(512);
+        }
+
+        @Override
+        public StringBuilder get() {
+            StringBuilder builder = super.get();
+            builder.setLength(0);
+            return builder;
+        }
+    };
+
     private final int maxInitialLineLength;
     private final int maxHeaderSize;
     private final int maxChunkSize;
     private final boolean chunkedSupported;
+
     private ByteBuf content;
     private HttpMessage message;
     private long chunkSize;
@@ -315,8 +330,8 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
          * read chunk, read and ignore the CRLF and repeat until 0
          */
         case READ_CHUNK_SIZE: try {
-            String line = readLine(buffer, maxInitialLineLength);
-            int chunkSize = getChunkSize(line);
+            StringBuilder line = readLine(buffer, maxInitialLineLength);
+            int chunkSize = getChunkSize(line.toString());
             this.chunkSize = chunkSize;
             if (chunkSize == 0) {
                 checkpoint(State.READ_CHUNK_FOOTER);
@@ -565,15 +580,15 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         final HttpMessage message = this.message;
         final HttpHeaders headers = message.headers();
 
-        String line = readHeader(buffer);
+        StringBuilder line = readHeader(buffer);
         String name = null;
         String value = null;
-        if (!line.isEmpty()) {
+        if (line.length() > 0) {
             headers.clear();
             do {
                 char firstChar = line.charAt(0);
                 if (name != null && (firstChar == ' ' || firstChar == '\t')) {
-                    value = value + ' ' + line.trim();
+                    value = value + ' ' + line.toString().trim();
                 } else {
                     if (name != null) {
                         headers.add(name, value);
@@ -584,7 +599,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
                 }
 
                 line = readHeader(buffer);
-            } while (!line.isEmpty());
+            } while (line.length() > 0);
 
             // Add the last header.
             if (name != null) {
@@ -609,9 +624,9 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
 
     private LastHttpContent readTrailingHeaders(ByteBuf buffer) {
         headerSize = 0;
-        String line = readHeader(buffer);
+        StringBuilder line = readHeader(buffer);
         String lastHeader = null;
-        if (!line.isEmpty()) {
+        if (line.length() > 0) {
             LastHttpContent trailer = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER);
             do {
                 char firstChar = line.charAt(0);
@@ -619,7 +634,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
                     List<String> current = trailer.trailingHeaders().getAll(lastHeader);
                     if (!current.isEmpty()) {
                         int lastPos = current.size() - 1;
-                        String newString = current.get(lastPos) + line.trim();
+                        String newString = current.get(lastPos) + line.toString().trim();
                         current.set(lastPos, newString);
                     } else {
                         // Content-Length, Transfer-Encoding, or Trailer
@@ -636,7 +651,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
                 }
 
                 line = readHeader(buffer);
-            } while (!line.isEmpty());
+            } while (line.length() > 0);
 
             return trailer;
         }
@@ -644,8 +659,8 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         return LastHttpContent.EMPTY_LAST_CONTENT;
     }
 
-    private String readHeader(ByteBuf buffer) {
-        StringBuilder sb = new StringBuilder(64);
+    private StringBuilder readHeader(ByteBuf buffer) {
+        StringBuilder sb = BUILDERS.get();
         int headerSize = this.headerSize;
 
         loop:
@@ -680,7 +695,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         }
 
         this.headerSize = headerSize;
-        return sb.toString();
+        return sb;
     }
 
     protected abstract boolean isDecodingRequest();
@@ -700,18 +715,18 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         return Integer.parseInt(hex, 16);
     }
 
-    private static String readLine(ByteBuf buffer, int maxLineLength) {
-        StringBuilder sb = new StringBuilder(64);
+    private static StringBuilder readLine(ByteBuf buffer, int maxLineLength) {
+        StringBuilder sb = BUILDERS.get();
         int lineLength = 0;
         while (true) {
             byte nextByte = buffer.readByte();
             if (nextByte == HttpConstants.CR) {
                 nextByte = buffer.readByte();
                 if (nextByte == HttpConstants.LF) {
-                    return sb.toString();
+                    return sb;
                 }
             } else if (nextByte == HttpConstants.LF) {
-                return sb.toString();
+                return sb;
             } else {
                 if (lineLength >= maxLineLength) {
                     // TODO: Respond with Bad Request and discard the traffic
@@ -728,7 +743,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         }
     }
 
-    private static String[] splitInitialLine(String sb) {
+    private static String[] splitInitialLine(StringBuilder sb) {
         int aStart;
         int aEnd;
         int bStart;
@@ -751,7 +766,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
                 cStart < cEnd? sb.substring(cStart, cEnd) : "" };
     }
 
-    private static String[] splitHeader(String sb) {
+    private static String[] splitHeader(StringBuilder sb) {
         final int length = sb.length();
         int nameStart;
         int nameEnd;
@@ -789,7 +804,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         };
     }
 
-    private static int findNonWhitespace(String sb, int offset) {
+    private static int findNonWhitespace(CharSequence sb, int offset) {
         int result;
         for (result = offset; result < sb.length(); result ++) {
             if (!Character.isWhitespace(sb.charAt(result))) {
@@ -799,7 +814,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         return result;
     }
 
-    private static int findWhitespace(String sb, int offset) {
+    private static int findWhitespace(CharSequence sb, int offset) {
         int result;
         for (result = offset; result < sb.length(); result ++) {
             if (Character.isWhitespace(sb.charAt(result))) {
@@ -809,7 +824,7 @@ public abstract class HttpObjectDecoder extends ReplayingDecoder<HttpObjectDecod
         return result;
     }
 
-    private static int findEndOfString(String sb) {
+    private static int findEndOfString(CharSequence sb) {
         int result;
         for (result = sb.length(); result > 0; result --) {
             if (!Character.isWhitespace(sb.charAt(result - 1))) {
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
index 06cfce6526..3ef8281c33 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpVersion.java
@@ -28,6 +28,9 @@ public class HttpVersion implements Comparable<HttpVersion> {
     private static final Pattern VERSION_PATTERN =
         Pattern.compile("(\\S+)/(\\d+)\\.(\\d+)");
 
+    private static final String HTTP_1_0_STRING = "HTTP/1.0";
+    private static final String HTTP_1_1_STRING = "HTTP/1.1";
+
     /**
      * HTTP/1.0
      */
@@ -51,14 +54,39 @@ public class HttpVersion implements Comparable<HttpVersion> {
             throw new NullPointerException("text");
         }
 
-        text = text.trim().toUpperCase();
-        if ("HTTP/1.1".equals(text)) {
+        text = text.trim();
+        // Try to match without convert to uppercase first as this is what 99% of all clients
+        // will send anyway. Also there is a change to the RFC to make it clear that it is
+        // expected to be case-sensitive
+        //
+        // See:
+        // * http://trac.tools.ietf.org/wg/httpbis/trac/ticket/1
+        // * http://trac.tools.ietf.org/wg/httpbis/trac/wiki
+        //
+        // TODO: Remove the uppercase conversion in 4.1.0 as the RFC state it must be HTTP (uppercase)
+        //       See https://github.com/netty/netty/issues/1682
+        //
+        HttpVersion version = version0(text);
+        if (version == null) {
+            text = text.toUpperCase();
+            // try again after convert to uppercase
+            version = version0(text);
+            if (version == null) {
+                // still no match, construct a new one
+                version = new HttpVersion(text, true);
+            }
+        }
+        return version;
+    }
+
+    private static HttpVersion version0(String text) {
+        if (HTTP_1_1_STRING.equals(text)) {
             return HTTP_1_1;
         }
-        if ("HTTP/1.0".equals(text)) {
+        if (HTTP_1_0_STRING.equals(text)) {
             return HTTP_1_0;
         }
-        return new HttpVersion(text, true);
+        return null;
     }
 
     private final String protocolName;