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

Allocation path: initializing stores are not coalesced with pre-zeroing if NPEs are possible

    Details

    • Type: Enhancement
    • Status: Open
    • Priority: P4
    • Resolution: Unresolved
    • Affects Version/s: 9, 10
    • Fix Version/s: None
    • Component/s: hotspot

      Description

      This issue affects most copying constructors, notably String(String val).

      E.g. if you run this benchmark with current HotSpot, then "copy()" would be slower, because it will
      first pre-zero all the fields in MyClass, then check the incoming argument for nullity, and then overwrite
      the fields with incoming argument fields. When incoming argument is not null, we are doing excess
      writes.

          @Benchmark
          @CompilerControl(CompilerControl.Mode.DONT_INLINE)
          public MyClass instant() {
              return new MyClass(x1, x2, x3, x4);
          }

          @Benchmark
          @CompilerControl(CompilerControl.Mode.DONT_INLINE)
          public MyClass copy() {
              return new MyClass(inst);
          }

          private static class MyClass {
              private long x1, x2, x3, x4;

              public MyClass(long x1, long x2, long x3, long x4) {
                  this.x1 = x1;
                  this.x2 = x2;
                  this.x3 = x3;
                  this.x4 = x4;
              }

              public MyClass(MyClass other) {
                  this.x1 = other.x1;
                  this.x2 = other.x2;
                  this.x3 = other.x3;
                  this.x4 = other.x4;
              }
          }

      Full test, performance results, and disassembly interpretation is here:
       http://cr.openjdk.java.net/~shade/8074566/InitStoreCoalesce.java

      Runnable benchmarks JAR is here:
       http://cr.openjdk.java.net/~shade/8074566/benchmarks.jar

        Activity

        Hide
        jrose John Rose added a comment - - edited
        One contributing cause is the Java language: If a constructor needs to perform argument checking it must do so after the instance is allocated (even partially constructed by a super-class).

        Another contributing cause is the conservative organization of C2: After the instance is allocated, uncommon paths must preserve that instance, even if it is in an incomplete state, because the uncommon path may transition to the interpreter to finish the computation.

        But it would be better to raise the uncommon path (the NPE path in this case, for when an invalid operand is passed to the constructor) above the allocation, so that in the common case the normal initializations can be connected directly to the freshly allocated instance (and therefore merged if redundant).

        In terms of the exact sequencing of VM bytecode operations, the needed check could speculate (before the new/invokespecial) that the operands are well-formed, and dive into the interpreter if the speculation fails; then the JVM-required null check would fold up due to the dominating check and disappear. I think Graal may do this.

        Alternatively (and more complexly) the detection of an NPE could somehow be taken to roll back the JVM state, allowing the NPE check to commute with the allocation.

        Background: Although it is very reasonable to reorder object construction checks with object allocation, the JIT must be scrupulous to maintain the JVM semantics of the bytecodes. In cases like this, that includes maintaining the virtual (apparent) order in which checks are made, in such a way that if multiple failure conditions were present, the exception thrown would correspond to the check which occurs first virtually. Since object allocation can throw exceptions (once in a blue moon), the allocation must appear to execute before the NPE check, in this case.
        Show
        jrose John Rose added a comment - - edited One contributing cause is the Java language: If a constructor needs to perform argument checking it must do so after the instance is allocated (even partially constructed by a super-class). Another contributing cause is the conservative organization of C2: After the instance is allocated, uncommon paths must preserve that instance, even if it is in an incomplete state, because the uncommon path may transition to the interpreter to finish the computation. But it would be better to raise the uncommon path (the NPE path in this case, for when an invalid operand is passed to the constructor) above the allocation, so that in the common case the normal initializations can be connected directly to the freshly allocated instance (and therefore merged if redundant). In terms of the exact sequencing of VM bytecode operations, the needed check could speculate (before the new/invokespecial) that the operands are well-formed, and dive into the interpreter if the speculation fails; then the JVM-required null check would fold up due to the dominating check and disappear. I think Graal may do this. Alternatively (and more complexly) the detection of an NPE could somehow be taken to roll back the JVM state, allowing the NPE check to commute with the allocation. Background: Although it is very reasonable to reorder object construction checks with object allocation, the JIT must be scrupulous to maintain the JVM semantics of the bytecodes. In cases like this, that includes maintaining the virtual (apparent) order in which checks are made, in such a way that if multiple failure conditions were present, the exception thrown would correspond to the check which occurs first virtually. Since object allocation can throw exceptions (once in a blue moon), the allocation must appear to execute before the NPE check, in this case.
        Hide
        shade Aleksey Shipilev added a comment -
        Related: copying String(String val) constructor is the hottest String method at least in SPECjbb2005.
        Show
        shade Aleksey Shipilev added a comment - Related: copying String(String val) constructor is the hottest String method at least in SPECjbb2005.
        Hide
        shade Aleksey Shipilev added a comment - - edited
        I wonder if we "just" need to make any initializing stores available through the common (normal completion) paths only? If a constructor finishes abnormally, that probably means the instance is inaccessible, and therefore the initializing stores are not required? Do we actually care about leaving garbage in the fields for the instance that is not accessible anyway?
        Show
        shade Aleksey Shipilev added a comment - - edited I wonder if we "just" need to make any initializing stores available through the common (normal completion) paths only? If a constructor finishes abnormally, that probably means the instance is inaccessible, and therefore the initializing stores are not required? Do we actually care about leaving garbage in the fields for the instance that is not accessible anyway?
        Hide
        vlivanov Vladimir Ivanov added a comment - - edited
        Sometimes, an instance can be resurrected with finalizer trick (but it's easy to detect that statically and abort coalescing during compilation).
        Another possible problem is GC: if we safepoint when an object is still "reachable", but not fully initialized (oop fields contain garbage), GC can crash trying to traverse the object.

        So, in order to make the optimization work, we should ensure that uninitialized instance isn't "observable" by JVM as well.
        Show
        vlivanov Vladimir Ivanov added a comment - - edited Sometimes, an instance can be resurrected with finalizer trick (but it's easy to detect that statically and abort coalescing during compilation). Another possible problem is GC: if we safepoint when an object is still "reachable", but not fully initialized (oop fields contain garbage), GC can crash trying to traverse the object. So, in order to make the optimization work, we should ensure that uninitialized instance isn't "observable" by JVM as well.
        Hide
        thartmann Tobias Hartmann added a comment -
        Adding wnf-candidate label. We need to investigate if this optimization is possible without violating the Java spec.
        Show
        thartmann Tobias Hartmann added a comment - Adding wnf-candidate label. We need to investigate if this optimization is possible without violating the Java spec.

          People

          • Assignee:
            Unassigned
            Reporter:
            shade Aleksey Shipilev
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated: