From 13988afa2554aa2fec980ed662383ee839a69dd1 Mon Sep 17 00:00:00 2001 From: Pavel Mikula Date: Thu, 17 Feb 2022 11:15:33 +0100 Subject: [PATCH] Cache CreateCfg --- .../Roslyn/ControlFlowGraphCache.cs | 78 +++++++++++++++++++ .../Extensions/SyntaxNodeExtensions.cs | 44 ++++------- .../Extensions/SyntaxNodeExtensions.cs | 34 +++----- .../Extensions/SyntaxNodeExtensionsTest.cs | 65 ++++++++++++++++ 4 files changed, 168 insertions(+), 53 deletions(-) create mode 100644 analyzers/src/SonarAnalyzer.CFG/Roslyn/ControlFlowGraphCache.cs diff --git a/analyzers/src/SonarAnalyzer.CFG/Roslyn/ControlFlowGraphCache.cs b/analyzers/src/SonarAnalyzer.CFG/Roslyn/ControlFlowGraphCache.cs new file mode 100644 index 00000000000..5f519776402 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CFG/Roslyn/ControlFlowGraphCache.cs @@ -0,0 +1,78 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using System.Linq; +using System.Runtime.CompilerServices; +using Microsoft.CodeAnalysis; +using SonarAnalyzer.Extensions; +using StyleCop.Analyzers.Lightup; + +namespace SonarAnalyzer.CFG.Roslyn +{ + public abstract class ControlFlowGraphCacheBase + { + private readonly ConditionalWeakTable cache = new(); + + protected abstract bool HasNestedCfg(SyntaxNode node); + protected abstract bool IsLocalFunction(SyntaxNode node); + + public ControlFlowGraph FindOrCreate(SyntaxNode declaration, SemanticModel model) + { + var rootSyntax = model.GetOperation(declaration).RootOperation().Syntax; + var wrapper = cache.GetValue(rootSyntax, x => new Wrapper(ControlFlowGraph.Create(x, model))); + if (HasNestedCfg(declaration)) + { + // We need to go up and track all possible enclosing lambdas, local functions and other FlowAnonymousFunctionOperations + foreach (var node in declaration.AncestorsAndSelf().TakeWhile(x => x != rootSyntax).Reverse()) + { + if (IsLocalFunction(node)) + { + wrapper = new(wrapper.Cfg.GetLocalFunctionControlFlowGraph(node)); + } + else if (wrapper.FlowOperation(node) is { WrappedOperation: not null } flowOperation) + { + wrapper = new(wrapper.Cfg.GetAnonymousFunctionControlFlowGraph(flowOperation)); + } + else if (node == declaration) + { + return null; // Lambda syntax is not always recognized as a FlowOperation for invalid syntaxes + } + } + } + return wrapper.Cfg; + } + + private sealed class Wrapper + { + private IFlowAnonymousFunctionOperationWrapper[] flowOperations; + + public ControlFlowGraph Cfg { get; } + + public Wrapper(ControlFlowGraph cfg) => + Cfg = cfg; + + public IFlowAnonymousFunctionOperationWrapper FlowOperation(SyntaxNode node) + { + flowOperations ??= Cfg.FlowAnonymousFunctionOperations().ToArray(); // Avoid recomputing, it's expensive and called many times for a single CFG + return flowOperations.SingleOrDefault(x => x.WrappedOperation.Syntax == node); + } + } + } +} diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index 2342b97b6fa..1111611bdf9 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -30,6 +30,11 @@ namespace SonarAnalyzer.Extensions { internal static partial class SyntaxNodeExtensions { + private static readonly ControlFlowGraphCache CfgCache = new(); + + public static ControlFlowGraph CreateCfg(this SyntaxNode body, SemanticModel model) => + CfgCache.FindOrCreate(body.Parent, model); + public static bool ContainsConditionalConstructs(this SyntaxNode node) => node != null && node.DescendantNodes() @@ -113,36 +118,6 @@ public static SyntaxNode RemoveParentheses(this SyntaxNode expression) return current; } - public static ControlFlowGraph CreateCfg(this SyntaxNode body, SemanticModel semanticModel) - { - var operation = semanticModel.GetOperation(body.Parent); - var rootSyntax = operation.RootOperation().Syntax; - var cfg = ControlFlowGraph.Create(rootSyntax, semanticModel); - if (body.Parent.IsAnyKind(SyntaxKindEx.LocalFunctionStatement, SyntaxKind.SimpleLambdaExpression, SyntaxKind.AnonymousMethodExpression, SyntaxKind.ParenthesizedLambdaExpression)) - { - // We need to go up and track all possible enclosing lambdas, local functions and other FlowAnonymousFunctionOperations - var cfgFlowOperations = cfg.FlowAnonymousFunctionOperations().ToArray(); // Avoid recomputing for ancestors that do not produce FlowAnonymousFunction - foreach (var node in body.Parent.AncestorsAndSelf().TakeWhile(x => x != rootSyntax).Reverse()) - { - if (node.IsKind(SyntaxKindEx.LocalFunctionStatement)) - { - cfg = cfg.GetLocalFunctionControlFlowGraph(node); - cfgFlowOperations = cfg.FlowAnonymousFunctionOperations().ToArray(); - } - else if (cfgFlowOperations.SingleOrDefault(x => x.WrappedOperation.Syntax == node) is { WrappedOperation: not null } flowOperation) - { - cfg = cfg.GetAnonymousFunctionControlFlowGraph(flowOperation); - cfgFlowOperations = cfg.FlowAnonymousFunctionOperations().ToArray(); - } - else if (node == body) - { - return null; - } - } - } - return cfg; - } - private static string GetUnknownType(SyntaxKind kind) { #if DEBUG @@ -151,5 +126,14 @@ private static string GetUnknownType(SyntaxKind kind) return "type"; #endif } + + private sealed class ControlFlowGraphCache : ControlFlowGraphCacheBase + { + protected override bool IsLocalFunction(SyntaxNode node) => + node.IsKind(SyntaxKindEx.LocalFunctionStatement); + + protected override bool HasNestedCfg(SyntaxNode node) => + node.IsAnyKind(SyntaxKindEx.LocalFunctionStatement, SyntaxKind.SimpleLambdaExpression, SyntaxKind.AnonymousMethodExpression, SyntaxKind.ParenthesizedLambdaExpression); + } } } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs index e7706d2c29f..48ed10e2301 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.VisualBasic; using Microsoft.CodeAnalysis.VisualBasic.Syntax; @@ -29,6 +28,11 @@ namespace SonarAnalyzer.Extensions { internal static partial class SyntaxNodeExtensions { + private static readonly ControlFlowGraphCache CfgCache = new(); + + public static ControlFlowGraph CreateCfg(this SyntaxNode block, SemanticModel model) => + CfgCache.FindOrCreate(block, model); + public static bool IsPartOfBinaryNegationOrCondition(this SyntaxNode node) { if (!(node.Parent is MemberAccessExpressionSyntax)) @@ -56,29 +60,13 @@ public static object FindConstantValue(this SyntaxNode node, SemanticModel seman public static string FindStringConstant(this SyntaxNode node, SemanticModel semanticModel) => FindConstantValue(node, semanticModel) as string; - public static ControlFlowGraph CreateCfg(this SyntaxNode block, SemanticModel semanticModel) + private sealed class ControlFlowGraphCache : ControlFlowGraphCacheBase { - var operation = semanticModel.GetOperation(block); - var rootSyntax = operation.RootOperation().Syntax; - var cfg = ControlFlowGraph.Create(rootSyntax, semanticModel); - if (block is LambdaExpressionSyntax) - { - // We need to go up and track all possible enclosing lambdas and other FlowAnonymousFunctionOperations - var cfgFlowOperations = cfg.FlowAnonymousFunctionOperations().ToArray(); // Avoid recomputing for ancestors that do not produce FlowAnonymousFunction - foreach (var node in block.AncestorsAndSelf().TakeWhile(x => x != rootSyntax).Reverse()) - { - if (cfgFlowOperations.SingleOrDefault(x => x.WrappedOperation.Syntax == node) is { WrappedOperation: not null } flowOperation) - { - cfg = cfg.GetAnonymousFunctionControlFlowGraph(flowOperation); - cfgFlowOperations = cfg.FlowAnonymousFunctionOperations().ToArray(); - } - else if (node == block) - { - return null; // Lambda syntax is not always recognized as FlowOperation for invalid syntaxes - } - } - } - return cfg; + protected override bool IsLocalFunction(SyntaxNode node) => + false; + + protected override bool HasNestedCfg(SyntaxNode node) => + node is LambdaExpressionSyntax; } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index 33ea2d80c13..743bfdd41cd 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -20,8 +20,11 @@ extern alias csharp; extern alias vbnet; + +using System; using System.Linq; using FluentAssertions; +using FluentAssertions.Extensions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Operations; @@ -330,6 +333,68 @@ public void CreateCfg_InvalidSyntax_ReturnsCfg_CS(string code) SyntaxNodeExtensionsCS.CreateCfg(lambda.Body, model).Should().NotBeNull(); } + [TestMethod] + public void CreateCfg_Performance_UsesCache_CS() + { + const string code = @" +using System; +public class Sample +{ + public void Main(string noiseToHaveMoreOperations) + { + noiseToHaveMoreOperations.ToString(); + noiseToHaveMoreOperations.ToString(); + noiseToHaveMoreOperations.ToString(); + void LocalFunction() + { + noiseToHaveMoreOperations.ToString(); + noiseToHaveMoreOperations.ToString(); + noiseToHaveMoreOperations.ToString(); + Action a = () => 42.ToString(); + } + } +}"; + var (tree, model) = TestHelper.CompileCS(code); + var lambda = tree.GetRoot().DescendantNodes().OfType().Single(); + Action a = () => + { + for (var i = 0; i < 10000; i++) + { + SyntaxNodeExtensionsCS.CreateCfg(lambda, model); + } + }; + a.ExecutionTime().Should().BeLessThan(1.Seconds()); // Takes roughly 0.2 sec on CI + } + + [TestMethod] + public void CreateCfg_Performance_UsesCache_VB() + { + const string code = @" +Public Class Sample + Public Sub Main(NoiseToHaveMoreOperations As String) + NoiseToHaveMoreOperations.ToString() + NoiseToHaveMoreOperations.ToString() + NoiseToHaveMoreOperations.ToString() + Dim Outer As Action = Sub() + NoiseToHaveMoreOperations.ToString() + NoiseToHaveMoreOperations.ToString() + NoiseToHaveMoreOperations.ToString() + Dim Inner As Action = Sub() NoiseToHaveMoreOperations.ToString() + End Sub + End Sub +End Class"; + var (tree, model) = TestHelper.CompileVB(code); + var lambda = tree.GetRoot().DescendantNodes().OfType().Single(); + Action a = () => + { + for (var i = 0; i < 10000; i++) + { + SyntaxNodeExtensionsCS.CreateCfg(lambda, model); + } + }; + a.ExecutionTime().Should().BeLessThan(1.Seconds()); // Takes roughly 0.4 sec on CI + } + private static SyntaxToken GetFirstTokenOfKind(SyntaxTree syntaxTree, SyntaxKind kind) => syntaxTree.GetRoot().DescendantTokens().First(token => token.IsKind(kind)); }