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

Coalesce prolog and epilog for empty methods

    Details

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

      Description

      In low-level benchmarking, we sometimes resort to non-inlineable "sink" methods to escape dead-code elimination, like this:

          @Benchmark
          public void test() {
              doNothing(obj);
          }

          @CompilerControl(CompilerControl.Mode.DONT_INLINE)
          public void doNothing(Object obj) {
              // deliberately do nothing
          }

      The performance of this method is very important, since we usually deal with nanosecond-scale benchmarks. Ideally, the generated code should contain a "ret" right away.
      However, the generated code for doNothing contains prolog followed immediately with epilog:

                        [Verified Entry Point]
       10.93% 6.25% 0x00007f39f415fd80: mov %eax,-0x14000(%rsp)
        3.76% 3.03% 0x00007f39f415fd87: push %rbp
        1.92% 1.97% 0x00007f39f415fd88: sub $0x30,%rsp
       10.42% 10.64% 0x00007f39f415fd8c: add $0x30,%rsp
        2.88% 3.03% 0x00007f39f415fd90: pop %rbp
       25.45% 31.68% 0x00007f39f415fd91: test %eax,0x15df8369(%rip) # 0x00007f3a09f58100
                                                                        ; {poll_return}
        0.57% 0.47% 0x00007f39f415fd97: retq

      It seems that at least RSP operations are redundant, as well as saving/restoring RBP.
      It would be interesting to see if we can remove these redundant ops, e.g.:

       *) Peephole MachPrologNode -> MachEpilogNode out completely;
       *) Macro-expand MachProlog/EpilogNode into the individual ops, and then peephole (sub $const, %reg) -> (add $const, %reg) and (push %reg) -> (pop %reg);
       *) Massage frame_size_in_bytes() so that it is zero for empty method;

      Benchmark:
       http://cr.openjdk.java.net/~shade/8130398/EmptyMethod.java

      Runnable JAR:
       http://cr.openjdk.java.net/~shade/8130398/benchmarks.jar

      Output and disasssembly:
       http://cr.openjdk.java.net/~shade/8130398/perfasm.out

        Issue Links

          Activity

          Hide
          shade Aleksey Shipilev added a comment -
          This issue become more important as the go-to technique for implementing Fences.reachabilityFence and/or VarHandle.(get|set)Opaque in OpenJDK.
          Show
          shade Aleksey Shipilev added a comment - This issue become more important as the go-to technique for implementing Fences.reachabilityFence and/or VarHandle.(get|set)Opaque in OpenJDK.
          Hide
          thartmann Tobias Hartmann added a comment -
          Could this be because the empty method is considered as trivial and only compiled by C1 (see JDK-8145579)?
          Show
          thartmann Tobias Hartmann added a comment - Could this be because the empty method is considered as trivial and only compiled by C1 (see JDK-8145579 )?
          Hide
          shade Aleksey Shipilev added a comment -
          Indeed, the assembly above is created by C1. Re-checked with 9b152, and updated JMH that prints out the compiler level, and there is a similar issue with C2:
           http://cr.openjdk.java.net/~shade/8130398/c1.perfasm
           http://cr.openjdk.java.net/~shade/8130398/c2.perfasm
          Show
          shade Aleksey Shipilev added a comment - Indeed, the assembly above is created by C1. Re-checked with 9b152, and updated JMH that prints out the compiler level, and there is a similar issue with C2:   http://cr.openjdk.java.net/~shade/8130398/c1.perfasm   http://cr.openjdk.java.net/~shade/8130398/c2.perfasm
          Hide
          thartmann Tobias Hartmann added a comment -
          Thanks, Aleksey!
          Show
          thartmann Tobias Hartmann added a comment - Thanks, Aleksey!

            People

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

              Dates

              • Created:
                Updated: