> On 6 Apr 2020, at 11:29, Claes Redestad <[email protected]> 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('/');
>>>
>>>
