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
