From 4229ccb673fab1761e722bb3fc389129e605e972 Mon Sep 17 00:00:00 2001 From: noahfalk Date: Wed, 16 May 2018 17:25:35 -0700 Subject: [PATCH] Addressing code review feedback --- src/PerfView/JitStats.cs | 4 +- .../SupportFiles/HtmlReportUsersGuide.htm | 4 +- .../Computers/TraceManagedProcess.cs | 37 ++++++++++++------- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/PerfView/JitStats.cs b/src/PerfView/JitStats.cs index 88840faa5..89f71cabe 100644 --- a/src/PerfView/JitStats.cs +++ b/src/PerfView/JitStats.cs @@ -472,7 +472,7 @@ public static JITStatsEx Create(TraceLoadedDotNetRuntime mang) stats.TotalBGJITStats = new JITStats(); foreach (TraceJittedMethod _method in mang.JIT.Methods) { - stats.TotalBGJITStats.Update(_method); + stats.TotalBGJITStats.AddMethodToStatistics(_method); } } @@ -487,7 +487,7 @@ public static JITStatsEx Create(TraceLoadedDotNetRuntime mang) stats.TotalModuleStats.Add(_method.ModuleILPath, new JITStats()); } JITStats moduleStats = stats.TotalModuleStats[_method.ModuleILPath]; - moduleStats.Update(_method); + moduleStats.AddMethodToStatistics(_method); } } diff --git a/src/PerfView/SupportFiles/HtmlReportUsersGuide.htm b/src/PerfView/SupportFiles/HtmlReportUsersGuide.htm index 6afc8abbe..1e60c8654 100644 --- a/src/PerfView/SupportFiles/HtmlReportUsersGuide.htm +++ b/src/PerfView/SupportFiles/HtmlReportUsersGuide.htm @@ -165,7 +165,7 @@

Viewing Background JIT Compilation events. 

  • The 'Trigger' column of each method indicates whether the compilation happened in the foreground.

    • What can go wrong with background JIT compilation.

    -

    It your program has used SetProfileRoot and StartProfile, but JIT compilation (as shown in the JITStats view) does not show any or very little background JIT compilation, there are several issues that may be responsible.   Fundamentally a important design goal was to insure that background JIT compilation did not change the behavior of the program under any circumstances.    Unfortunately, it means that the algorithm tends to bail out quickly.   In particular

    +

    If your program has used SetProfileRoot and StartProfile, but JIT compilation (as shown in the JITStats view) does not show any or very little background JIT compilation, there are several issues that may be responsible.   Fundamentally a important design goal was to insure that background JIT compilation did not change the behavior of the program under any circumstances.    Unfortunately, it means that the algorithm tends to bail out quickly.   In particular

    1. When modules are loaded, a module constructor could be called, which could have side effects (even this is very rare).   Thus if background JITTing would cause a module to be loaded earlier than it otherwise would be, it could expose (rare) bugs.  Because background JIT had a very high compatibility bar, it protects against this by taging each method with the EXACT modules that were loaded at the time of JIT compilation, and only allows them to be background JIT compiled after all those EXACT modules were also loaded in the current run.   Thus if you have a scenario (say a menu opening), where sometimes more or fewer modules are loaded (because previous user actions caused different modules to load), then background JIT may not work well. 
    2. If you have attached a callback to the System.Assembly.ModuleResolve event, it is possible (although extremely unlikely and very bad design) that background JITing could have side effects if the ModuleResolve callback returned different answers on the second run than it did on the first run.   Because of this background JIT compilation is suspended the first time an ModuleResolve callback in invoked.
      3. @@ -196,7 +196,7 @@

      Understanding Tiered Compilation events

    3. The method could be jitted by a foreground thread just prior to its first execution, in which case it is contained in the 'Foreground' group
    4. If the Background JIT feature is enabled the method's usage may have been accurately predicted and jitted in advance on the Multicore JIT background thread, in which case it is accounted for in the 'Multicore JIT Background' group
    -

    In the individual method listings the column 'Trigger' contains the value 'TC' for each Tier1 background recompilation due to Tiered compilation

    +

    In the individual method listings the column 'Trigger' contains the value 'TC' for each Tier1 background recompilation due to Tiered Compilation

     

     

     

    diff --git a/src/TraceEvent/Computers/TraceManagedProcess.cs b/src/TraceEvent/Computers/TraceManagedProcess.cs index d31d5dc65..116915f1a 100644 --- a/src/TraceEvent/Computers/TraceManagedProcess.cs +++ b/src/TraceEvent/Computers/TraceManagedProcess.cs @@ -3412,29 +3412,29 @@ public class JITStats public HashSet SymbolsMissing = new HashSet(); /// - /// Update method statistics + /// Aggregate a method to be included in the statistics /// - /// - public void Update(TraceJittedMethod _method) + /// + public void AddMethodToStatistics(TraceJittedMethod method) { Count++; - TotalCpuTimeMSec += _method.CompileCpuTimeMSec; - TotalILSize += _method.ILSize; - TotalNativeSize += _method.NativeSize; - if (_method.CompilationThreadKind == CompilationThreadKind.MulticoreJitBackground) + TotalCpuTimeMSec += method.CompileCpuTimeMSec; + TotalILSize += method.ILSize; + TotalNativeSize += method.NativeSize; + if (method.CompilationThreadKind == CompilationThreadKind.MulticoreJitBackground) { CountBackgroundMultiCoreJit++; - TotalBackgroundMultiCoreJitCpuTimeMSec += _method.CompileCpuTimeMSec; + TotalBackgroundMultiCoreJitCpuTimeMSec += method.CompileCpuTimeMSec; } - else if (_method.CompilationThreadKind == CompilationThreadKind.TieredCompilationBackground) + else if (method.CompilationThreadKind == CompilationThreadKind.TieredCompilationBackground) { CountBackgroundTieredCompilation++; - TotalBackgroundTieredCompilationCpuTimeMSec += _method.CompileCpuTimeMSec; + TotalBackgroundTieredCompilationCpuTimeMSec += method.CompileCpuTimeMSec; } - else if(_method.CompilationThreadKind == CompilationThreadKind.Foreground) + else if(method.CompilationThreadKind == CompilationThreadKind.Foreground) { CountForeground++; - TotalForegroundCpuTimeMSec += _method.CompileCpuTimeMSec; + TotalForegroundCpuTimeMSec += method.CompileCpuTimeMSec; } } @@ -3479,7 +3479,7 @@ internal static TraceJittedMethod MethodComplete(TraceLoadedDotNetRuntime stats, } _method.Completed++; - stats.JIT.m_stats.Update(_method); + stats.JIT.m_stats.AddMethodToStatistics(_method); return _method; } @@ -3679,6 +3679,17 @@ public class TraceJittedMethod /// public int ThreadID; /// + /// Indication of if it was JIT'd in the background + /// + [Obsolete("Use CompilationThreadKind")] + public bool IsBackground + { + get + { + return CompilationThreadKind != CompilationThreadKind.Foreground; + } + } + /// /// Indication of if it was JIT'd in the background and why /// public CompilationThreadKind CompilationThreadKind;