On Tue, 15 Sep 2020 08:53:35 GMT, Julia Boes <jb...@openjdk.org> wrote:
> Replaced the use of ThreadLocalCoders with regular non-caching CharsetEncoder
> and added a benchmark to confirm that
> there is no performance impact.
Drive-by benchmark comments below.
test/micro/org/openjdk/bench/java/net/ThreadLocalParseUtil.java line 65:
> 63:
> 64: @Benchmark
> 65: @CompilerControl(CompilerControl.Mode.DONT_INLINE)
Why `DONT_INLINE`, though? It is useful for looking at disassembly, but it
penalizes the benchmark for the regular
performance runs.
test/micro/org/openjdk/bench/java/net/ThreadLocalParseUtil.java line 71:
> 69:
> 70: @Benchmark
> 71: @CompilerControl(CompilerControl.Mode.DONT_INLINE)
Same comment as above.
test/micro/org/openjdk/bench/java/net/ThreadLocalParseUtil.java line 47:
> 45: @OutputTimeUnit(TimeUnit.NANOSECONDS)
> 46: @State(Scope.Thread)
> 47: @Fork(value = 1, warmups = 0, jvmArgsAppend =
> "--add-exports=java.base/sun.net.www=ALL-UNNAMED")
No need for `warmups = 0`, that's the default already.
test/micro/org/openjdk/bench/java/net/ThreadLocalParseUtil.java line 61:
> 59: Class<?> c = Class.forName("sun.net.www.ParseUtil");
> 60: MH_DECODE = LOOKUP.findStatic(c, "decode",
> methodType(String.class, String.class));
> 61: MH_TO_URI = LOOKUP.findStatic(c, "toURI", methodType(URI.class,
> URL.class));
Not sure why `MethodHandles` are used here. Is this to avoid the compile-time
dependency on these classes? I believe
for better benchmarking `MH_DECODE` and `MH_TO_URI` need to be `static final`
to be folded. This would require
initializing them in `<clinit>`.
test/micro/org/openjdk/bench/java/net/ThreadLocalParseUtil.java line 51:
> 49:
> 50: private static URL url;
> 51: private static MethodHandles.Lookup LOOKUP;
Could be just local variable?
-------------
Changes requested by shade (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/170
