Use array to buffer decoded query instead of ByteBuffer. (#9886)
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.
This commit is contained in:
parent
7931501769
commit
0f42eb1ceb
@ -16,15 +16,11 @@
|
|||||||
package io.netty.handler.codec.http;
|
package io.netty.handler.codec.http;
|
||||||
|
|
||||||
import io.netty.util.CharsetUtil;
|
import io.netty.util.CharsetUtil;
|
||||||
|
import io.netty.util.internal.PlatformDependent;
|
||||||
|
|
||||||
import java.net.URI;
|
import java.net.URI;
|
||||||
import java.net.URLDecoder;
|
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.Charset;
|
||||||
import java.nio.charset.CharsetDecoder;
|
|
||||||
import java.nio.charset.CoderResult;
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.LinkedHashMap;
|
import java.util.LinkedHashMap;
|
||||||
@ -32,7 +28,9 @@ import java.util.List;
|
|||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
|
||||||
import static io.netty.util.internal.ObjectUtil.checkPositive;
|
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;
|
import static java.util.Objects.requireNonNull;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -352,12 +350,10 @@ public class QueryStringDecoder {
|
|||||||
return s.substring(from, toExcluded);
|
return s.substring(from, toExcluded);
|
||||||
}
|
}
|
||||||
|
|
||||||
CharsetDecoder decoder = CharsetUtil.decoder(charset);
|
|
||||||
|
|
||||||
// Each encoded byte takes 3 characters (e.g. "%20")
|
// Each encoded byte takes 3 characters (e.g. "%20")
|
||||||
int decodedCapacity = (toExcluded - firstEscaped) / 3;
|
int decodedCapacity = (toExcluded - firstEscaped) / 3;
|
||||||
ByteBuffer byteBuf = ByteBuffer.allocate(decodedCapacity);
|
byte[] buf = PlatformDependent.allocateUninitializedArray(decodedCapacity);
|
||||||
CharBuffer charBuf = CharBuffer.allocate(decodedCapacity);
|
int bufIdx;
|
||||||
|
|
||||||
StringBuilder strBuf = new StringBuilder(len);
|
StringBuilder strBuf = new StringBuilder(len);
|
||||||
strBuf.append(s, from, firstEscaped);
|
strBuf.append(s, from, firstEscaped);
|
||||||
@ -369,31 +365,17 @@ public class QueryStringDecoder {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
byteBuf.clear();
|
bufIdx = 0;
|
||||||
do {
|
do {
|
||||||
if (i + 3 > toExcluded) {
|
if (i + 3 > toExcluded) {
|
||||||
throw new IllegalArgumentException("unterminated escape sequence at index " + i + " of: " + s);
|
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;
|
i += 3;
|
||||||
} while (i < toExcluded && s.charAt(i) == '%');
|
} while (i < toExcluded && s.charAt(i) == '%');
|
||||||
i--;
|
i--;
|
||||||
|
|
||||||
byteBuf.flip();
|
strBuf.append(new String(buf, 0, bufIdx, charset));
|
||||||
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());
|
|
||||||
}
|
}
|
||||||
return strBuf.toString();
|
return strBuf.toString();
|
||||||
}
|
}
|
||||||
|
@ -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<String, List<String>> noDecoding() {
|
||||||
|
return new QueryStringDecoder("foo=bar&cat=dog", false).parameters();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Benchmark
|
||||||
|
public Map<String, List<String>> 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<String, List<String>> 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<String, List<String>> 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();
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user