Uploaded image for project: 'JikesRVM'
  1. JikesRVM
  2. RVM-38

VM_BaselineBootImageCompiler should use System.nanoTime rather than DNA for compilation time

    Details

    • Type: Task
    • Status: Closed
    • Priority: Low
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: 2.9.2
    • Component/s: Compiler: Optimizing
    • Labels:
      None

      Description

      See VM_BaselineBootImageCompiler.compileMethod for TODO item.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            ianrogers Ian Rogers added a comment -

            It's a simple change (see below) but trying to see the performance difference is hard as it only really effects Base? builds. This is well off the normal performance path so I'm going to lower its priority.

            Index: rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java
            ===================================================================
            — rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java (revision 13688)
            +++ rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java (working copy)
            @@ -47,6 +47,7 @@

            • @return the compiled method
              */
              protected VM_CompiledMethod compileMethod(VM_NormalMethod method, VM_TypeReference[] params) { + long startTime = VM.BuildForAdaptiveSystem ? System.nanoTime() : 0; VM_CompiledMethod cm; VM_Callbacks.notifyMethodCompile(method, VM_CompiledMethod.BASELINE); cm = VM_BaselineCompiler.compile(method); @@ -57,7 +58,8 @@ // but 1 millisecond granularity isn't good enough because the // the baseline compiler is just too fast. // TODO: Try Using System.nanoTime() instead - double compileTime = method.getBytecodeLength() / VM_CompilerDNA.getBaselineCompilationRate(); + double compileTime = (double)(System.nanoTime() - startTime); + //method.getBytecodeLength() / VM_CompilerDNA.getBaselineCompilationRate(); cm.setCompilationTime(compileTime); }

              return cm;

            Show
            ianrogers Ian Rogers added a comment - It's a simple change (see below) but trying to see the performance difference is hard as it only really effects Base? builds. This is well off the normal performance path so I'm going to lower its priority. Index: rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java =================================================================== — rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java (revision 13688) +++ rvm/src/org/jikesrvm/compilers/baseline/VM_BaselineBootImageCompiler.java (working copy) @@ -47,6 +47,7 @@ @return the compiled method */ protected VM_CompiledMethod compileMethod(VM_NormalMethod method, VM_TypeReference[] params) { + long startTime = VM.BuildForAdaptiveSystem ? System.nanoTime() : 0; VM_CompiledMethod cm; VM_Callbacks.notifyMethodCompile(method, VM_CompiledMethod.BASELINE); cm = VM_BaselineCompiler.compile(method); @@ -57,7 +58,8 @@ // but 1 millisecond granularity isn't good enough because the // the baseline compiler is just too fast. // TODO: Try Using System.nanoTime() instead - double compileTime = method.getBytecodeLength() / VM_CompilerDNA.getBaselineCompilationRate(); + double compileTime = (double)(System.nanoTime() - startTime); + //method.getBytecodeLength() / VM_CompilerDNA.getBaselineCompilationRate(); cm.setCompilationTime(compileTime); } return cm;
            Hide
            dgrove David Grove added a comment -

            Change committed in 13756. Decided that we should actually not do what the TODO was suggesting. See commit log repeated below:

            Consistently use CompilerDNA values to record compilation times during bootimage writing instead of timing the opt compiler and using DNA for the baseline compiler. We used to think that it was better to really time the compilations, but we can't actually get reliable times from the Host VM because we can't control for GC pauses in the middle of compilation. It's also not clear that compilation times on the build machine would actually be useful to guide recompilation decisions during actual runtime. Therefore, this change causes us to uniformly use CompilerDNA to estimate the compile time for bootimage compilation for the purpose of guiding runtime recompilation decisions. This may not be very accurate, but at least it is a stable and easy to reason about metric.

            Show
            dgrove David Grove added a comment - Change committed in 13756. Decided that we should actually not do what the TODO was suggesting. See commit log repeated below: Consistently use CompilerDNA values to record compilation times during bootimage writing instead of timing the opt compiler and using DNA for the baseline compiler. We used to think that it was better to really time the compilations, but we can't actually get reliable times from the Host VM because we can't control for GC pauses in the middle of compilation. It's also not clear that compilation times on the build machine would actually be useful to guide recompilation decisions during actual runtime. Therefore, this change causes us to uniformly use CompilerDNA to estimate the compile time for bootimage compilation for the purpose of guiding runtime recompilation decisions. This may not be very accurate, but at least it is a stable and easy to reason about metric.

              People

              • Assignee:
                dgrove David Grove
                Reporter:
                pdonald Peter Donald
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: