JDK-8332840 : C2: Scalar replacement does not work for multi-argument Objects.hash
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 17,21,22,23
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2024-05-23
  • Updated: 2024-05-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
Description
We can clearly see this in a simple benchmark like this:

```
@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(value = 3)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Thread)
public class HashVarArgsBench {
    private Object a = new Object();
    private Object b = new Object();

    @Benchmark
    public int single() {
        return Objects.hash(a);
    }

    @Benchmark
    public int pair() {
        return Objects.hash(a, b);
    }
}
```

```
Benchmark                                   Mode  Cnt     Score    Error   Units
HashVarArgsBench.pair:gc.alloc.rate.norm    avgt   15    24,000 ±  0,001    B/op
HashVarArgsBench.single:gc.alloc.rate.norm  avgt   15    ≈ 10⁻⁵             B/op
```

I think this happens because the downstream `Arrays.hashCode` effectively does a loop, which confuses scalar replacement due to:

```
    // 4. An object is not scalar replaceable if it has a field with unknown
    // offset (array's element is accessed in loop).
    if (offset == Type::OffsetBot) {
      set_not_scalar_replaceable(jobj NOT_PRODUCT(COMMA "has field with unknown offset"));
      return;
    }
```

Relevant EA/SR trace:

```
JavaObject(5) NoEscape(NoEscape) is NSR. has field with unknown offset

JavaObject(5) NoEscape(NoEscape) NSR [ 878F 881F 264F 265F [ 121 126 ]]   109  AllocateArray  === 60 6 85 8 1 (99 88 51 87 71 10 49 1 1 1 15 1 1 67 29 1 29 1 15 67 ) [[ 110 111 112 119 120 121 ]]  rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, int, bool ) org.openjdk.jmh.samples.HashVarArgsBench::pair @ bci:1 (line 59) org.openjdk.jmh.samples.jmh_generated.HashVarArgsBench_pair_jmhTest::pair_avgt_jmhStub @ bci:17 (line 190)  Type:{0:control, 1:abIO, 2:memory, 3:rawptr:BotPTR, 4:return_address, 5:rawptr:NotNull} !jvms: org.openjdk.jmh.samples.HashVarArgsBench::pair @ bci:1 (line 59) org.openjdk.jmh.samples.jmh_generated.HashVarArgsBench_pair_jmhTest::pair_avgt_jmhStub @ bci:17 (line 190)
```

As the sanity check, this helps to scalarize the array:

```
diff --git a/src/java.base/share/classes/java/util/Arrays.java b/src/java.base/share/classes/java/util/Arrays.java
index c5412776d20..c9191ac3b39 100644
--- a/src/java.base/share/classes/java/util/Arrays.java
+++ b/src/java.base/share/classes/java/util/Arrays.java
@@ -4553,10 +4553,15 @@ public static int hashCode(Object[] a) {
             return 0;
 
         int result = 1;
-
-        for (Object element : a)
-            result = 31 * result + Objects.hashCode(element);
-
+        if (a.length > 0) {
+            result = 31 * result + Objects.hashCode(a[0]);
+        }
+        if (a.length > 1) {
+            result = 31 * result + Objects.hashCode(a[1]);
+        }
+        for (int c = 2; c < a.length; c++) {
+            result = 31 * result + Objects.hashCode(a[c]);
+        }
         return result;
     }
```

I believe this should be handled at C2 side. JDK-8231291 is supposed to unroll the loop aggressively to expand the scope for EA optos, but maybe something else prevents us from seeing through all the accesses for this small array?
Comments
The issue is that the hashCode intrinsic has a slow path that includes a call and that unrolling heuristics exclude loops with calls.
27-05-2024

Yes, I would expect such small loops could be fully unrolled if we inline all related methods. It could be `a.length` is not propagated as constant at that point.
23-05-2024