Address the comments from @anuraaga

line · Dec 19, 2019 · 1cb66cb · 1cb66cb
1 parent 5b7dfaf
commit 1cb66cb
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 11 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/common/QueryStringDecoder.java b/core/src/main/java/com/linecorp/armeria/common/QueryStringDecoder.java
@@ -264,8 +264,15 @@ private static int decodeHexByte(char c1, char c2) {
         return (hi << 4) | lo;
     }
 
+    // Do not inline `index` or remove `index < 0` check.
+    // They are specifically written to tell JVM to optimize away the array boundary check.
+    @SuppressWarnings({ "ConstantConditions", "UnnecessaryLocalVariable" })
     private static int decodeHexNibble(char c) {
-        return c < OCTETS_TO_HEX.length ? OCTETS_TO_HEX[c] : -1;
+        final int index = c;
+        if (index < 0 || index > OCTETS_TO_HEX.length) {
+            return -1;
+        }
+        return OCTETS_TO_HEX[index];
     }
 
     private static boolean isContinuation(int b) {

diff --git a/core/src/main/java/com/linecorp/armeria/common/QueryStringEncoder.java b/core/src/main/java/com/linecorp/armeria/common/QueryStringEncoder.java
@@ -80,17 +80,19 @@ static void encodeParams(StringBuilder buf, QueryParamGetters params) {
     }
 
     private static StringBuilder encodeComponent(StringBuilder buf, String s) {
-        final int firstUnsafeOctetIdx = indexOfUnsafeOctet(s, 0);
-        if (firstUnsafeOctetIdx < 0) {
-            buf.append(s);
-        } else {
-            if (firstUnsafeOctetIdx != 0) {
-                buf.append(s, 0, firstUnsafeOctetIdx);
+        final int len = s.length();
+        for (int i = 0; i < len; i++) {
+            final char c = s.charAt(i);
+            if (isUnsafeOctet(c)) {
+                if (i != 0) {
+                    buf.append(s, 0, i);
+                }
+                encodeUtf8Component(buf, s, i);
+                return buf;
             }
-
-            encodeUtf8Component(buf, s, firstUnsafeOctetIdx);
         }
-        return buf;
+
+        return buf.append(s);
     }
 
     /**
@@ -188,13 +190,24 @@ private static int indexOfUnsafeOctet(String s, int start) {
         final int len = s.length();
         for (int i = start; i < len; i++) {
             final char c = s.charAt(i);
-            if (c >= SAFE_OCTETS.length || SAFE_OCTETS[c] == 0) {
+            if (isUnsafeOctet(c)) {
                 return i;
             }
         }
 
         return -1;
     }
 
+    // Do not inline `index` or remove `index < 0` check.
+    // They are specifically written to tell JVM to optimize away the array boundary check.
+    @SuppressWarnings({ "ConstantConditions", "UnnecessaryLocalVariable" })
+    private static boolean isUnsafeOctet(char c) {
+        final int index = c;
+        if (index < 0 || index >= SAFE_OCTETS.length) {
+            return true;
+        }
+        return SAFE_OCTETS[index] == 0;
+    }
+
     private QueryStringEncoder() {}
 }