So, I filed a Jira bug:
BUGURL: https://bugs.openjdk.java.net/browse/JDK-8236705
WEBREV: http://cr.openjdk.java.net/~igerasim/8236705/00/webrev/
If we agree on the content of the fix, I can push it after a sanity build.
With kind regards,
Ivan
On 1/6/20 10:41 PM, Ivan Gerasimov wrote:
Hi Christoph!
First, the same optimization can be done in
src/java.base/share/classes/jdk/internal/module/ModulePath.java:
mainClass = mainClass.replace("/", ".");
Second, I wonder what was the JDK version you ran your benchmark on?
After the fix for JDK-8222955 the method replace(CharSequence,
CharSequence) now detects one-char arguments as a special case.
Thus, I think, more work should actually reduce the difference in
performance between two versions of replace.
Still, I think this optimization is worthwhile. I can sponsor it for
you.
With kind regards,
Ivan
On 1/6/20 9:33 AM, Christoph Dreis wrote:
Hi and a happy new year,
I recently noticed that
jdk.internal.module.Resources.toPackageName(String) makes use of
String.replace(CharSequence, CharSequence) while it could use the
single char variant in my opinion:
diff --git
a/src/java.base/share/classes/jdk/internal/module/Resources.java
b/src/java.base/share/classes/jdk/internal/module/Resources.java
--- a/src/java.base/share/classes/jdk/internal/module/Resources.java
+++ b/src/java.base/share/classes/jdk/internal/module/Resources.java
@@ -64,7 +64,7 @@
if (index == -1 || index == name.length()-1) {
return "";
} else {
- return name.substring(0, index).replace("/", ".");
+ return name.substring(0, index).replace('/', '.');
}
}
I ran an isolated benchmark with some test data on it, which shows
the following results
Benchmark (param) Mode Cnt Score Error Units
MyBenchmark.testNew java/lang avgt 10 14,905 ± 0,130 ns/op
MyBenchmark.testNew:·gc.alloc.rate.norm java/lang avgt 10
48,000 ± 0,001 B/op
MyBenchmark.testNew a/b/c/d/e avgt 10 23,122 ± 0,389 ns/op
MyBenchmark.testNew:·gc.alloc.rate.norm a/b/c/d/e avgt 10
96,000 ± 0,001 B/op
MyBenchmark.testOld java/lang avgt 10 16,614 ± 0,420 ns/op
MyBenchmark.testOld:·gc.alloc.rate.norm java/lang avgt
10 48,000 ± 0,001 B/op
MyBenchmark.testOld a/b/c/d/e avgt 10 84,745 ± 1,329 ns/op
MyBenchmark.testOld:·gc.alloc.rate.norm a/b/c/d/e avgt 10
120,000 ± 0,001 B/op
As you can see the more replacing needs to be done the better the
newer version seems to be perform.
In case this is considered worthwhile I would need someone to sponsor
the patch. If not, I'm sorry for the noise.
Cheers,
Christoph Dreis
--
With kind regards,
Ivan Gerasimov
