[NEW BUG] Small optimization in URLStreamHandler.parseURL

[Claes Redestad]

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('/');
> 
>