RFR: 8309502: RISC-V: String.indexOf intrinsic may produce misaligned memory loads [v2]

[Vladimir Kempik]

On Wed, 7 Jun 2023 15:56:27 GMT, Vladimir Kempik <vkempik at openjdk.org> wrote:

>> Please review this attempt to remove misaligned loads in String.indexOf intrinsic on RISC-V
>> 
>> Initialy found these misaligned loads when profiling finagle-http test from renaissance suite.
>> The majority of trp_lam events (about 66k per finagle-http round) came at line 706 (https://github.com/openjdk/jdk/pull/14320/files#diff-35eb1d2f1e2f0514dd46bd7fbad49ff2c87703d5a3041a6433956df00a3fe6e6L706)
>> The other two produced about 100 events combined.
>> Later I've found this can partially be reproduced with StringIndexOf.advancedWithMediumSub.
>> Numbers on hifive before and after applying the patch:
>> 
>> 
>> Benchmark                                                  Mode  Cnt       Score      Error  Units
>> StringIndexOf.advancedWithMediumSub                        avgt   25   47031.406 ±  144.005  ns/op
>> 
>> 
>> After:
>> 
>> Benchmark                                                 Mode  Cnt       Score     Error  Units
>> StringIndexOf.advancedWithMediumSub                       avgt   25    4256.830 ±  23.075  ns/op
>> 
>> 
>> Testing: tier1/tier2 is clean on hifive.
>
> Vladimir Kempik has updated the pull request incrementally with one additional commit since the last revision:
> 
>   make DO2 read by one character from memory per loop

I have made a microtest to test specifically DO2 part of string_indexof_linear

diff --git a/test/micro/org/openjdk/bench/java/lang/StringIndexOf.java b/test/micro/org/openjdk/bench/java/lang/StringIndexOf.java
index 57ced6d8e13..33c8d998d8d 100644
--- a/test/micro/org/openjdk/bench/java/lang/StringIndexOf.java
+++ b/test/micro/org/openjdk/bench/java/lang/StringIndexOf.java
@@ -46,6 +46,8 @@ public class StringIndexOf {
     private String shortSub1;
     private String data2;
     private String shortSub2;
+
+    private String shortSub3;
     private String string16Short;
     private String string16Medium;
     private String string16Long;
@@ -64,6 +66,7 @@ public class StringIndexOf {
         shortSub1 = "1";
         data2 = "00001001010100a10110101010010101110101001110110101010010101010010000010111010101010101010a100010010101110111010101101010100010010a100a0010101111111000010101010010101000010101010010101010101110a10010101010101010101010101010";
         shortSub2 = "a";
+        shortSub3 = "a1";
         searchChar = 's';
 
         string16Short = "scar\u01fe1";
@@ -246,6 +249,20 @@ public class StringIndexOf {
         return dummy;
     }
 
+    /**
+     * Benchmarks String.indexOf with a rather big String. Search repeatedly for a matched that is 2 chars but only with
+     * a few matches.
+     */
+    @Benchmark
+    public int advancedWithShortSub3() {
+        int dummy = 0;
+        int index = 0;
+        while ((index = data2.indexOf(shortSub3, index)) > -1) {
+            index++;
+            dummy += index;
+        }
+        return dummy;
+    }
     @Benchmark
     public void constantPattern() {
         String tmp = "simple-hash:SHA-1/UTF-8";


Results, v1 - original patch in this PR, v2 - latest update to DO2

hifive
Benchmark                            Mode  Cnt     Score    Error  Units
Before
StringIndexOf.advancedWithShortSub3  avgt   25  37302.933 ± 80.306  ns/op
V1
StringIndexOf.advancedWithShortSub3  avgt   25  1362.159 ± 37.021  ns/op
V2
StringIndexOf.advancedWithShortSub3  avgt   25  1248.750 ± 40.432  ns/op


thead
Benchmark                            Mode  Cnt     Score    Error  Units
Before
StringIndexOf.advancedWithShortSub3  avgt   25  632.976 ? 42.601  ns/op
V1
StringIndexOf.advancedWithShortSub3  avgt   25  916.040 ? 45.086  ns/op
V2
StringIndexOf.advancedWithShortSub3  avgt   25  919.363 ? 21.977  ns/op


while hifive benefits the update, thead doesn't care and like misaligned way the most

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14320#issuecomment-1581203623