Use single character replace variant in Resources.toPackageName(String)

[Ivan Gerasimov]

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