Skip to content

Commit

Permalink
Addressing code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
noahfalk committed May 17, 2018
1 parent eba4f09 commit 4229ccb
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 17 deletions.
4 changes: 2 additions & 2 deletions src/PerfView/JitStats.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/PerfView/SupportFiles/HtmlReportUsersGuide.htm
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ <h4>Viewing Background JIT Compilation events.&nbsp; </h4>
<li>The &#39;Trigger&#39; column of each method indicates whether the compilation happened in the foreground.</li>
</ol>
<p><strong>What can go wrong with background JIT compilation.</strong></p>
<p>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.&nbsp;&nbsp; Fundamentally a important design goal was to insure that background JIT compilation did not change the behavior of the program under <strong>any circumstances</strong>.&nbsp;&nbsp;&nbsp; Unfortunately, it means that the algorithm tends to bail out quickly.&nbsp;&nbsp; In particular</p>
<p>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.&nbsp;&nbsp; Fundamentally a important design goal was to insure that background JIT compilation did not change the behavior of the program under <strong>any circumstances</strong>.&nbsp;&nbsp;&nbsp; Unfortunately, it means that the algorithm tends to bail out quickly.&nbsp;&nbsp; In particular</p>
<ol>
<li>When modules are loaded, a module constructor <strong>could</strong> be called, which <strong>could</strong> have side effects (even this is very rare).&nbsp;&nbsp; Thus if background JITTing would cause a module to be loaded earlier than it otherwise would be, it <strong>could</strong> expose (rare) bugs.&nbsp; 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.&nbsp;&nbsp; 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.&nbsp; </li>
<li>If you have attached a callback to the <strong>System.Assembly.ModuleResolve</strong> 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.&nbsp;&nbsp; Because of this background JIT compilation is suspended the first time an ModuleResolve callback in invoked.</li>
Expand Down Expand Up @@ -196,7 +196,7 @@ <h4>Understanding Tiered Compilation events</h4>
<li>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</li>
<li>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</li>
</ol>
<p>In the individual method listings the column 'Trigger' contains the value 'TC' for each Tier1 background recompilation due to Tiered compilation</p>
<p>In the individual method listings the column 'Trigger' contains the value 'TC' for each Tier1 background recompilation due to Tiered Compilation</p>
<p>&nbsp;</p>
<p>&nbsp;</p>
<p>&nbsp;</p>
Expand Down
37 changes: 24 additions & 13 deletions src/TraceEvent/Computers/TraceManagedProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3412,29 +3412,29 @@ public class JITStats
public HashSet<string> SymbolsMissing = new HashSet<string>();

/// <summary>
/// Update method statistics
/// Aggregate a method to be included in the statistics
/// </summary>
/// <param name="_method"></param>
public void Update(TraceJittedMethod _method)
/// <param name="method"></param>
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;
}
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -3679,6 +3679,17 @@ public class TraceJittedMethod
/// </summary>
public int ThreadID;
/// <summary>
/// Indication of if it was JIT'd in the background
/// </summary>
[Obsolete("Use CompilationThreadKind")]
public bool IsBackground
{
get
{
return CompilationThreadKind != CompilationThreadKind.Foreground;
}
}
/// <summary>
/// Indication of if it was JIT'd in the background and why
/// </summary>
public CompilationThreadKind CompilationThreadKind;
Expand Down

0 comments on commit 4229ccb

Please sign in to comment.