Repository: netty/netty
Issue: Investigate performance of Base64

`Base64.encode(...)` / `Base64.decode(...)` is quite slow as it acts on byte-level. We may want to look into optimise this.

See also:
https://github.com/netty/netty/pull/6145#issuecomment-271899281

Issue comments:
thanks for reporting!
This can be closed as these are now merged: 

#6433
#6404

Pull request review comments:
Can't this be simplified to the following:

```java
inBuff = (value & 0xff0000) | (value & 0xff00) | (value & 0xff);
```
```java
inBuff = (value & 0xff00) << 8 | (value & 0xff) << 8;
```
```java
inBuff = (value & 0xff00) << 8 | (value & 0xff0000) >>> 8 | (value & 0xff000000) >>> 24;
```
if this is the case it makes bytes swapping more straightforward below
```java
inBuff = (value & 0xff) << 16 | (value & 0xff00);
```
`dest.order()` -> `src.order()`
`(alphabet[inBuff >>> 18       ])` -> `alphabet[inBuff >>> 18       ]`
general comment remove the unnecessary parenthesis
is `default` (also `0`) an error, or a noop? Can we make this explicit?
we already love bitshifting so why not `int len34 = (len * 3) >>> 2;` 😉 
also nit but you can remove this temporary variable bcz its only used in a single place
nit: move onto previous line?
nit: to avoid an additional variable you can simplify to:

```java
            try {
                src.forEachByte(off, len, this);
                return dest.slice(0, outBuffPosn);
            } catch (Throwable cause) {
                dest.release();
                PlatformDependent.throwException(cause);
                return null;
            }
```
```java
dest.setShort(destOffset, (outBuff & 0xff0000) >>> 8 | (outBuff & 0xff00) >>> 8);
```
```java
dest.setShort(destOffset, (outBuff & 0xff00) | (outBuff & 0xff0000) >>> 16);
```
`final`
nit: `else` is unnecessary 
or may it's possible here `int len34 = len - (len >>> 2);`?
does this buffer need to be released?
we discussed this previously and the rational for not doing this was performance ... is this no longer a concern, or do you think it is unlikely that folks use `LE` buffers? The single mask and single shift per byte suggestions makes the endianness conversion more straightforward and potentially avoids 2 intermediate `int` swap operations.
consider using a consistent approach with the other cases:

```java
rvalue = (src.getByte(srcOffset) & 0xff) << 16;
```
nit: kill extra space after `=`.
you didn't like `int len34 = (len * 3) >>> 2;` with a comment above?

Also update the `int len43` in the `encode` method to be consistent.
`slice()` shares the reference count of the "parent" so the ownership is transferred to the caller of `decode` as it was before.
consider using the following (simpler and reduced casting)

```java
short value = (short) ((outBuff & 0xff0000) >>> 8 | (outBuff & 0xff00) >>> 8);
```
You can save a shift operation and temp variable by doing the following:

```java
dest.setByte(destOffset, (byte) ((src[srcOffset] & 0xff) << 2 | (src[srcOffset + 1] & 0xff) >>> 4));
```
@Scottmitch I think this code is easier to understand and little-endian is not very likely. So after thinking more about it I thought we should just keep it simple for now and improve later if needed. WDYT ?
good point
missed this. Let me change
actually he was correct. I missed to call `dest.release()` in the catch block (already fixed)
+1
Good idea
oh it was there when I looked ... code looked like https://github.com/netty/netty/pull/6404#discussion_r101837914 ... good catch @johnou 
I don't know how likely `LE` is but we can always improve it later if necessary. I may submit a followup PR while all this stuff is fresh on my mind :)
Just let me know if you would prefer to not double swap and I can make it happen. Just thought it is easer to follow if I just not care too much for now. 
I would prefer avoiding the double swap but I agree it isn't necessary for this PR (fine for followup PR, or otherwise delay). Since we are already doing pretty low level byte/bit manipulation here and it has been shown to have a noticeable performance gain I think handling the endianness in `Base64` is reasonable.
this is back :)
we can reduce the operations if we do the byte swap and shifts in one operation:

```java
private static int toIntLE(short valueBE) {
  return (valueBE & 0xff) << 16 | (valueBE & 0xff00);
}
```
```java
private static int toIntLE(int mediumBE) {
  return (mediumBE & 0xff) << 16 | (mediumBE & 0xff00) | (mediumBE & 0xff0000) >>> 16;
}
```
this is back again? (also update encode too).
`len * 4` -> `len << 2` (not `<< 4`)
do we have tests for the encode side?
consider renaming `value` to `mediumValue` or somehow indicate the value only occupies 24 bits.
```java
dest.setShort(destOffset, (short) ((outBuff & 0xff0000) >>> 8 | (outBuff & 0xff00) >>> 8));
```
https://github.com/netty/netty/pull/6404#discussion_r102082437
You can save a shift operation and temp variable by doing the following:

```java
dest.setByte(destOffset, (byte) ((decodabet[src[0]] & 0xff) << 2 | (decodabet[src[1]] & 0xff) >>> 4));
```
https://github.com/netty/netty/pull/6404#discussion_r102086391
you could also get rid of the `outBuff` and just directly put the bytes where they need to be
```java
dest.setShort(destOffset, (short) ((outBuff & 0xff0000) >>> 16 | (outBuff & 0xff00)));
```
we could also just directly compute the desired value of `outBuf` here.
ups... yes in Base64Test. 
Actually I think it must be `(len << 2) / 3`
![CRITICAL](https://raw.githubusercontent.com/SonarCommunity/sonar-github/master/images/severity-critical.png) Bitwise OR of signed byte value computed in io.netty.handler.codec.base64.Base64.encode3to4LittleEndian(int, int, ByteBuf, int, byte[]) [![rule](https://raw.githubusercontent.com/SonarCommunity/sonar-github/master/images/rule.png)](https://garage.netty.io/sonarqube/coding_rules#rule_key=findbugs%3ABIT_IOR_OF_SIGNED_BYTE)

I wonder why our unit tests didn't catch this
because it will just re-size under the hood ?
should we re-catch the `IndexOutOfBoundsException` like above?
also no need to compute`outBuff` above if in this conditional.
nit: `return dest.slice(0, outBuffPosn);`

Patch that resolved the issue:
diff --git a/codec/src/main/java/io/netty/handler/codec/base64/Base64.java b/codec/src/main/java/io/netty/handler/codec/base64/Base64.java
index dc3f1aff801..770940854bd 100644
--- a/codec/src/main/java/io/netty/handler/codec/base64/Base64.java
+++ b/codec/src/main/java/io/netty/handler/codec/base64/Base64.java
@@ -21,6 +21,11 @@
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.util.ByteProcessor;
+import io.netty.util.internal.PlatformDependent;
+
+import java.nio.ByteOrder;
 
 /**
  * Utility class for {@link ByteBuf} that encodes and decodes to and from
@@ -109,7 +114,6 @@ public static ByteBuf encode(
 
     public static ByteBuf encode(
             ByteBuf src, int off, int len, boolean breakLines, Base64Dialect dialect, ByteBufAllocator allocator) {
-
         if (src == null) {
             throw new NullPointerException("src");
         }
@@ -117,17 +121,18 @@ public static ByteBuf encode(
             throw new NullPointerException("dialect");
         }
 
-        int len43 = len * 4 / 3;
+        int len43 = (len << 2) / 3;
         ByteBuf dest = allocator.buffer(
                 len43 +
                         (len % 3 > 0 ? 4 : 0) + // Account for padding
                         (breakLines ? len43 / MAX_LINE_LENGTH : 0)).order(src.order()); // New lines
+        byte[] alphabet = alphabet(dialect);
         int d = 0;
         int e = 0;
         int len2 = len - 2;
         int lineLength = 0;
         for (; d < len2; d += 3, e += 4) {
-            encode3to4(src, d + off, 3, dest, e, dialect);
+            encode3to4(src, d + off, 3, dest, e, alphabet);
 
             lineLength += 4;
 
@@ -139,7 +144,7 @@ public static ByteBuf encode(
         } // end for: each piece of array
 
         if (d < len) {
-            encode3to4(src, d + off, len - d, dest, e, dialect);
+            encode3to4(src, d + off, len - d, dest, e, alphabet);
             e += 4;
         } // end if: some padding needed
 
@@ -152,11 +157,7 @@ public static ByteBuf encode(
     }
 
     private static void encode3to4(
-            ByteBuf src, int srcOffset, int numSigBytes,
-            ByteBuf dest, int destOffset, Base64Dialect dialect) {
-
-        byte[] ALPHABET = alphabet(dialect);
-
+            ByteBuf src, int srcOffset, int numSigBytes, ByteBuf dest, int destOffset, byte[] alphabet) {
         //           1         2         3
         // 01234567890123456789012345678901 Bit position
         // --------000000001111111122222222 Array position from threeBytes
@@ -168,30 +169,110 @@ private static void encode3to4(
         // significant bytes passed in the array.
         // We have to shift left 24 in order to flush out the 1's that appear
         // when Java treats a value as negative that is cast from a byte to an int.
-        int inBuff =
-                (numSigBytes > 0? src.getByte(srcOffset)     << 24 >>>  8 : 0) |
-                (numSigBytes > 1? src.getByte(srcOffset + 1) << 24 >>> 16 : 0) |
-                (numSigBytes > 2? src.getByte(srcOffset + 2) << 24 >>> 24 : 0);
+        if (src.order() == ByteOrder.BIG_ENDIAN) {
+            final int inBuff;
+            switch (numSigBytes) {
+                case 1:
+                    inBuff = toInt(src.getByte(srcOffset));
+                    break;
+                case 2:
+                    inBuff = toIntBE(src.getShort(srcOffset));
+                    break;
+                default:
+                    inBuff = numSigBytes <= 0 ? 0 : toIntBE(src.getMedium(srcOffset));
+                    break;
+            }
+            encode3to4BigEndian(inBuff, numSigBytes, dest, destOffset, alphabet);
+        } else {
+            final int inBuff;
+            switch (numSigBytes) {
+                case 1:
+                    inBuff = toInt(src.getByte(srcOffset));
+                    break;
+                case 2:
+                    inBuff = toIntLE(src.getShort(srcOffset));
+                    break;
+                default:
+                    inBuff = numSigBytes <= 0 ? 0 : toIntLE(src.getMedium(srcOffset));
+                    break;
+            }
+            encode3to4LittleEndian(inBuff, numSigBytes, dest, destOffset, alphabet);
+        }
+    }
+
+    private static int toInt(byte value) {
+        return (value & 0xff) << 16;
+    }
 
+    private static int toIntBE(short value) {
+        return (value & 0xff00) << 8 | (value & 0xff) << 8;
+    }
+
+    private static int toIntLE(short value) {
+        return (value & 0xff) << 16 | (value & 0xff00);
+    }
+
+    private static int toIntBE(int mediumValue) {
+        return (mediumValue & 0xff0000) | (mediumValue & 0xff00) | (mediumValue & 0xff);
+    }
+
+    private static int toIntLE(int mediumValue) {
+        return (mediumValue & 0xff) << 16 | (mediumValue & 0xff00) | (mediumValue & 0xff0000) >>> 16;
+    }
+
+    private static void encode3to4BigEndian(
+            int inBuff, int numSigBytes, ByteBuf dest, int destOffset, byte[] alphabet) {
+        // Packing bytes into an int to reduce bound and reference count checking.
         switch (numSigBytes) {
-        case 3:
-            dest.setByte(destOffset    , ALPHABET[inBuff >>> 18       ]);
-            dest.setByte(destOffset + 1, ALPHABET[inBuff >>> 12 & 0x3f]);
-            dest.setByte(destOffset + 2, ALPHABET[inBuff >>>  6 & 0x3f]);
-            dest.setByte(destOffset + 3, ALPHABET[inBuff        & 0x3f]);
-            break;
-        case 2:
-            dest.setByte(destOffset    , ALPHABET[inBuff >>> 18       ]);
-            dest.setByte(destOffset + 1, ALPHABET[inBuff >>> 12 & 0x3f]);
-            dest.setByte(destOffset + 2, ALPHABET[inBuff >>> 6  & 0x3f]);
-            dest.setByte(destOffset + 3, EQUALS_SIGN);
-            break;
-        case 1:
-            dest.setByte(destOffset    , ALPHABET[inBuff >>> 18       ]);
-            dest.setByte(destOffset + 1, ALPHABET[inBuff >>> 12 & 0x3f]);
-            dest.setByte(destOffset + 2, EQUALS_SIGN);
-            dest.setByte(destOffset + 3, EQUALS_SIGN);
-            break;
+            case 3:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ] << 24 |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 16 |
+                                        alphabet[inBuff >>>  6 & 0x3f] << 8  |
+                                        alphabet[inBuff        & 0x3f]);
+                break;
+            case 2:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ] << 24 |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 16 |
+                                        alphabet[inBuff >>> 6  & 0x3f] << 8  |
+                                        EQUALS_SIGN);
+                break;
+            case 1:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ] << 24 |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 16 |
+                                        EQUALS_SIGN << 8                     |
+                                        EQUALS_SIGN);
+                break;
+            default:
+                // NOOP
+                break;
+        }
+    }
+
+    private static void encode3to4LittleEndian(
+            int inBuff, int numSigBytes, ByteBuf dest, int destOffset, byte[] alphabet) {
+        // Packing bytes into an int to reduce bound and reference count checking.
+        switch (numSigBytes) {
+            case 3:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ]       |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 8  |
+                                        alphabet[inBuff >>>  6 & 0x3f] << 16 |
+                                        alphabet[inBuff        & 0x3f] << 24);
+                break;
+            case 2:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ]       |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 8  |
+                                        alphabet[inBuff >>> 6  & 0x3f] << 16 |
+                                        EQUALS_SIGN << 24);
+                break;
+            case 1:
+                dest.setInt(destOffset, alphabet[inBuff >>> 18       ]      |
+                                        alphabet[inBuff >>> 12 & 0x3f] << 8 |
+                                        EQUALS_SIGN << 16                   |
+                                        EQUALS_SIGN << 24);
+                break;
+            default:
+                // NOOP
+                break;
         }
     }
 
@@ -200,7 +281,6 @@ public static ByteBuf decode(ByteBuf src) {
     }
 
     public static ByteBuf decode(ByteBuf src, Base64Dialect dialect) {
-
         if (src == null) {
             throw new NullPointerException("src");
         }
@@ -222,7 +302,6 @@ public static ByteBuf decode(
 
     public static ByteBuf decode(
             ByteBuf src, int off, int len, Base64Dialect dialect, ByteBufAllocator allocator) {
-
         if (src == null) {
             throw new NullPointerException("src");
         }
@@ -230,85 +309,96 @@ public static ByteBuf decode(
             throw new NullPointerException("dialect");
         }
 
-        byte[] DECODABET = decodabet(dialect);
+        // Using a ByteProcessor to reduce bound and reference count checking.
+        return new Decoder().decode(src, off, len, allocator, dialect);
+    }
+
+    private static final class Decoder implements ByteProcessor {
+        private final byte[] b4 = new byte[4];
+        private int b4Posn;
+        private byte sbiCrop;
+        private byte sbiDecode;
+        private byte[] decodabet;
+        private int outBuffPosn;
+        private ByteBuf dest;
+
+        ByteBuf decode(ByteBuf src, int off, int len, ByteBufAllocator allocator, Base64Dialect dialect) {
+            int len34 = (len * 3) >>> 2;
+            dest = allocator.buffer(len34).order(src.order()); // Upper limit on size of output
 
-        int len34 = len * 3 / 4;
-        ByteBuf dest = allocator.buffer(len34).order(src.order()); // Upper limit on size of output
-        int outBuffPosn = 0;
+            decodabet = decodabet(dialect);
+            try {
+                src.forEachByte(off, len, this);
+                return dest.slice(0, outBuffPosn);
+            } catch (Throwable cause) {
+                dest.release();
+                PlatformDependent.throwException(cause);
+                return null;
+            }
+        }
 
-        byte[] b4 = new byte[4];
-        int b4Posn = 0;
-        int i;
-        byte sbiCrop;
-        byte sbiDecode;
-        for (i = off; i < off + len; i ++) {
-            sbiCrop = (byte) (src.getByte(i) & 0x7f); // Only the low seven bits
-            sbiDecode = DECODABET[sbiCrop];
+        @Override
+        public boolean process(byte value) throws Exception {
+            sbiCrop = (byte) (value & 0x7f); // Only the low seven bits
+            sbiDecode = decodabet[sbiCrop];
 
             if (sbiDecode >= WHITE_SPACE_ENC) { // White space, Equals sign or better
                 if (sbiDecode >= EQUALS_SIGN_ENC) { // Equals sign or better
                     b4[b4Posn ++] = sbiCrop;
                     if (b4Posn > 3) { // Quartet built
-                        outBuffPosn += decode4to3(
-                                b4, 0, dest, outBuffPosn, dialect);
+                        outBuffPosn += decode4to3(b4, dest, outBuffPosn, decodabet);
                         b4Posn = 0;
 
                         // If that was the equals sign, break out of 'for' loop
                         if (sbiCrop == EQUALS_SIGN) {
-                            break;
+                            return false;
                         }
                     }
                 }
-            } else {
-                throw new IllegalArgumentException(
-                        "bad Base64 input character at " + i + ": " +
-                        src.getUnsignedByte(i) + " (decimal)");
+                return true;
             }
+            throw new IllegalArgumentException(
+                    "invalid bad Base64 input character: " + (short) (value & 0xFF) + " (decimal)");
         }
 
-        return dest.slice(0, outBuffPosn);
-    }
+        private static int decode4to3(byte[] src, ByteBuf dest, int destOffset, byte[] decodabet) {
+            if (src[2] == EQUALS_SIGN) {
+                // Example: Dk==
+                dest.setByte(destOffset, (byte) ((decodabet[src[0]] & 0xff) << 2 | (decodabet[src[1]] & 0xff) >>> 4));
+                return 1;
+            }
+
+            if (src[3] == EQUALS_SIGN) {
+                // Example: DkL=
+                int outBuff = (decodabet[src[0]] & 0xff) << 18 |
+                              (decodabet[src[1]] & 0xff) << 12 |
+                              (decodabet[src[2]] & 0xff) << 6;
+
+                // Packing bytes into a short to reduce bound and reference count checking.
+                if (dest.order() == ByteOrder.BIG_ENDIAN) {
+                    dest.setShort(destOffset, (short) ((outBuff & 0xff0000) >>> 8 | (outBuff & 0xff00) >>> 8));
+                } else {
+                    dest.setShort(destOffset, (short) ((outBuff & 0xff0000) >>> 16 | (outBuff & 0xff00)));
+                }
+                return 2;
+            }
 
-    private static int decode4to3(
-            byte[] src, int srcOffset,
-            ByteBuf dest, int destOffset, Base64Dialect dialect) {
-
-        byte[] DECODABET = decodabet(dialect);
-
-        if (src[srcOffset + 2] == EQUALS_SIGN) {
-            // Example: Dk==
-            int outBuff =
-                    (DECODABET[src[srcOffset    ]] & 0xFF) << 18 |
-                    (DECODABET[src[srcOffset + 1]] & 0xFF) << 12;
-
-            dest.setByte(destOffset, (byte) (outBuff >>> 16));
-            return 1;
-        } else if (src[srcOffset + 3] == EQUALS_SIGN) {
-            // Example: DkL=
-            int outBuff =
-                    (DECODABET[src[srcOffset    ]] & 0xFF) << 18 |
-                    (DECODABET[src[srcOffset + 1]] & 0xFF) << 12 |
-                    (DECODABET[src[srcOffset + 2]] & 0xFF) <<  6;
-
-            dest.setByte(destOffset    , (byte) (outBuff >>> 16));
-            dest.setByte(destOffset + 1, (byte) (outBuff >>>  8));
-            return 2;
-        } else {
             // Example: DkLE
-            int outBuff;
+            final int outBuff;
             try {
-                outBuff =
-                        (DECODABET[src[srcOffset    ]] & 0xFF) << 18 |
-                        (DECODABET[src[srcOffset + 1]] & 0xFF) << 12 |
-                        (DECODABET[src[srcOffset + 2]] & 0xFF) <<  6 |
-                         DECODABET[src[srcOffset + 3]] & 0xFF;
+                outBuff = (decodabet[src[0]] & 0xff) << 18 |
+                          (decodabet[src[1]] & 0xff) << 12 |
+                          (decodabet[src[2]] & 0xff) <<  6 |
+                           decodabet[src[3]] & 0xff;
             } catch (IndexOutOfBoundsException ignored) {
                 throw new IllegalArgumentException("not encoded in Base64");
             }
-
-            dest.setByte(destOffset    , (byte) (outBuff >> 16));
-            dest.setByte(destOffset + 1, (byte) (outBuff >>  8));
-            dest.setByte(destOffset + 2, (byte)  outBuff);
+            // Just directly set it as medium
+            if (dest.order() == ByteOrder.BIG_ENDIAN) {
+                dest.setMedium(destOffset, outBuff);
+            } else {
+                dest.setMedium(destOffset,  ByteBufUtil.swapMedium(outBuff));
+            }
             return 3;
         }
     }


Test patch:
diff --git a/codec/src/test/java/io/netty/handler/codec/base64/Base64Test.java b/codec/src/test/java/io/netty/handler/codec/base64/Base64Test.java
index 6b8622ac24a..4be23a8d864 100644
--- a/codec/src/test/java/io/netty/handler/codec/base64/Base64Test.java
+++ b/codec/src/test/java/io/netty/handler/codec/base64/Base64Test.java
@@ -18,10 +18,12 @@
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.util.CharsetUtil;
+import io.netty.util.internal.PlatformDependent;
 import org.junit.Test;
 
 
 import java.io.ByteArrayInputStream;
+import java.nio.ByteOrder;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 
@@ -116,4 +118,42 @@ private static void testEncode(ByteBuf src, ByteBuf expectedEncoded) {
             encoded.release();
         }
     }
+
+    @Test
+    public void testEncodeDecodeBE() {
+        testEncodeDecode(ByteOrder.BIG_ENDIAN);
+    }
+
+    @Test
+    public void testEncodeDecodeLE() {
+        testEncodeDecode(ByteOrder.LITTLE_ENDIAN);
+    }
+
+    private static void testEncodeDecode(ByteOrder order) {
+        testEncodeDecode(64, order);
+        testEncodeDecode(128, order);
+        testEncodeDecode(512, order);
+        testEncodeDecode(1024, order);
+        testEncodeDecode(4096, order);
+        testEncodeDecode(8192, order);
+        testEncodeDecode(16384, order);
+    }
+
+    private static void testEncodeDecode(int size, ByteOrder order) {
+        byte[] bytes = new byte[size];
+        PlatformDependent.threadLocalRandom().nextBytes(bytes);
+
+        ByteBuf src = Unpooled.wrappedBuffer(bytes).order(order);
+        ByteBuf encoded = Base64.encode(src);
+        ByteBuf decoded = Base64.decode(encoded);
+        ByteBuf expectedBuf = Unpooled.wrappedBuffer(bytes);
+        try {
+            assertEquals(expectedBuf, decoded);
+        } finally {
+            src.release();
+            encoded.release();
+            decoded.release();
+            expectedBuf.release();
+        }
+    }
 }