[GR-57831] StringLatin1.charAt has a substantially supoptimal translation

[nreid260]

[GR-57831] StringLatin1.charAt has a substantially supoptimal translation

[nreid260]

I'm seeing the problem in our binaries produced by native-image.

The substitution is:

@TargetClass(className = "java.lang.StringLatin1")
public final class TargetJavaLangStringLatin1 {

  @Substitute
  public static char charAt(byte[] value, int index) {
    // Delete the explicit bounds checks and rely on the implicit array bounds checks instead.
    return (char) (value[index] & 0xff);
  }

  private TargetJavaLangStringLatin1() {}
}

[davleopo]

@nreid260 thanks for your report - can you share a micro benchmark that outlines the problem? Ideally a pure java one?

[nreid260]

I don't have a micro benchmark, but I do have before vs after profiles of the same program invocation, with the explicit bounds check removed using @substitute. For this program (the Kotlin compiler) ~10% of this execution can be saved.

Before
After

---

We also have a decompilation of String.charAt showing the call to StringLatin1.charAt which isn't inlined:

Disassembly of section .text:

0000000006f377f0 <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548>:
 6f377f0:       55                      push   %rbp
 6f377f1:       48 8b ec                mov    %rsp,%rbp
 6f377f4:       48 83 ec 20             sub    $0x20,%rsp
 6f377f8:       49 3b 67 08             cmp    0x8(%r15),%rsp
 6f377fc:       0f 86 a1 00 00 00       jbe    6f378a3 <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0xb3>
 6f37802:       48 8b 57 08             mov    0x8(%rdi),%rdx
 6f37806:       49 8d 04 16             lea    (%r14,%rdx,1),%rax
 6f3780a:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
 6f37810:       80 7f 14 00             cmpb   $0x0,0x14(%rdi)
 6f37814:       0f 84 59 00 00 00       je     6f37873 <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0x83>
 6f3781a:       48 85 d2                test   %rdx,%rdx
 6f3781d:       0f 84 8a 00 00 00       je     6f378ad <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0xbd>
 6f37823:       48 89 44 24 10          mov    %rax,0x10(%rsp)
 6f37828:       41 8b 7c 16 0c          mov    0xc(%r14,%rdx,1),%edi
 6f3782d:       8b d7                   mov    %edi,%edx
 6f3782f:       d1 ea                   shr    $1,%edx
 6f37831:       48 b9 18 82 0e 03 00    movabs $0x30e8218,%rcx
 6f37838:       00 00 00 
 6f3783b:       49 8d 0c 0e             lea    (%r14,%rcx,1),%rcx
 6f3783f:       8b fe                   mov    %esi,%edi
 6f37841:       89 74 24 1c             mov    %esi,0x1c(%rsp)
 6f37845:       8b f2                   mov    %edx,%esi
 6f37847:       48 8b d1                mov    %rcx,%rdx
 6f3784a:       e8 d1 59 5b 00          call   74ed220 <Preconditions_checkIndex_793460742de58ad655f33d73929c9caa323ff0f5>
 6f3784f:       90                      nop
 6f37850:       8b 74 24 1c             mov    0x1c(%rsp),%esi
 6f37854:       d1 e6                   shl    $1,%esi
 6f37856:       48 63 c6                movslq %esi,%rax
 6f37859:       48 8b 4c 24 10          mov    0x10(%rsp),%rcx
 6f3785e:       0f b7 44 01 10          movzwl 0x10(%rcx,%rax,1),%eax
 6f37863:       41 83 6f 10 01          subl   $0x1,0x10(%r15)
 6f37868:       0f 8e 2d 00 00 00       jle    6f3789b <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0xab>
 6f3786e:       48 8b e5                mov    %rbp,%rsp
 6f37871:       5d                      pop    %rbp
 6f37872:       c3                      ret
 6f37873:       48 8b c8                mov    %rax,%rcx
 6f37876:       89 74 24 1c             mov    %esi,0x1c(%rsp)
 6f3787a:       48 8b f9                mov    %rcx,%rdi
 6f3787d:       e8 ce 7b 01 00          call   6f4f450 <StringLatin1_charAt_63e028c5b786b8663c4a4aea1cc147a8c4714d9d>
 6f37882:       90                      nop
 6f37883:       41 83 6f 10 01          subl   $0x1,0x10(%r15)
 6f37888:       0f 8e 05 00 00 00       jle    6f37893 <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0xa3>
 6f3788e:       48 8b e5                mov    %rbp,%rsp
 6f37891:       5d                      pop    %rbp
 6f37892:       c3                      ret
 6f37893:       e8 68 47 b5 ff          call   6a8c000 <Safepoint_enterSlowPathSafepointCheck_c39040d70cf6aa8104bda7eae589ec9601192610>
 6f37898:       90                      nop
 6f37899:       eb f3                   jmp    6f3788e <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0x9e>
 6f3789b:       e8 60 47 b5 ff          call   6a8c000 <Safepoint_enterSlowPathSafepointCheck_c39040d70cf6aa8104bda7eae589ec9601192610>
 6f378a0:       90                      nop
 6f378a1:       eb cb                   jmp    6f3786e <String_charAt_1b771a25b5f8608a29c775fc6ff4199f0d397548+0x7e>
 6f378a3:       89 74 24 1c             mov    %esi,0x1c(%rsp)
 6f378a7:       e8 94 97 a8 ff          call   69c1040 <StackOverflowCheckImpl_throwNewStackOverflowError_31341960d080a71e3dff8d322e20c16c7dc860eb>
 6f378ac:       90                      nop
 6f378ad:       48 8b c8                mov    %rax,%rcx
 6f378b0:       e8 8b ce b4 ff          call   6a84740 <ImplicitExceptions_throwNewNullPointerException_83e8a13d7d211b9efc4f752fd9d24059e9defb61>
 6f378b5:       90                      nop

[davleopo]

@nreid260 thanks - could you share your substitution ? I was not aware you are seeing these problems in the context of native image.

[davleopo]

@nreid260 thanks - we will have a look.

[davleopo]

@nreid260 I had a look today, thanks for your substitution code - I cannot reproduce what you describe - what version of GraalVM are you running here? Please provide the exact version if possible. The bounds check of String.checkIndex-> Preconditions.checkIndex is dominating the explicit bounds check from the array access. The array access bounds check should fold away and normally does in my experiments. Also, substituting charAt to remove the Preconditions call changes the semantic of this code, even if its a subclass, Id prefer a solution that does not involve changing upstream code here. It might be that we need to tune inlining to understand Precondition special cases better.
Any chance you can share a reproducer with us?