Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8332840

C2: Scalar replacement does not work for multi-argument Objects.hash

XMLWordPrintable

      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?

            roland Roland Westrelin
            shade Aleksey Shipilev
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

              Created:
              Updated: