RVM
  1. RVM
  2. RVM-38

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

    Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: 2.9.2
    • Component/s: Compiler: Optimizing
    • Labels:
      None
    • Number of attachments :
      0

      Description

      See VM_BaselineBootImageCompiler.compileMethod for TODO item.

        Activity

        Hide
        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
        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
        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
        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:
            David Grove
            Reporter:
            Peter Donald
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: