Skip to content

Commit

Permalink
Constant propagation directly from MarkStep (dotnet/linker#1771)
Browse files Browse the repository at this point in the history 
Call constant propagation and branch removal processing directly from `MarkStep`:
* Renames `RemoveUnreachableBlocksStep` to `UnreachableBlocksOptimizer` - it's not a step anymore
* Rename members to use the `_name` naming convention (which is also used by `MarkStep`)
* Removes the "go over all methods in all assemblies and process them" logic
* Some small changes to the `ProcessMethod` API to be more suitable for the `MarkStep`
* The class is now instantiated by `MarkStep` and only lives as long as `MarkStep` does
* `MarkStep.ProcessMethod` calls the optimizer's `ProcessMethod` before doing anything else on the method (this is because the optimizer can modify methods body, so mark step should only look at the modified version of the method)

Some interesting statistics (on console helloworld):
* Before this change the optimizer processed 73K methods (every method at least once - total number of methods is 72K). After the change the optimizer only processes 12K methods (so a huge reduction). This also means that the large dictionary maintained by the optimizer on all processed methods is now 20% of before this change.
* Max stack size increased to 62 - this is because of different order of processing methods. The 62 comes from `ThrowHelpers` which initializes lot of resource strings (each a separate call to a different method), so at one point all of these dependencies are on the stack. This is not a problem, just slightly higher memory usage. It would become problematic if the stack depth reached several 1000s which is VERY unlikely to happen.
* Number of rewritten methods is down to 20% (from 746 to 117) - all of those not rewritten are actually not used by the app
* As far as I can tell, it produces the exact same output.

Very rough perf measurement on Blazor template app:
* Before all the constant prop changes (SDK from early December 2020): 6.1 seconds (on average)
* After this change: 3.4 seconds (on average)

Commit migrated from dotnet/linker@d8c44b8
  • Loading branch information
@vitek-karas
vitek-karas authored Jan 22, 2021
1 parent d00ebc7 commit 1e2d7a2
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 87 deletions.
5 changes: 4 additions & 1 deletion src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection.Runtime.TypeParsing;
using System.Runtime.CompilerServices;
using System.Text.RegularExpressions;
using Mono.Cecil;
using Mono.Cecil.Cil;
Expand All @@ -57,6 +56,7 @@ public partial class MarkStep : IStep
protected List<MethodBody> _unreachableBodies;

readonly List<(TypeDefinition Type, MethodBody Body, Instruction Instr)> _pending_isinst_instr;
UnreachableBlocksOptimizer _unreachableBlocksOptimizer;

#if DEBUG
static readonly DependencyKind[] _entireTypeReasons = new DependencyKind[] {
Expand Down Expand Up @@ -182,6 +182,7 @@ public MarkStep ()
public virtual void Process (LinkContext context)
{
_context = context;
_unreachableBlocksOptimizer = new UnreachableBlocksOptimizer (_context);

Initialize ();
Process ();
Expand Down Expand Up @@ -2529,6 +2530,8 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
if (CheckProcessed (method))
return;

_unreachableBlocksOptimizer.ProcessMethod (method);

if (!markedForCall)
MarkType (method.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, method), method);
MarkCustomAttributes (method, new DependencyInfo (DependencyKind.CustomAttribute, method), method);
Expand Down
157 changes: 72 additions & 85 deletions ...nker.Steps/RemoveUnreachableBlocksStep.cs → ...inker.Steps/UnreachableBlocksOptimizer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
namespace Mono.Linker.Steps
{
//
// This steps evaluates simple properties or methods for constant expressions and
// Evaluates simple properties or methods for constant expressions and
// then uses this information to remove unreachable conditional blocks. It does
// not do any inlining-like code changes.
//
public class RemoveUnreachableBlocksStep : BaseStep
public class UnreachableBlocksOptimizer
{
readonly LinkContext _context;
MethodDefinition IntPtrSize, UIntPtrSize;

readonly struct ProcessingNode
Expand All @@ -39,7 +40,7 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion)
// Stack of method nodes which are currently being processed.
// Implemented as linked list to allow easy referal to nodes and efficient moving of nodes within the list.
// The top of the stack is the first item in the list.
LinkedList<ProcessingNode> processingStack;
readonly LinkedList<ProcessingNode> _processingStack;

// Each time an item is added or removed from the processing stack this value is incremented.
// Moving items in the stack doesn't increment.
Expand All @@ -50,11 +51,11 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion)
// if we get around to process the method again and the version of the stack didn't change, then there's a loop
// (nothing changed in the stack - order is unimportant, as such no new information has been added and so
// we can't resolve the situation with just the info at hand).
int processingStackVersion;
int _processingStackVersion;

// Just a fast lookup from method to the node on the stack. This is needed to be able to quickly
// access the node and move it to the top of the stack.
Dictionary<MethodDefinition, LinkedListNode<ProcessingNode>> processingMethods;
readonly Dictionary<MethodDefinition, LinkedListNode<ProcessingNode>> _processingMethods;

// Stores results of method processing. This state is kept forever to avoid reprocessing of methods.
// If method is not in the dictionary it has not yet been processed.
Expand All @@ -64,54 +65,17 @@ public ProcessingNode (in ProcessingNode other, int newLastAttempStackVersion)
// - Instruction instance - method has been processed and it has a constant return value (the value of the instruction)
// Note: ProcessedUnchangedSentinel is used as an optimization. running constant value analysis on a method is relatively expensive
// and so we delay it and only do it for methods where the value is asked for (or in case of changed methods upfront due to implementation detailds)
Dictionary<MethodDefinition, Instruction> processedMethods;
readonly Dictionary<MethodDefinition, Instruction> _processedMethods;
static readonly Instruction ProcessedUnchangedSentinel = Instruction.Create (OpCodes.Ldstr, "ProcessedUnchangedSentinel");
static readonly Instruction NonConstSentinel = Instruction.Create (OpCodes.Ldstr, "NonConstSentinel");

protected override void Process ()
public UnreachableBlocksOptimizer (LinkContext context)
{
var assemblies = Context.Annotations.GetAssemblies ().ToArray ();
_context = context;

processingStack = new LinkedList<ProcessingNode> ();
processingMethods = new Dictionary<MethodDefinition, LinkedListNode<ProcessingNode>> ();
processedMethods = new Dictionary<MethodDefinition, Instruction> ();

foreach (var assembly in assemblies) {
if (Annotations.GetAction (assembly) != AssemblyAction.Link)
continue;

ProcessMethods (assembly.MainModule.Types);
}
}

void ProcessMethods (Collection<TypeDefinition> types)
{
foreach (var type in types) {
if (type.IsInterface)
continue;

if (!type.HasMethods)
continue;

foreach (var method in type.Methods) {
if (!method.HasBody)
continue;

//
// Block methods which rewrite does not support
//
switch (method.ReturnType.MetadataType) {
case MetadataType.ByReference:
case MetadataType.FunctionPointer:
continue;
}

ProcessMethod (method);
}

if (type.HasNestedTypes)
ProcessMethods (type.NestedTypes);
}
_processingStack = new LinkedList<ProcessingNode> ();
_processingMethods = new Dictionary<MethodDefinition, LinkedListNode<ProcessingNode>> ();
_processedMethods = new Dictionary<MethodDefinition, Instruction> ();
}

/// <summary>
Expand All @@ -120,52 +84,75 @@ void ProcessMethods (Collection<TypeDefinition> types)
/// It may optimize other methods as well - those are remembered for future reuse.
/// </summary>
/// <param name="method">The method to process</param>
void ProcessMethod (MethodDefinition method)
public void ProcessMethod (MethodDefinition method)
{
Debug.Assert (processingStack.Count == 0 && processingMethods.Count == 0);
processingStackVersion = 0;
if (!IsMethodSupported (method))
return;

if (_context.Annotations.GetAction (method.Module.Assembly) != AssemblyAction.Link)
return;

Debug.Assert (_processingStack.Count == 0 && _processingMethods.Count == 0);
_processingStackVersion = 0;

if (!processedMethods.ContainsKey (method)) {
if (!_processedMethods.ContainsKey (method)) {
AddMethodForProcessing (method);

ProcessStack ();
}

Debug.Assert (processedMethods.ContainsKey (method));
Debug.Assert (_processedMethods.ContainsKey (method));
}

static bool IsMethodSupported (MethodDefinition method)
{
if (!method.HasBody)
return false;

//
// Block methods which rewrite does not support
//
switch (method.ReturnType.MetadataType) {
case MetadataType.ByReference:
case MetadataType.FunctionPointer:
return false;
}

return true;
}

void AddMethodForProcessing (MethodDefinition method)
{
Debug.Assert (!processedMethods.ContainsKey (method));
Debug.Assert (!_processedMethods.ContainsKey (method));

var processingNode = new ProcessingNode (method, -1);

var stackNode = processingStack.AddFirst (processingNode);
processingMethods.Add (method, stackNode);
processingStackVersion++;
var stackNode = _processingStack.AddFirst (processingNode);
_processingMethods.Add (method, stackNode);
_processingStackVersion++;
}

void StoreMethodAsProcessedAndRemoveFromQueue (LinkedListNode<ProcessingNode> stackNode, Instruction methodValue)
{
Debug.Assert (stackNode.List == processingStack);
Debug.Assert (stackNode.List == _processingStack);
Debug.Assert (methodValue != null);

var method = stackNode.Value.Method;
processingMethods.Remove (method);
processingStack.Remove (stackNode);
processingStackVersion++;
_processingMethods.Remove (method);
_processingStack.Remove (stackNode);
_processingStackVersion++;

processedMethods[method] = methodValue;
_processedMethods[method] = methodValue;
}

void ProcessStack ()
{
while (processingStack.Count > 0) {
var stackNode = processingStack.First;
while (_processingStack.Count > 0) {
var stackNode = _processingStack.First;
var method = stackNode.Value.Method;

bool treatUnprocessedDependenciesAsNonConst = false;
if (stackNode.Value.LastAttemptStackVersion == processingStackVersion) {
if (stackNode.Value.LastAttemptStackVersion == _processingStackVersion) {
// Loop was detected - the stack hasn't changed since the last time we tried to process this method
// as such there's no way to resolve the situation (running the code below would produce the exact same result).

Expand All @@ -180,18 +167,18 @@ void ProcessStack ()
// To fix this go over the stack and find the "oldest" node with the current version - the "oldest" node which
// is part of the loop:
LinkedListNode<ProcessingNode> lastNodeWithCurrentVersion = null;
var candidateNodeToMoveToTop = processingStack.Last;
var candidateNodeToMoveToTop = _processingStack.Last;
bool foundNodesWithNonCurrentVersion = false;
while (candidateNodeToMoveToTop != stackNode) {
var previousNode = candidateNodeToMoveToTop.Previous;

if (candidateNodeToMoveToTop.Value.LastAttemptStackVersion == processingStackVersion) {
if (candidateNodeToMoveToTop.Value.LastAttemptStackVersion == _processingStackVersion) {
lastNodeWithCurrentVersion = candidateNodeToMoveToTop;
} else if (lastNodeWithCurrentVersion != null) {
// We've found the "oldest" node with current version and the current node is not of that version
// so it's older version. Move this node to the top of the stack.
processingStack.Remove (candidateNodeToMoveToTop);
processingStack.AddFirst (candidateNodeToMoveToTop);
_processingStack.Remove (candidateNodeToMoveToTop);
_processingStack.AddFirst (candidateNodeToMoveToTop);
foundNodesWithNonCurrentVersion = true;
}

Expand All @@ -215,14 +202,14 @@ void ProcessStack ()
treatUnprocessedDependenciesAsNonConst = true;
}

stackNode.Value = new ProcessingNode (stackNode.Value, processingStackVersion);
stackNode.Value = new ProcessingNode (stackNode.Value, _processingStackVersion);

if (!method.HasBody) {
if (!IsMethodSupported (method)) {
StoreMethodAsProcessedAndRemoveFromQueue (stackNode, ProcessedUnchangedSentinel);
continue;
}

var reducer = new BodyReducer (method.Body, Context);
var reducer = new BodyReducer (method.Body, _context);

//
// Temporary inlines any calls which return contant expression.
Expand All @@ -232,7 +219,7 @@ void ProcessStack ()
if (!TryInlineBodyDependencies (ref reducer, treatUnprocessedDependenciesAsNonConst, out bool changed)) {
// Method has unprocessed dependencies - so back off and try again later
// Leave it in the stack on its current position (it should not be on the first position anymore)
Debug.Assert (processingStack.First != stackNode);
Debug.Assert (_processingStack.First != stackNode);
continue;
}

Expand All @@ -251,7 +238,7 @@ void ProcessStack ()
// branch is replaced with nops.
//
if (reducer.RewriteBody ())
Context.LogMessage ($"Reduced '{reducer.InstructionsReplaced}' instructions in conditional branches for [{method.DeclaringType.Module.Assembly.Name}] method {method.GetDisplayName ()}");
_context.LogMessage ($"Reduced '{reducer.InstructionsReplaced}' instructions in conditional branches for [{method.DeclaringType.Module.Assembly.Name}] method {method.GetDisplayName ()}");

// Even if the rewriter doesn't find any branches to fold the inlining above may have changed the method enough
// such that we can now deduce its return value.
Expand All @@ -272,7 +259,7 @@ void ProcessStack ()
AnalyzeMethodForConstantResult (method, reducer.FoldedInstructions) ?? NonConstSentinel);
}

Debug.Assert (processingMethods.Count == 0);
Debug.Assert (_processingMethods.Count == 0);
}

Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection<Instruction> instructions)
Expand All @@ -283,17 +270,17 @@ Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection<
if (method.ReturnType.MetadataType == MetadataType.Void)
return null;

switch (Context.Annotations.GetAction (method)) {
switch (_context.Annotations.GetAction (method)) {
case MethodAction.ConvertToThrow:
return null;
case MethodAction.ConvertToStub:
return CodeRewriterStep.CreateConstantResultInstruction (Context, method);
return CodeRewriterStep.CreateConstantResultInstruction (_context, method);
}

if (method.IsIntrinsic () || method.NoInlining)
return null;

if (!Context.IsOptimizationEnabled (CodeOptimizations.IPConstantPropagation, method))
if (!_context.IsOptimizationEnabled (CodeOptimizations.IPConstantPropagation, method))
return null;

var analyzer = new ConstantExpressionMethodAnalyzer (method, instructions ?? method.Body.Instructions);
Expand All @@ -318,12 +305,12 @@ Instruction AnalyzeMethodForConstantResult (MethodDefinition method, Collection<
/// </returns>
bool TryGetConstantResultForMethod (MethodDefinition method, out Instruction constantResultInstruction)
{
if (!processedMethods.TryGetValue (method, out Instruction methodValue)) {
if (processingMethods.TryGetValue (method, out var stackNode)) {
if (!_processedMethods.TryGetValue (method, out Instruction methodValue)) {
if (_processingMethods.TryGetValue (method, out var stackNode)) {
// Method is already in the stack - not yet processed
// Move it to the top of the stack
processingStack.Remove (stackNode);
processingStack.AddFirst (stackNode);
_processingStack.Remove (stackNode);
_processingStack.AddFirst (stackNode);

// Note that stack version is not changing - we're just postponing work, not resolving anything.
// There's no result available for this method, so return false.
Expand All @@ -342,7 +329,7 @@ bool TryGetConstantResultForMethod (MethodDefinition method, out Instruction con
// Also its value has not been needed yet. Now we need to know if it's constant, so run the analyzer on it
var result = AnalyzeMethodForConstantResult (method, instructions: null);
Debug.Assert (result is Instruction || result == null);
processedMethods[method] = result ?? NonConstSentinel;
_processedMethods[method] = result ?? NonConstSentinel;
constantResultInstruction = result;
} else if (methodValue == NonConstSentinel) {
// Method was processed and found to not have a constant value
Expand Down Expand Up @@ -376,7 +363,7 @@ bool TryInlineBodyDependencies (ref BodyReducer reducer, bool treatUnprocessedDe
if (md.CallingConvention == MethodCallingConvention.VarArg)
break;

bool explicitlyAnnotated = Annotations.GetAction (md) == MethodAction.ConvertToStub;
bool explicitlyAnnotated = _context.Annotations.GetAction (md) == MethodAction.ConvertToStub;

// Allow inlining results of instance methods which are explicitly annotated
// but don't allow inling results of any other instance method.
Expand Down Expand Up @@ -432,7 +419,7 @@ bool TryInlineBodyDependencies (ref BodyReducer reducer, bool treatUnprocessedDe
if (field == null)
break;

if (Context.Annotations.TryGetFieldUserValue (field, out object value)) {
if (_context.Annotations.TryGetFieldUserValue (field, out object value)) {
targetResult = CodeRewriterStep.CreateConstantResultInstruction (field.FieldType, value);
if (targetResult == null)
break;
Expand Down
1 change: 0 additions & 1 deletion src/tools/illink/src/linker/Linker/Driver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ protected int SetupContext (ILogger customLogger = null)
if (removeCAS)
p.AddStepBefore (typeof (MarkStep), new RemoveSecurityStep ());

p.AddStepBefore (typeof (MarkStep), new RemoveUnreachableBlocksStep ());
p.AddStepBefore (typeof (MarkStep), new MarkExportedTypesTargetStep ());

p.AddStepBefore (typeof (OutputStep), new SealerStep ());
Expand Down
Loading

0 comments on commit 1e2d7a2

Please sign in to comment.