From a2ad5c2ae0d11fdfc49c3ac5c26e5a65b56c768c Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Fri, 28 Jul 2017 16:34:59 -0700 Subject: [PATCH] Fix free in lambda analysis (#21088) Lambda analysis uses a lot of pooled data structures (as it should, since it will be redone from scratch using the exact same structure for every method) but did not properly free everything. This should properly recursively free all pooled data structures used in analysis. --- .../LambdaRewriter/LambdaRewriter.Analysis.cs | 33 +++++++++---------- .../Lowering/LambdaRewriter/LambdaRewriter.cs | 2 ++ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs index ab857ae97a5..8e3e0d936d1 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs @@ -1,8 +1,5 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Diagnostics; using System.Linq; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.PooledObjects; @@ -22,7 +19,7 @@ internal sealed partial class Analysis /// If a local function is in the set, at some point in the code it is converted to a delegate and should then not be optimized to a struct closure. /// Also contains all lambdas (as they are converted to delegates implicitly). /// - public readonly HashSet MethodsConvertedToDelegates; + public readonly PooledHashSet MethodsConvertedToDelegates; /// /// True if the method signature can be rewritten to contain ref/out parameters. @@ -36,19 +33,16 @@ internal sealed partial class Analysis /// Any scope that a method that doesn't close over. /// If a scope is in this set, don't use a struct closure. /// - public readonly HashSet ScopesThatCantBeStructs = new HashSet(); + public readonly PooledHashSet ScopesThatCantBeStructs = PooledHashSet.GetInstance(); /// /// Blocks that are positioned between a block declaring some lifted variables /// and a block that contains the lambda that lifts said variables. /// If such block itself requires a closure, then it must lift parent frame pointer into the closure /// in addition to whatever else needs to be lifted. - /// - /// NOTE: This information is computed in addition to the regular analysis of the tree and only needed for rewriting. - /// If someone only needs diagnostics or information about captures, this information is not necessary. /// needs to be called to compute this. /// - public HashSet NeedsParentFrame; + public readonly PooledHashSet NeedsParentFrame = PooledHashSet.GetInstance(); /// /// Optimized locations of lambdas. @@ -56,19 +50,17 @@ internal sealed partial class Analysis /// Lambda does not need to be placed in a frame that corresponds to its lexical scope if lambda does not reference any local state in that scope. /// It is advantageous to place lambdas higher in the scope tree, ideally in the innermost scope of all scopes that contain variables captured by a given lambda. /// Doing so reduces indirections needed when captured locals are accessed. For example locals from the innermost scope can be accessed with no indirection at all. - /// - /// NOTE: This information is computed in addition to the regular analysis of the tree and only needed for rewriting. - /// If someone only needs diagnostics or information about captures, this information is not necessary. /// needs to be called to compute this. /// - public Dictionary LambdaScopes; + public readonly SmallDictionary LambdaScopes = + new SmallDictionary(ReferenceEqualityComparer.Instance); /// /// The root of the scope tree for this method. /// public readonly Scope ScopeTree; - private Analysis(Scope scopeTree, HashSet methodsConvertedToDelegates) + private Analysis(Scope scopeTree, PooledHashSet methodsConvertedToDelegates) { ScopeTree = scopeTree; MethodsConvertedToDelegates = methodsConvertedToDelegates; @@ -76,7 +68,7 @@ private Analysis(Scope scopeTree, HashSet methodsConvertedToDelega public static Analysis Analyze(BoundNode node, MethodSymbol method, DiagnosticBag diagnostics) { - var methodsConvertedToDelegates = new HashSet(); + var methodsConvertedToDelegates = PooledHashSet.GetInstance(); var scopeTree = ScopeTreeBuilder.Build( node, method, @@ -121,9 +113,6 @@ internal void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam) { RemoveUnneededReferences(thisParam); - LambdaScopes = new Dictionary(ReferenceEqualityComparer.Instance); - NeedsParentFrame = new HashSet(); - VisitClosures(ScopeTree, (scope, closure) => { if (closure.CapturedVariables.Count > 0) @@ -354,6 +343,14 @@ Closure Helper(Scope scope) return null; } } + + public void Free() + { + MethodsConvertedToDelegates.Free(); + ScopesThatCantBeStructs.Free(); + NeedsParentFrame.Free(); + ScopeTree.Free(); + } } } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs index 855f6a9971a..b0e9ffca56a 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.cs @@ -289,6 +289,8 @@ protected override bool NeedsProxy(Symbol localOrParameter) CheckLocalsDefined(body); + analysis.Free(); + return body; } -- GitLab