From 90d866d78a742e23e13b8334f1dcac2d74d896d7 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 15 Jul 2018 15:21:46 +0200 Subject: [PATCH] Fix #1193: Performance issues with deeply nested block structures --- .../IL/ControlFlow/ConditionDetection.cs | 15 ++++---- .../IL/Transforms/DelegateConstruction.cs | 37 +++++++++---------- .../IL/Transforms/TransformExpressionTrees.cs | 2 + 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs index e75313e65..4d95465fb 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ConditionDetection.cs @@ -125,8 +125,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow context.Step("Inline block as then-branch", ifInst.TrueInst); // The targetBlock was already processed, and is ready to embed var targetBlock = ((Branch)ifInst.TrueInst).TargetBlock; + targetBlock.AddRef(); // Peformance: avoid temporarily disconnecting targetBlock targetBlock.Remove(); ifInst.TrueInst = targetBlock; + targetBlock.ReleaseRef(); return true; } @@ -430,8 +432,10 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow context.Step("Swap then-branch with else-branch to match IL order", ifInst); var oldTrue = ifInst.TrueInst; + oldTrue.AddRef(); // Peformance: avoid temporarily disconnecting oldTrue ifInst.TrueInst = ifInst.FalseInst; ifInst.FalseInst = oldTrue; + oldTrue.ReleaseRef(); ifInst.Condition = Comp.LogicNot(ifInst.Condition); } @@ -599,14 +603,9 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // continue blocks have exactly 2 incoming edges if (currentContainer.EntryPoint.IncomingEdgeCount == 2) { - try { - var forIncrement = HighLevelLoopTransform.GetIncrementBlock(currentContainer, currentContainer.EntryPoint); - if (forIncrement != null) - return forIncrement; - } catch (InvalidOperationException) { - // multiple potential increment blocks. Can get this because we don't check that the while loop - // has a condition first, as we don't need to do too much of HighLevelLoopTransform's job. - } + var forIncrement = HighLevelLoopTransform.GetIncrementBlock(currentContainer, currentContainer.EntryPoint); + if (forIncrement != null) + return forIncrement; } return currentContainer.EntryPoint; diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs index 30b08fbd3..6c6cc0366 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs @@ -40,26 +40,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms var orphanedVariableInits = new List(); var targetsToReplace = new List(); var translatedDisplayClasses = new HashSet(); - foreach (var block in function.Descendants.OfType()) { - for (int i = block.Instructions.Count - 1; i >= 0; i--) { - context.CancellationToken.ThrowIfCancellationRequested(); - foreach (var call in block.Instructions[i].Descendants.OfType()) { - ILFunction f = TransformDelegateConstruction(call, out ILInstruction target); - if (f != null) { - call.ReplaceWith(f); - if (target is IInstructionWithVariableOperand && !target.MatchLdThis()) - targetsToReplace.Add((IInstructionWithVariableOperand)target); - } + var cancellationToken = context.CancellationToken; + foreach (var inst in function.Descendants) { + cancellationToken.ThrowIfCancellationRequested(); + if (inst is NewObj call) { + ILFunction f = TransformDelegateConstruction(call, out ILInstruction target); + if (f != null) { + call.ReplaceWith(f); + if (target is IInstructionWithVariableOperand && !target.MatchLdThis()) + targetsToReplace.Add((IInstructionWithVariableOperand)target); } - if (block.Instructions[i].MatchStLoc(out ILVariable targetVariable, out ILInstruction value)) { - var newObj = value as NewObj; - // TODO : it is probably not a good idea to remove *all* display-classes - // is there a way to minimize the false-positives? - if (newObj != null && IsInSimpleDisplayClass(newObj.Method)) { - targetVariable.CaptureScope = BlockContainer.FindClosestContainer(block); - targetsToReplace.Add((IInstructionWithVariableOperand)block.Instructions[i]); - translatedDisplayClasses.Add(newObj.Method.DeclaringTypeDefinition); - } + } + if (inst.MatchStLoc(out ILVariable targetVariable, out ILInstruction value)) { + var newObj = value as NewObj; + // TODO : it is probably not a good idea to remove *all* display-classes + // is there a way to minimize the false-positives? + if (newObj != null && IsInSimpleDisplayClass(newObj.Method)) { + targetVariable.CaptureScope = BlockContainer.FindClosestContainer(inst); + targetsToReplace.Add((IInstructionWithVariableOperand)inst); + translatedDisplayClasses.Add(newObj.Method.DeclaringTypeDefinition); } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs index dba75bd17..b38857dd1 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs @@ -130,6 +130,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms } return false; } + if (instruction is Block block && block.Kind == BlockKind.ControlFlow) + return false; // don't look into nested blocks foreach (var child in instruction.Children) { if (TryConvertExpressionTree(child, statement)) return true;