From 0f42eb1ceb5f6490e99b00254527e1db4a446bf2 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Thu, 19 Dec 2019 05:11:28 +0900 Subject: [PATCH] Use array to buffer decoded query instead of ByteBuffer. (#9886) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: In Java, it is almost always at least slower to use `ByteBuffer` than `byte[]` without pooling or I/O. `QueryStringDecoder` can use `byte[]` with arguably simpler code. Modification: Replace `ByteBuffer` / `CharsetDecoder` with `byte[]` and `new String` Result: After ``` Benchmark Mode Cnt Score Error Units QueryStringDecoderBenchmark.noDecoding thrpt 6 5.612 ± 2.639 ops/us QueryStringDecoderBenchmark.onlyDecoding thrpt 6 1.393 ± 0.067 ops/us QueryStringDecoderBenchmark.mixedDecoding thrpt 6 1.223 ± 0.048 ops/us ``` Before ``` Benchmark Mode Cnt Score Error Units QueryStringDecoderBenchmark.noDecoding thrpt 6 6.123 ± 0.250 ops/us QueryStringDecoderBenchmark.onlyDecoding thrpt 6 0.922 ± 0.159 ops/us QueryStringDecoderBenchmark.mixedDecoding thrpt 6 1.032 ± 0.178 ops/us ``` I notice #6781 switched from an array to `ByteBuffer` but I can't find any motivation for that in the PR. Unit tests pass fine with an array and we get a reasonable speed bump. --- .../codec/http/QueryStringDecoder.java | 36 +++------- .../http/QueryStringDecoderBenchmark.java | 66 +++++++++++++++++++ 2 files changed, 75 insertions(+), 27 deletions(-) create mode 100644 microbench/src/main/java/io/netty/handler/codec/http/QueryStringDecoderBenchmark.java diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java index 845bc1940d..8d5f560f2d 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/QueryStringDecoder.java @@ -16,15 +16,11 @@ package io.netty.handler.codec.http; import io.netty.util.CharsetUtil; +import io.netty.util.internal.PlatformDependent; import java.net.URI; import java.net.URLDecoder; -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; -import java.nio.charset.CharsetDecoder; -import java.nio.charset.CoderResult; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; @@ -32,7 +28,9 @@ import java.util.List; import java.util.Map; import static io.netty.util.internal.ObjectUtil.checkPositive; -import static io.netty.util.internal.StringUtil.*; +import static io.netty.util.internal.StringUtil.EMPTY_STRING; +import static io.netty.util.internal.StringUtil.SPACE; +import static io.netty.util.internal.StringUtil.decodeHexByte; import static java.util.Objects.requireNonNull; /** @@ -352,12 +350,10 @@ public class QueryStringDecoder { return s.substring(from, toExcluded); } - CharsetDecoder decoder = CharsetUtil.decoder(charset); - // Each encoded byte takes 3 characters (e.g. "%20") int decodedCapacity = (toExcluded - firstEscaped) / 3; - ByteBuffer byteBuf = ByteBuffer.allocate(decodedCapacity); - CharBuffer charBuf = CharBuffer.allocate(decodedCapacity); + byte[] buf = PlatformDependent.allocateUninitializedArray(decodedCapacity); + int bufIdx; StringBuilder strBuf = new StringBuilder(len); strBuf.append(s, from, firstEscaped); @@ -369,31 +365,17 @@ public class QueryStringDecoder { continue; } - byteBuf.clear(); + bufIdx = 0; do { if (i + 3 > toExcluded) { throw new IllegalArgumentException("unterminated escape sequence at index " + i + " of: " + s); } - byteBuf.put(decodeHexByte(s, i + 1)); + buf[bufIdx++] = decodeHexByte(s, i + 1); i += 3; } while (i < toExcluded && s.charAt(i) == '%'); i--; - byteBuf.flip(); - charBuf.clear(); - CoderResult result = decoder.reset().decode(byteBuf, charBuf, true); - try { - if (!result.isUnderflow()) { - result.throwException(); - } - result = decoder.flush(charBuf); - if (!result.isUnderflow()) { - result.throwException(); - } - } catch (CharacterCodingException ex) { - throw new IllegalStateException(ex); - } - strBuf.append(charBuf.flip()); + strBuf.append(new String(buf, 0, bufIdx, charset)); } return strBuf.toString(); } diff --git a/microbench/src/main/java/io/netty/handler/codec/http/QueryStringDecoderBenchmark.java b/microbench/src/main/java/io/netty/handler/codec/http/QueryStringDecoderBenchmark.java new file mode 100644 index 0000000000..efd1daee7f --- /dev/null +++ b/microbench/src/main/java/io/netty/handler/codec/http/QueryStringDecoderBenchmark.java @@ -0,0 +1,66 @@ +/* + * Copyright 2019 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.microbench.util.AbstractMicrobenchmark; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +import java.nio.charset.Charset; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +@Threads(1) +@Warmup(iterations = 3) +@Measurement(iterations = 3) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +public class QueryStringDecoderBenchmark extends AbstractMicrobenchmark { + + private static final Charset SHIFT_JIS = Charset.forName("Shift-JIS"); + + @Benchmark + public Map> noDecoding() { + return new QueryStringDecoder("foo=bar&cat=dog", false).parameters(); + } + + @Benchmark + public Map> onlyDecoding() { + // ほげ=ぼけ&ねこ=いぬ + return new QueryStringDecoder("%E3%81%BB%E3%81%92=%E3%81%BC%E3%81%91&%E3%81%AD%E3%81%93=%E3%81%84%E3%81%AC", + false) + .parameters(); + } + + @Benchmark + public Map> mixedDecoding() { + // foo=bar&ほげ=ぼけ&cat=dog&ねこ=いぬ + return new QueryStringDecoder("foo=bar%E3%81%BB%E3%81%92=%E3%81%BC%E3%81%91&cat=dog&" + + "&%E3%81%AD%E3%81%93=%E3%81%84%E3%81%AC", false) + .parameters(); + } + + @Benchmark + public Map> nonStandardDecoding() { + // ほげ=ぼけ&ねこ=いぬ in Shift-JIS + return new QueryStringDecoder("%82%D9%82%B0=%82%DA%82%AF&%82%CB%82%B1=%82%A2%82%CA", + SHIFT_JIS, false) + .parameters(); + } +}