RFR: 8245309: Re-examine use of ThreadLocalCoders in sun.net.www.ParseUtil

[Aleksey Shipilev]

On Tue, 15 Sep 2020 08:53:35 GMT, Julia Boes <jboes at 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