> On 6 Apr 2020, at 11:29, Claes Redestad <claes.redes...@oracle.com> wrote: > > Hi, > > (including net-dev@.. since this is in java.net) > > I intend to sponsor this trivial patch provided by Christoph: > > diff -r 65b345254ca3 > src/java.base/share/classes/java/net/URLStreamHandler.java > --- a/src/java.base/share/classes/java/net/URLStreamHandler.java Thu Apr > 02 05:44:04 2020 +0000 > +++ b/src/java.base/share/classes/java/net/URLStreamHandler.java Mon Apr > 06 12:27:09 2020 +0200 > @@ -266,8 +266,8 @@ > spec.substring(start, limit); > > } else { > - String separator = (authority != null) ? "/" : ""; > - path = separator + spec.substring(start, limit); > + path = spec.substring(start, limit); > + path = (authority != null) ? "/" + path : path; > } > } else if (queryOnly && path != null) { > int ind = path.lastIndexOf('/'); > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8242186 > > Let me know if there are any concerns, otherwise I'll push > once tier1 testing comes back green. Thanks!
Thanks all for this nice micro-optimization. The changes look good to me. -Chris. > /Claes > > On 2020-04-05 19:48, Claes Redestad wrote: >> Hi Christoph, >> looks good and trivial. I'll sponsor. >> /Claes >> On 2020-04-05 10:57, Christoph Dreis wrote: >>> Hi, >>> >>> I just noticed a small optimization opportunity in >>> URLStreamHandler.parseURL for inputs like the following: >>> >>> new URL(new URL("file:"), "file:./path/to/some/local.properties"); >>> >>> In those cases there is no authority involved, which makes it possible to >>> rewrite URLStreamHandler:L269-270: >>> String separator = (authority != null) ? "/" : ""; >>> path = separator + spec.substring(start, limit); >>> >>> into the following to avoid unnecessary string concatenations with an empty >>> string: >>> >>> path = spec.substring(start, limit); >>> path = (authority != null) ? "/" + path : path; >>> >>> I've written a small benchmark with two URLStreamHandler variants (one with >>> the old parseURL and one with the new): >>> >>> @BenchmarkMode(Mode.AverageTime) >>> @OutputTimeUnit(TimeUnit.NANOSECONDS) >>> public class MyBenchmark { >>> >>> @State(Scope.Benchmark) >>> public static class ThreadState { >>> >>> private URL context; >>> private URLStreamHandler newHandler = new NewURLStreamHandler(); >>> private URLStreamHandler oldHandler = new OldURLStreamHandler(); >>> private String spec = "file:./path/to/some/application.properties"; >>> >>> public ThreadState() { >>> try { >>> context = new URL("file:"); >>> } catch (MalformedURLException ignored) { >>> >>> } >>> } >>> } >>> >>> @Benchmark >>> public URL testNew(ThreadState threadState) throws >>> MalformedURLException { >>> return new URL(threadState.context, threadState.spec, >>> threadState.newHandler); >>> } >>> >>> @Benchmark >>> public URL testOld(ThreadState threadState) throws >>> MalformedURLException { >>> return new URL(threadState.context, threadState.spec, >>> threadState.oldHandler); >>> } >>> >>> } >>> >>> The benchmark above yields the following results on my local machine: >>> >>> Benchmark Mode Cnt Score >>> Error Units >>> MyBenchmark.testNew avgt 10 112,655 ± >>> 3,494 ns/op >>> MyBenchmark.testNew:·gc.alloc.rate avgt 10 1299,558 ± >>> 41,238 MB/sec >>> MyBenchmark.testNew:·gc.alloc.rate.norm avgt 10 192,015 ± >>> 0,003 B/op >>> MyBenchmark.testNew:·gc.count avgt 10 98,000 >>> counts >>> MyBenchmark.testNew:·gc.time avgt 10 105,000 >>> ms >>> MyBenchmark.testOld avgt 10 128,592 ± >>> 1,183 ns/op >>> MyBenchmark.testOld:·gc.alloc.rate avgt 10 1612,743 ± >>> 15,792 MB/sec >>> MyBenchmark.testOld:·gc.alloc.rate.norm avgt 10 272,019 ± >>> 0,001 B/op >>> MyBenchmark.testOld:·gc.count avgt 10 110,000 >>> counts >>> MyBenchmark.testOld:·gc.time avgt 10 121,000 >>> ms >>> >>> >>> In case you think this is worthwhile I would need someone to sponsor the >>> attached patch for me. >>> I would highly appreciate that. >>> >>> Cheers, >>> Christoph >>> >>> >>> ===== PATCH ===== >>> diff --git a/src/java.base/share/classes/java/net/URLStreamHandler.java >>> b/src/java.base/share/classes/java/net/URLStreamHandler.java >>> --- a/src/java.base/share/classes/java/net/URLStreamHandler.java >>> +++ b/src/java.base/share/classes/java/net/URLStreamHandler.java >>> @@ -266,8 +266,8 @@ >>> spec.substring(start, limit); >>> >>> } else { >>> - String separator = (authority != null) ? "/" : ""; >>> - path = separator + spec.substring(start, limit); >>> + path = spec.substring(start, limit); >>> + path = (authority != null) ? "/" + path : path; >>> } >>> } else if (queryOnly && path != null) { >>> int ind = path.lastIndexOf('/'); >>> >>>