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