From bebb3ac815c177c8c231714a734cdf322e1fd52f Mon Sep 17 00:00:00 2001 From: Heejae Chang Date: Fri, 9 Jun 2017 12:22:00 -0700 Subject: [PATCH] some clean up --- .../Compilation/MemberSemanticModel.cs | 2 +- .../LocalRewriter/LocalRewriter_Call.cs | 1 - .../Operations/CSharpOperationFactory.cs | 2 +- .../CSharpOperationFactory_Methods.cs | 14 ++++----- .../Portable/Operations/OperationCache.cs | 29 +++++++++++++++++-- .../Operations/VisualBasicOperationFactory.vb | 2 +- .../VisualBasicOperationFactory_Methods.vb | 24 +++++++-------- 7 files changed, 48 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs index 3639aff5ef6..034e9145085 100644 --- a/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs +++ b/src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs @@ -1546,7 +1546,7 @@ private static Binder GetQueryEnclosingBinder(int position, CSharpSyntaxNode sta } while (node != null); - done: +done: return GetEnclosingBinderInternalWithinRoot(AdjustStartingNodeAccordingToNewRoot(startingNode, queryClause.Syntax), position, queryClause.Binder, queryClause.Syntax); } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs index 3998c0d8cd0..e30d2a298c7 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs @@ -622,7 +622,6 @@ private static ImmutableArray GetRefKindsOrNull(ArrayBuilder r // then we need to create a temporary as usual. The step that // produces the parameter array will need to deal with that // eventuality. - if (IsBeginningOfParamArray(p, a, expanded, arguments.Length, rewrittenArguments, argsToParamsOpt, out int paramArrayArgumentCount) && a + paramArrayArgumentCount == rewrittenArguments.Length) { diff --git a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs index 2cee5ba7d60..d4da5515310 100644 --- a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs +++ b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs @@ -19,7 +19,7 @@ public IOperation Create(BoundNode boundNode) return null; } - return _cache.GetValue(boundNode, n => CreateInternal(n)); + return _cache.GetOrCreateOperationFrom(boundNode, n => CreateInternal(n)); } private IOperation CreateInternal(BoundNode boundNode) diff --git a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory_Methods.cs b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory_Methods.cs index d9351fc9693..62934c383a3 100644 --- a/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory_Methods.cs +++ b/src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory_Methods.cs @@ -31,7 +31,7 @@ private ImmutableArray ToStatements(BoundStatement statement) private ILiteralExpression CreateIncrementOneLiteralExpression(BoundIncrementOperator boundIncrementOperator) { - return _cache.GetValue(boundIncrementOperator, nameof(CreateIncrementOneLiteralExpression), (increment) => + return _cache.GetOrCreateOperationFrom(boundIncrementOperator, nameof(CreateIncrementOneLiteralExpression), (increment) => { string text = increment.Syntax.ToString(); bool isInvalid = false; @@ -77,7 +77,7 @@ internal static IArgument CreateArgumentOperation(ArgumentKind kind, IParameterS return ImmutableArray.Empty; } - return _cache.GetValue( + return _cache.GetOrCreateOperationsFrom( boundNode, nameof(DeriveArguments), (n) => { @@ -167,7 +167,7 @@ private static ITypeSymbol GetArrayCreationElementType(BoundArrayCreation creati private ImmutableArray GetBlockStatement(BoundBlock block) { // This is to filter out operations of kind None. - return _cache.GetValue(block, nameof(GetBlockStatement), + return _cache.GetOrCreateOperationsFrom(block, nameof(GetBlockStatement), blockStatement => { return blockStatement.Statements.Select(s => Create(s)).Where(s => s.Kind != OperationKind.None).ToImmutableArray(); @@ -176,7 +176,7 @@ private ImmutableArray GetBlockStatement(BoundBlock block) private ImmutableArray GetSwitchStatementCases(BoundSwitchStatement statement) { - return _cache.GetValue(statement, nameof(GetSwitchStatementCases), + return _cache.GetOrCreateOperationsFrom(statement, nameof(GetSwitchStatementCases), switchStatement => { return switchStatement.SwitchSections.SelectAsArray(switchSection => @@ -228,14 +228,14 @@ private static BinaryOperationKind GetLabelEqualityKind(BoundSwitchLabel label) private ImmutableArray GetVariableDeclarationStatementVariables(BoundLocalDeclaration decl) { - return _cache.GetValue(decl, nameof(GetVariableDeclarationStatementVariables), + return _cache.GetOrCreateOperationsFrom(decl, nameof(GetVariableDeclarationStatementVariables), declaration => ImmutableArray.Create( OperationFactory.CreateVariableDeclaration(declaration.LocalSymbol, Create(declaration.InitializerOpt), declaration.Syntax))); } private ImmutableArray GetVariableMultipleDeclarationStatementVariables(BoundMultipleLocalDeclarations decl) { - return _cache.GetValue(decl, nameof(GetVariableMultipleDeclarationStatementVariables), + return _cache.GetOrCreateOperationsFrom(decl, nameof(GetVariableMultipleDeclarationStatementVariables), multipleDeclarations => multipleDeclarations.LocalDeclarations.SelectAsArray(declaration => OperationFactory.CreateVariableDeclaration(declaration.LocalSymbol, Create(declaration.InitializerOpt), declaration.Syntax))); @@ -243,7 +243,7 @@ private ImmutableArray GetVariableMultipleDeclarationState private ImmutableArray GetInterpolatedStringExpressionParts(BoundInterpolatedString boundInterpolatedString) { - return _cache.GetValue(boundInterpolatedString, nameof(GetInterpolatedStringExpressionParts), + return _cache.GetOrCreateOperationsFrom(boundInterpolatedString, nameof(GetInterpolatedStringExpressionParts), interpolatedString => interpolatedString.Parts.SelectAsArray(interpolatedStringContent => CreateBoundInterpolatedStringContentOperation(interpolatedStringContent))); } diff --git a/src/Compilers/Core/Portable/Operations/OperationCache.cs b/src/Compilers/Core/Portable/Operations/OperationCache.cs index b19365b4297..c7486212661 100644 --- a/src/Compilers/Core/Portable/Operations/OperationCache.cs +++ b/src/Compilers/Core/Portable/Operations/OperationCache.cs @@ -2,21 +2,44 @@ using System; using System.Collections.Concurrent; +using System.Collections.Immutable; namespace Microsoft.CodeAnalysis { + /// + /// Cache data associated with TBoundNode + /// internal class OperationCache { + // REVIEW: not sure about perf of value tuple used as key on a dictionary. I might need custom type + // if it is somehow not good on perf + // + // cache's key is (TBoundNode, string) since some of TBoundNode has multiple data associated with it. private readonly ConcurrentDictionary<(TBoundNode key, string kind), object> _cache = new ConcurrentDictionary<(TBoundNode key, string kind), object>(concurrencyLevel: 2, capacity: 10); - public TRet GetValue(TNode key, Func creator) + public TRet GetOrCreateOperationFrom(TNode key, Func creator) where TNode : TBoundNode + where TRet : IOperation { - return GetValue(key, "Root", creator); + return GetOrCreateOperationFrom(key, "Root", creator); } - public TRet GetValue(TNode key, string kind, Func creator) + public TRet GetOrCreateOperationFrom(TNode key, string kind, Func creator) + where TNode : TBoundNode + where TRet : IOperation + { + return GetValue(key, kind, creator); + } + + public ImmutableArray GetOrCreateOperationsFrom(TNode key, string kind, Func> creator) + where TNode : TBoundNode + where TRet : IOperation + { + return GetValue(key, kind, creator); + } + + private TRet GetValue(TNode key, string kind, Func creator) where TNode : TBoundNode { return (TRet)_cache.GetOrAdd((key, kind), kv => creator((TNode)kv.key)); diff --git a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb index d6a33c35e02..b7b7bacf96c 100644 --- a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb +++ b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb @@ -13,7 +13,7 @@ Namespace Microsoft.CodeAnalysis.Semantics Return Nothing End If - Return _cache.GetValue(boundNode, Function(n) CreateInternal(n)) + Return _cache.GetOrCreateOperationFrom(boundNode, Function(n) CreateInternal(n)) End Function Private Function CreateInternal(boundNode As BoundNode) As IOperation diff --git a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_Methods.vb b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_Methods.vb index 789b079d8d9..6aa3981db50 100644 --- a/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_Methods.vb +++ b/src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_Methods.vb @@ -72,7 +72,7 @@ Namespace Microsoft.CodeAnalysis.Semantics Private Function DeriveArgument(index As Integer, argument As BoundExpression, parameters As ImmutableArray(Of VisualBasic.Symbols.ParameterSymbol)) As IArgument Select Case argument.Kind Case BoundKind.ByRefArgumentWithCopyBack - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( argument, NameOf(DeriveArgument), Function(argumentValue) Dim byRefArgument = DirectCast(argumentValue, BoundByRefArgumentWithCopyBack) @@ -90,7 +90,7 @@ Namespace Microsoft.CodeAnalysis.Semantics constantValue:=Nothing) End Function) Case Else - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( argument, NameOf(DeriveArgument), Function(argumentValue) Dim lastParameterIndex = parameters.Length - 1 @@ -167,7 +167,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetSwitchStatementCases(statement As BoundSelectStatement) As ImmutableArray(Of ISwitchCase) - Dim cases = _cache.GetValue(statement, NameOf(GetSwitchStatementCases), + Dim cases = _cache.GetOrCreateOperationsFrom(statement, NameOf(GetSwitchStatementCases), Function(boundSelect) Return boundSelect.CaseBlocks.SelectAsArray( Function(boundCaseBlock) @@ -249,7 +249,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetForLoopStatementBefore(boundForToStatement As BoundForToStatement) As ImmutableArray(Of IOperation) - Dim result = _cache.GetValue( + Dim result = _cache.GetOrCreateOperationsFrom( boundForToStatement, NameOf(GetForLoopStatementBefore), Function(boundFor) Dim statements As ArrayBuilder(Of IOperation) = ArrayBuilder(Of IOperation).GetInstance() @@ -295,7 +295,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetForLoopStatementAtLoopBottom(boundForToStatement As BoundForToStatement) As ImmutableArray(Of IOperation) - Dim result = _cache.GetValue( + Dim result = _cache.GetOrCreateOperationsFrom( boundForToStatement, NameOf(GetForLoopStatementAtLoopBottom), Function(boundFor) Dim statements As ArrayBuilder(Of IOperation) = ArrayBuilder(Of IOperation).GetInstance() @@ -334,7 +334,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetForWhileUntilLoopStatmentCondition(boundForToStatement As BoundForToStatement) As IOperation - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( boundForToStatement, NameOf(GetForWhileUntilLoopStatmentCondition), Function(boundFor) As IOperation Dim operationValue = Create(boundFor.LimitValue) @@ -397,7 +397,7 @@ Namespace Microsoft.CodeAnalysis.Semantics Private Function GetBlockStatementStatements(block As BoundBlock) As ImmutableArray(Of IOperation) ' This is to filter out operations of kind None. - Dim statements = _cache.GetValue( + Dim statements = _cache.GetOrCreateOperationsFrom( block, NameOf(GetBlockStatementStatements), Function(boundBlock) Return boundBlock.Statements.Select(Function(n) Create(n)).Where(Function(s) s.Kind <> OperationKind.None).ToImmutableArray() @@ -406,7 +406,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetVariableDeclarationStatementVariables(statement As BoundDimStatement) As ImmutableArray(Of IVariableDeclaration) - Dim variables = _cache.GetValue( + Dim variables = _cache.GetOrCreateOperationsFrom( statement, NameOf(GetVariableDeclarationStatementVariables), Function(dimStatement) Dim builder = ArrayBuilder(Of IVariableDeclaration).GetInstance() @@ -427,7 +427,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetUsingStatementDeclaration(boundUsingStatement As BoundUsingStatement) As IVariableDeclarationStatement - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( boundUsingStatement, NameOf(GetUsingStatementDeclaration), Function(boundUsing) Dim declaration = If(boundUsing.ResourceList.IsDefaultOrEmpty, @@ -443,7 +443,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetAddHandlerStatementExpression(handlerStatement As BoundAddHandlerStatement) As IOperation - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( handlerStatement, NameOf(GetAddHandlerStatementExpression), Function(statement) Dim eventAccess As BoundEventAccess = TryCast(statement.EventAccess, BoundEventAccess) @@ -456,7 +456,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetRemoveStatementExpression(handlerStatement As BoundRemoveHandlerStatement) As IOperation - Return _cache.GetValue( + Return _cache.GetOrCreateOperationFrom( handlerStatement, NameOf(GetRemoveStatementExpression), Function(statement) Dim eventAccess As BoundEventAccess = TryCast(statement.EventAccess, BoundEventAccess) @@ -470,7 +470,7 @@ Namespace Microsoft.CodeAnalysis.Semantics End Function Private Function GetInterpolatedStringExpressionParts(boundInterpolatedString As BoundInterpolatedStringExpression) As ImmutableArray(Of IInterpolatedStringContent) - Return _cache.GetValue( + Return _cache.GetOrCreateOperationsFrom( boundInterpolatedString, NameOf(GetInterpolatedStringExpressionParts), Function(interpolatedString) Return interpolatedString.Contents.SelectAsArray(Function(interpolatedStringContent) CreateBoundInterpolatedStringContentOperation(interpolatedStringContent)) -- GitLab