Optimise DisplayClass Allocations (#32092)

The current implementation of closure conversion creates closure environments for each
new scope. This change tries to minimize the number of closure environments, and thus
closure environment allocations, when possible by merging "adjacent" closure environments.

To merge two closure environments, they have to:

1. Be captured by exactly the same set of closures
2. Have no backwards branching between the allocation of the two environments
3. Have the same environment lifetime

If so, all of the variables will be merged into a single closure environment. 

Fixes #29965

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.Tree.cs

@@ -77,6 +77,12 @@ public Scope(Scope parent, BoundNode boundNode, Closure containingClosure)
                     ContainingClosureOpt = containingClosure;
                 }
 
+                /// <summary>
+                /// Is it safe to move any of the variables declared in this scope to the parent scope, 
+                /// or would doing so change the meaning of the program?
+                /// </summary>
+                public bool CanMergeWithParent { get; internal set; } = true;
+
                 public void Free()
                 {
                     foreach (var scope in NestedScopes)
@@ -164,7 +170,7 @@ public sealed class ClosureEnvironment
                 /// <summary>
                 /// True if this environment captures a reference to a class environment
                 /// declared in a higher scope. Assigned by
-                /// <see cref="ComputeLambdaScopesAndFrameCaptures(ParameterSymbol)"/>
+                /// <see cref="ComputeLambdaScopesAndFrameCaptures()"/>
                 /// </summary>
                 public bool CapturesParent;
 
@@ -242,10 +248,26 @@ public static void VisitScopeTree(Scope treeRoot, Action<Scope> action)
             /// visits the bound tree and translates information from the bound tree about
             /// variable scope, declared variables, and variable captures into the resulting
             /// <see cref="Scope"/> tree.
+            /// 
+            /// At the same time it sets <see cref="Scope.CanMergeWithParent"/>
+            /// for each Scope. This is done by looking for <see cref="BoundGotoStatement"/>s 
+            /// and <see cref="BoundConditionalGoto"/>s that jump from a point 
+            /// after the beginning of a <see cref="Scope"/>, to a <see cref="BoundLabelStatement"/>
+            /// before the start of the scope, but after the start of <see cref="Scope.Parent"/>.
+            /// 
+            /// All loops have been converted to gotos and labels by this stage,
+            /// so we do not have to visit them to do so. Similarly all <see cref="BoundLabeledStatement"/>s
+            /// have been converted to <see cref="BoundLabelStatement"/>s, so we do not have to 
+            /// visit them.
             /// </summary>
             private class ScopeTreeBuilder : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
             {
+                /// <summary>
+                /// Do not set this directly, except when setting the root scope. 
+                /// Instead use <see cref="PopScope"/> or <see cref="CreateAndPushScope"/>.
+                /// </summary>
                 private Scope _currentScope;
+
                 /// <summary>
                 /// Null if we're not inside a closure scope, otherwise the nearest closure scope
                 /// </summary>
@@ -284,6 +306,21 @@ private class ScopeTreeBuilder : BoundTreeWalkerWithStackGuardWithoutRecursionOn
                 private readonly HashSet<MethodSymbol> _methodsConvertedToDelegates;
                 private readonly DiagnosticBag _diagnostics;
 
+                /// <summary>
+                /// For every label visited so far, this dictionary maps to a list of all scopes either visited so far, or currently being visited,
+                /// that are both after the label, and are on the same level of the scope tree as the label.
+                /// </summary>
+                private readonly PooledDictionary<LabelSymbol, ArrayBuilder<Scope>> _scopesAfterLabel = PooledDictionary<LabelSymbol, ArrayBuilder<Scope>>.GetInstance();
+
+                /// <summary>
+                /// Contains a list of the labels visited so far for each scope. 
+                /// The outer ArrayBuilder is a stack representing the chain of scopes from the root scope to the current scope,
+                /// and for each item on the stack, the ArrayBuilder is the list of the labels visited so far for the scope.
+                /// 
+                /// Used by <see cref="CreateAndPushScope"/> to determine which labels a new child scope appears after.
+                /// </summary>
+                private readonly ArrayBuilder<ArrayBuilder<LabelSymbol>> _labelsInScope = ArrayBuilder<ArrayBuilder<LabelSymbol>>.GetInstance();
+
                 private ScopeTreeBuilder(
                     Scope rootScope,
                     MethodSymbol topLevelMethod,
@@ -296,6 +333,7 @@ private class ScopeTreeBuilder : BoundTreeWalkerWithStackGuardWithoutRecursionOn
                     Debug.Assert(diagnostics != null);
 
                     _currentScope = rootScope;
+                    _labelsInScope.Push(ArrayBuilder<LabelSymbol>.GetInstance());
                     _topLevelMethod = topLevelMethod;
                     _methodsConvertedToDelegates = methodsConvertedToDelegates;
                     _diagnostics = diagnostics;
@@ -332,6 +370,19 @@ private void Build()
                     }
 
                     Visit(_currentScope.BoundNode);
+
+                    // Clean Up Resources
+
+                    foreach (var scopes in _scopesAfterLabel.Values)
+                    {
+                        scopes.Free();
+                    }
+                    _scopesAfterLabel.Free();
+
+                    Debug.Assert(_labelsInScope.Count == 1);
+                    var labels = _labelsInScope.Pop();
+                    labels.Free();
+                    _labelsInScope.Free();
                 }
 
                 public override BoundNode VisitMethodGroup(BoundMethodGroup node)
@@ -340,27 +391,27 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node)
                 public override BoundNode VisitBlock(BoundBlock node)
                 {
                     var oldScope = _currentScope;
-                    _currentScope = CreateOrReuseScope(node, node.Locals);
+                    PushOrReuseScope(node, node.Locals);
                     var result = base.VisitBlock(node);
-                    _currentScope = oldScope;
+                    PopScope(oldScope);
                     return result;
                 }
 
                 public override BoundNode VisitCatchBlock(BoundCatchBlock node)
                 {
                     var oldScope = _currentScope;
-                    _currentScope = CreateOrReuseScope(node, node.Locals);
+                    PushOrReuseScope(node, node.Locals);
                     var result = base.VisitCatchBlock(node);
-                    _currentScope = oldScope;
+                    PopScope(oldScope);
                     return result;
                 }
 
                 public override BoundNode VisitSequence(BoundSequence node)
                 {
                     var oldScope = _currentScope;
-                    _currentScope = CreateOrReuseScope(node, node.Locals);
+                    PushOrReuseScope(node, node.Locals);
                     var result = base.VisitSequence(node);
-                    _currentScope = oldScope;
+                    PopScope(oldScope);
                     return result;
                 }
 
@@ -442,6 +493,52 @@ public override BoundNode VisitThisReference(BoundThisReference node)
                     return base.VisitThisReference(node);
                 }
 
+                public override BoundNode VisitLabelStatement(BoundLabelStatement node)
+                {
+                    _labelsInScope.Peek().Add(node.Label);
+                    _scopesAfterLabel.Add(node.Label, ArrayBuilder<Scope>.GetInstance());
+                    return base.VisitLabelStatement(node);
+                }
+
+                public override BoundNode VisitGotoStatement(BoundGotoStatement node)
+                {
+                    CheckCanMergeWithParent(node.Label);
+
+                    return base.VisitGotoStatement(node);
+                }
+
+                public override BoundNode VisitConditionalGoto(BoundConditionalGoto node)
+                {
+                    CheckCanMergeWithParent(node.Label);
+
+                    return base.VisitConditionalGoto(node);
+                }
+
+                /// <summary>
+                /// This is where we calculate <see cref="Scope.CanMergeWithParent"/>.
+                /// <see cref="Scope.CanMergeWithParent"/> is always true unless we jump from after 
+                /// the beginning of a scope, to a point in between the beginning of the parent scope, and the beginning of the scope
+                /// </summary>
+                /// <param name="jumpTarget"></param>
+                private void CheckCanMergeWithParent(LabelSymbol jumpTarget)
+                {
+                    // since forward jumps can never effect Scope.SemanticallySafeToMergeIntoParent
+                    // if we have not yet seen the jumpTarget, this is a forward jump, and can be ignored
+                    if (_scopesAfterLabel.TryGetValue(jumpTarget, out var scopesAfterLabel))
+                    {
+                        foreach (var scope in scopesAfterLabel)
+                        {
+                            // this jump goes from a point after the beginning of the scope (as we have already visited or started visiting the scope), 
+                            // to a point in between the beginning of the parent scope, and the beginning of the scope, so it is not safe to move
+                            // variables in the scope to the parent scope.
+                            scope.CanMergeWithParent = false;
+                        }
+
+                        // Prevent us repeating this process for all scopes if another jumps goes to the same label
+                        scopesAfterLabel.Clear();
+                    }
+                }
+
                 private BoundNode VisitClosure(MethodSymbol closureSymbol, BoundBlock body)
                 {
                     Debug.Assert((object)closureSymbol != null);
@@ -455,7 +552,7 @@ private BoundNode VisitClosure(MethodSymbol closureSymbol, BoundBlock body)
                     _currentClosure = closure;
 
                     var oldScope = _currentScope;
-                    _currentScope = CreateNestedScope(body, _currentScope, _currentClosure);
+                    CreateAndPushScope(body);
 
                     // For the purposes of scoping, parameters live in the same scope as the
                     // closure block. Expression tree variables are free variables for the
@@ -466,7 +563,7 @@ private BoundNode VisitClosure(MethodSymbol closureSymbol, BoundBlock body)
                         ? base.VisitBlock(body)
                         : VisitBlock(body);
 
-                    _currentScope = oldScope;
+                    PopScope(oldScope);
                     _currentClosure = oldClosure;
                     return result;
                 }
@@ -570,38 +667,85 @@ private void AddDiagnosticIfRestrictedType(Symbol capturedVariable, SyntaxNode s
                 }
 
                 /// <summary>
-                /// Create a new nested scope under the current scope, or reuse the current
-                /// scope if there's no change in the bound node for the nested scope.
+                /// Create a new nested scope under the current scope, and replace <see cref="_currentScope"/> with the new scope,
+                /// or reuse the current scope if there's no change in the bound node for the nested scope.
                 /// Records the given locals as declared in the aforementioned scope.
                 /// </summary>
-                private Scope CreateOrReuseScope<TSymbol>(BoundNode node, ImmutableArray<TSymbol> locals)
+                private void PushOrReuseScope<TSymbol>(BoundNode node, ImmutableArray<TSymbol> locals)
                     where TSymbol : Symbol
                 {
-                    Scope scope;
-                    if (locals.IsEmpty || _currentScope.BoundNode == node)
+                    // We should never create a new scope with the same bound
+                    // node. We can get into this situation for methods and
+                    // closures where a new scope is created to add parameters
+                    // and a new scope would be created for the method block,
+                    // despite the fact that they should be the same scope.
+                    if (!locals.IsEmpty && _currentScope.BoundNode != node)
                     {
-                        // We should never create a new scope with the same bound
-                        // node. We can get into this situation for methods and
-                        // closures where a new scope is created to add parameters
-                        // and a new scope would be created for the method block,
-                        // despite the fact that they should be the same scope.
-                        scope = _currentScope;
+                        CreateAndPushScope(node);
                     }
-                    else
+
+                    DeclareLocals(_currentScope, locals);
+                }
+
+                /// <summary>
+                /// Creates a new nested scope which is a child of <see cref="_currentScope"/>,
+                /// and replaces <see cref="_currentScope"/> with the new scope
+                /// </summary>
+                /// <param name="node"></param>
+                private void CreateAndPushScope(BoundNode node)
+                {
+                    var scope = CreateNestedScope(_currentScope, _currentClosure);
+
+                    foreach (var label in _labelsInScope.Peek())
+                    {
+                        _scopesAfterLabel[label].Add(scope);
+                    }
+
+                    _labelsInScope.Push(ArrayBuilder<LabelSymbol>.GetInstance());
+
+                    _currentScope = scope;
+
+                    Scope CreateNestedScope(Scope parentScope, Closure currentClosure)
                     {
-                        scope = CreateNestedScope(node, _currentScope, _currentClosure);
+                        Debug.Assert(parentScope.BoundNode != node);
+
+                        var newScope = new Scope(parentScope, node, currentClosure);
+                        parentScope.NestedScopes.Add(newScope);
+                        return newScope;
                     }
-                    DeclareLocals(scope, locals);
-                    return scope;
                 }
 
-                private static Scope CreateNestedScope(BoundNode node, Scope parentScope, Closure currentClosure)
+                /// <summary>
+                /// Requires that scope is either the same as <see cref="_currentScope"/>,
+                /// or is the <see cref="Scope.Parent"/> of <see cref="_currentScope"/>.
+                /// Returns imediately in the first case,
+                /// Replaces <see cref="_currentScope"/> with scope in the second.
+                /// </summary>
+                /// <param name="scope"></param>
+                private void PopScope(Scope scope)
                 {
-                    Debug.Assert(parentScope.BoundNode != node);
+                    if (scope == _currentScope)
+                    {
+                        return;
+                    }
+
+                    Debug.Assert(scope == _currentScope.Parent, $"{nameof(scope)} must be {nameof(_currentScope)} or {nameof(_currentScope)}.{nameof(_currentScope.Parent)}");
+
+                    // Since it is forbidden to jump into a scope, 
+                    // we can forget all information we have about labels in the child scope
+
+                    var labels = _labelsInScope.Pop();
+
+                    foreach (var label in labels)
+                    {
+                        var scopes = _scopesAfterLabel[label];
+                        scopes.Free();
+                        _scopesAfterLabel.Remove(label);
+                    }
+
+                    labels.Free();
 
-                    var newScope = new Scope(parentScope, node, currentClosure);
-                    parentScope.NestedScopes.Add(newScope);
-                    return newScope;
+                    _currentScope = _currentScope.Parent;
                 }
 
                 private void DeclareLocals<TSymbol>(Scope scope, ImmutableArray<TSymbol> locals, bool declareAsFree = false)

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/LambdaRewriter.Analysis.cs

 // 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.CodeGen;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
@@ -40,7 +40,6 @@ internal sealed partial class Analysis
 
             private readonly MethodSymbol _topLevelMethod;
             private readonly int _topLevelMethodOrdinal;
-            private readonly MethodSymbol _substitutedSourceMethod;
             private readonly VariableSlotAllocator _slotAllocatorOpt;
             private readonly TypeCompilationState _compilationState;
 
@@ -49,7 +48,6 @@ internal sealed partial class Analysis
                 PooledHashSet<MethodSymbol> methodsConvertedToDelegates,
                 MethodSymbol topLevelMethod,
                 int topLevelMethodOrdinal,
-                MethodSymbol substitutedSourceMethod,
                 VariableSlotAllocator slotAllocatorOpt,
                 TypeCompilationState compilationState)
             {
@@ -57,7 +55,6 @@ internal sealed partial class Analysis
                 MethodsConvertedToDelegates = methodsConvertedToDelegates;
                 _topLevelMethod = topLevelMethod;
                 _topLevelMethodOrdinal = topLevelMethodOrdinal;
-                _substitutedSourceMethod = substitutedSourceMethod;
                 _slotAllocatorOpt = slotAllocatorOpt;
                 _compilationState = compilationState;
             }
@@ -85,12 +82,16 @@ internal sealed partial class Analysis
                     methodsConvertedToDelegates,
                     method,
                     topLevelMethodOrdinal,
-                    substitutedSourceMethod,
                     slotAllocatorOpt,
                     compilationState);
 
                 analysis.MakeAndAssignEnvironments();
-                analysis.ComputeLambdaScopesAndFrameCaptures(method.ThisParameter);
+                analysis.ComputeLambdaScopesAndFrameCaptures();
+                if (compilationState.Compilation.Options.OptimizationLevel == OptimizationLevel.Release)
+                {
+                    // This can affect when a variable is in scope whilst debugging, so only do this in release mode.
+                    analysis.MergeEnvironments();
+                }
                 analysis.InlineThisOnlyEnvironments();
                 return analysis;
             }
@@ -134,7 +135,7 @@ private static BoundNode FindNodeToAnalyze(BoundNode node)
             /// the number of indirections we may have to traverse to access captured
             /// variables.
             /// </summary>
-            private void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
+            private void ComputeLambdaScopesAndFrameCaptures()
             {
                 VisitClosures(ScopeTree, (scope, closure) =>
                 {
@@ -163,6 +164,7 @@ private void ComputeLambdaScopesAndFrameCaptures(ParameterSymbol thisParam)
 
                         // Now we need to walk up the scopes to find environment captures
                         var oldEnv = curScope?.DeclaredEnvironments[0];
+                        curScope = curScope?.Parent;
                         while (curScope != null)
                         {
                             if (capturedEnvs.Count == 0)
@@ -342,6 +344,177 @@ private void MakeAndAssignEnvironments()
                 });
             }
 
+            /// <summary>
+            /// Calculates all closures which directly or indirectly capture a scopes variables.
+            /// </summary>
+            /// <returns></returns>
+            private PooledDictionary<Scope, PooledHashSet<Closure>> CalculateClosuresCapturingScopeVariables()
+            {
+                var closuresCapturingScopeVariables = PooledDictionary<Scope, PooledHashSet<Closure>>.GetInstance();
+
+                // calculate closures which directly capture a scope
+
+                var environmentsToScopes = PooledDictionary<ClosureEnvironment, Scope>.GetInstance();
+
+                VisitScopeTree(ScopeTree, scope =>
+                {
+                    if (scope.DeclaredEnvironments.Count > 0)
+                    {
+                        // Right now we only create one environment per scope
+                        Debug.Assert(scope.DeclaredEnvironments.Count == 1);
+
+                        closuresCapturingScopeVariables[scope] = PooledHashSet<Closure>.GetInstance();
+                        environmentsToScopes[scope.DeclaredEnvironments[0]] = scope;
+                    }
+
+                    foreach (var closure in scope.Closures)
+                    {
+                        foreach (var env in closure.CapturedEnvironments)
+                        {
+                            // A closure should only ever capture a scope which is an ancestor of its own,
+                            // which we should have already visited
+                            Debug.Assert(environmentsToScopes.ContainsKey(env));
+
+                            closuresCapturingScopeVariables[environmentsToScopes[env]].Add(closure);
+                        }
+                    }
+                });
+
+                environmentsToScopes.Free();
+
+                // if a closure captures a scope, which captures its parent, then the closure also captures the parents scope.
+                // we update closuresCapturingScopeVariables to reflect this.
+                foreach (var (scope, capturingClosures) in closuresCapturingScopeVariables)
+                {
+                    if (scope.DeclaredEnvironments.Count == 0)
+                        continue;
+
+                    var currentScope = scope;
+                    while (currentScope.DeclaredEnvironments.Count == 0 || currentScope.DeclaredEnvironments[0].CapturesParent)
+                    {
+                        currentScope = currentScope.Parent;
+
+                        if (currentScope == null)
+                        {
+                            throw ExceptionUtilities.Unreachable;
+                        }
+
+                        if (currentScope.DeclaredEnvironments.Count == 0 ||
+                            currentScope.DeclaredEnvironments[0].IsStruct)
+                        {
+                            continue;
+                        }
+
+                        closuresCapturingScopeVariables[currentScope].AddAll(capturingClosures);
+                    }
+                }
+
+                return closuresCapturingScopeVariables;
+            }
+
+            /// <summary>
+            /// Must be called only after <see cref="MakeAndAssignEnvironments"/> and <see cref="ComputeLambdaScopesAndFrameCaptures"/>.
+            /// 
+            /// In order to reduce allocations, merge environments into a parent environment when it is safe to do so.
+            /// This must be done whilst preserving semantics.
+            /// 
+            /// We also have to make sure not to extend the life of any variable.
+            /// This means that we can only merge an environment into its parent if exactly the same closures directly or indirectly reference both environments.
+            /// </summary>
+            private void MergeEnvironments()
+            {
+                var closuresCapturingScopeVariables = CalculateClosuresCapturingScopeVariables();
+
+                // now we merge environments into their parent environments if it is safe to do so
+                foreach (var (scope, closuresCapturingScope) in closuresCapturingScopeVariables)
+                {
+                    if (closuresCapturingScope.Count == 0)
+                        continue;
+
+                    var scopeEnv = scope.DeclaredEnvironments[0];
+
+                    // structs don't allocate, so no point merging them
+                    if (scopeEnv.IsStruct)
+                        continue;
+
+                    var bestScope = scope;
+                    var currentScope = scope;
+
+                    // Walk up the scope tree, checking at each point if it is:
+                    // a) semantically safe to merge the scope's environment into it's parent scope's environment
+                    // b) doing so would not change GC behaviour
+                    // Once either of these conditions fails, we merge into the closure environment furthest up the scope tree we've found so far
+                    while (currentScope.Parent != null)
+                    {
+                        if (!currentScope.CanMergeWithParent)
+                            break;
+
+                        var parentScope = currentScope.Parent;
+
+                        // Right now we only create one environment per scope
+                        Debug.Assert(parentScope.DeclaredEnvironments.Count < 2);
+
+                        // we skip any scopes which do not have any captured variables, and try to merge into the parent scope instead.
+                        // We also skip any struct environments as they don't allocate, so no point merging them
+                        if (parentScope.DeclaredEnvironments.Count == 0 || parentScope.DeclaredEnvironments[0].IsStruct)
+                        {
+                            currentScope = parentScope;
+                            continue;
+                        }
+
+                        var closuresCapturingParentScope = closuresCapturingScopeVariables[parentScope];
+
+                        // if more closures reference one scope's environments than the other scope's environments,
+                        // then merging the two environments would increase the number of objects referencing some variables, 
+                        // which may prevent the variables being garbage collected.
+                        if (!closuresCapturingParentScope.SetEquals(closuresCapturingScope))
+                            break;
+
+                        bestScope = parentScope;
+
+                        currentScope = parentScope;
+                    }
+
+
+                    if (bestScope == scope) // no better scope was found, so continue
+                        continue;
+
+                    // do the actual work of merging the closure environments
+
+                    var targetEnv = bestScope.DeclaredEnvironments[0];
+
+                    foreach (var variable in scopeEnv.CapturedVariables)
+                    {
+                        targetEnv.CapturedVariables.Add(variable);
+                    }
+
+                    scope.DeclaredEnvironments.Clear();
+
+                    foreach (var closure in closuresCapturingScope)
+                    {
+                        closure.CapturedEnvironments.Remove(scopeEnv);
+
+                        if (!closure.CapturedEnvironments.Contains(targetEnv))
+                        {
+                            closure.CapturedEnvironments.Add(targetEnv);
+                        }
+
+                        if (closure.ContainingEnvironmentOpt == scopeEnv)
+                        {
+                            closure.ContainingEnvironmentOpt = targetEnv;
+                        }
+                    }
+                }
+
+                // cleanup
+                foreach (var set in closuresCapturingScopeVariables.Values)
+                {
+                    set.Free();
+                }
+
+                closuresCapturingScopeVariables.Free();
+            }
+
             internal DebugId GetTopLevelMethodId()
             {
                 return _slotAllocatorOpt?.MethodId ?? new DebugId(_topLevelMethodOrdinal, _compilationState.ModuleBuilderOpt.CurrentGenerationOrdinal);

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenClosureLambdaTests.cs

@@ -1111,11 +1111,10 @@ static void Main()
             var verifier = CompileAndVerify(source, expectedOutput: "pass_xy");
             verifier.VerifyIL("Program.<>c__DisplayClass1_0<T>.<F>b__0", @"
 {
-  // Code size      131 (0x83)
+  // Code size      113 (0x71)
   .maxstack  3
   .locals init (Program.<>c__DisplayClass1_1<T> V_0, //CS$<>8__locals0
-                Program.<>c__DisplayClass1_2<T> V_1, //CS$<>8__locals1
-                T V_2)
+                T V_1)
   IL_0000:  newobj     ""Program.<>c__DisplayClass1_1<T>..ctor()""
   IL_0005:  stloc.0
   IL_0006:  ldloc.0
@@ -1137,38 +1136,32 @@ .maxstack  3
     IL_0029:  brtrue.s   IL_002f
     IL_002b:  pop
     IL_002c:  ldc.i4.0
-    IL_002d:  br.s       IL_005d
+    IL_002d:  br.s       IL_0050
     IL_002f:  unbox.any  ""T""
-    IL_0034:  newobj     ""Program.<>c__DisplayClass1_2<T>..ctor()""
-    IL_0039:  stloc.1
-    IL_003a:  ldloc.1
-    IL_003b:  ldloc.0
-    IL_003c:  stfld      ""Program.<>c__DisplayClass1_1<T> Program.<>c__DisplayClass1_2<T>.CS$<>8__locals2""
-    IL_0041:  stloc.2
-    IL_0042:  ldloc.1
-    IL_0043:  ldloc.2
-    IL_0044:  stfld      ""T Program.<>c__DisplayClass1_2<T>.e""
-    IL_0049:  ldloc.1
-    IL_004a:  ldftn      ""bool Program.<>c__DisplayClass1_2<T>.<F>b__1()""
-    IL_0050:  newobj     ""System.Func<bool>..ctor(object, System.IntPtr)""
-    IL_0055:  callvirt   ""bool System.Func<bool>.Invoke()""
-    IL_005a:  ldc.i4.0
-    IL_005b:  cgt.un
-    IL_005d:  endfilter
+    IL_0034:  stloc.1
+    IL_0035:  ldloc.0
+    IL_0036:  ldloc.1
+    IL_0037:  stfld      ""T Program.<>c__DisplayClass1_1<T>.e""
+    IL_003c:  ldloc.0
+    IL_003d:  ldftn      ""bool Program.<>c__DisplayClass1_1<T>.<F>b__1()""
+    IL_0043:  newobj     ""System.Func<bool>..ctor(object, System.IntPtr)""
+    IL_0048:  callvirt   ""bool System.Func<bool>.Invoke()""
+    IL_004d:  ldc.i4.0
+    IL_004e:  cgt.un
+    IL_0050:  endfilter
   }  // end filter
   {  // handler
-    IL_005f:  pop
-    IL_0060:  ldstr      ""pass_""
-    IL_0065:  ldarg.0
-    IL_0066:  ldfld      ""string Program.<>c__DisplayClass1_0<T>.x""
-    IL_006b:  ldloc.1
-    IL_006c:  ldfld      ""Program.<>c__DisplayClass1_1<T> Program.<>c__DisplayClass1_2<T>.CS$<>8__locals2""
-    IL_0071:  ldfld      ""string Program.<>c__DisplayClass1_1<T>.y""
-    IL_0076:  call       ""string string.Concat(string, string, string)""
-    IL_007b:  call       ""void System.Console.Write(string)""
-    IL_0080:  leave.s    IL_0082
+    IL_0052:  pop
+    IL_0053:  ldstr      ""pass_""
+    IL_0058:  ldarg.0
+    IL_0059:  ldfld      ""string Program.<>c__DisplayClass1_0<T>.x""
+    IL_005e:  ldloc.0
+    IL_005f:  ldfld      ""string Program.<>c__DisplayClass1_1<T>.y""
+    IL_0064:  call       ""string string.Concat(string, string, string)""
+    IL_0069:  call       ""void System.Console.Write(string)""
+    IL_006e:  leave.s    IL_0070
   }
-  IL_0082:  ret
+  IL_0070:  ret
 }
 ");
         }
@@ -3481,18 +3474,18 @@ public void Test()
     }
 }";
             CompileAndVerify(source, expectedOutput: "13").
-            VerifyIL("Program.c1.<>c__DisplayClass1_1.<Test>b__2",
+            VerifyIL("Program.c1.<>c__DisplayClass1_0.<Test>b__2",
 @"{
   // Code size       31 (0x1f)
   .maxstack  3
-  IL_0000:  newobj     ""Program.c1.<>c__DisplayClass1_2..ctor()""
+  IL_0000:  newobj     ""Program.c1.<>c__DisplayClass1_1..ctor()""
   IL_0005:  dup
   IL_0006:  ldarg.0
-  IL_0007:  stfld      ""Program.c1.<>c__DisplayClass1_1 Program.c1.<>c__DisplayClass1_2.CS$<>8__locals2""
+  IL_0007:  stfld      ""Program.c1.<>c__DisplayClass1_0 Program.c1.<>c__DisplayClass1_1.CS$<>8__locals1""
   IL_000c:  dup
   IL_000d:  ldarg.1
-  IL_000e:  stfld      ""int Program.c1.<>c__DisplayClass1_2.z""
-  IL_0013:  ldftn      ""int Program.c1.<>c__DisplayClass1_2.<Test>b__3(int)""
+  IL_000e:  stfld      ""int Program.c1.<>c__DisplayClass1_1.z""
+  IL_0013:  ldftn      ""int Program.c1.<>c__DisplayClass1_1.<Test>b__3(int)""
   IL_0019:  newobj     ""System.Func<int, int>..ctor(object, System.IntPtr)""
   IL_001e:  ret
 }");
@@ -3782,8 +3775,7 @@ .maxstack  2
         [Fact]
         public void ParentFrame05()
         {
-            // IMPORTANT: this code should not initialize any fields in Program.c1.<>c__DisplayClass0 except "a"
-            //            Program.c1.<>c__DisplayClass0 should not capture any frame pointers.
+            // IMPORTANT: Program.c1.<>c__DisplayClass1_0 should not capture any frame pointers.
 
             string source = @"
 using System;
@@ -3829,8 +3821,8 @@ private bool T()
             CompileAndVerify(source, expectedOutput: "6").
             VerifyIL("Program.c1.Test",
 @"{
-  // Code size       96 (0x60)
-  .maxstack  3
+  // Code size       85 (0x55)
+  .maxstack  2
   .locals init (System.Func<int> V_0, //ff
                 System.Func<int> V_1, //aa
                 Program.c1.<>c__DisplayClass1_0 V_2) //CS$<>8__locals0
@@ -3840,7 +3832,7 @@ .maxstack  3
   IL_0003:  stloc.1
   IL_0004:  ldarg.0
   IL_0005:  call       ""bool Program.c1.T()""
-  IL_000a:  brfalse.s  IL_004d
+  IL_000a:  brfalse.s  IL_0042
   IL_000c:  newobj     ""Program.c1.<>c__DisplayClass1_0..ctor()""
   IL_0011:  stloc.2
   IL_0012:  ldloc.2
@@ -3848,28 +3840,25 @@ .maxstack  3
   IL_0014:  stfld      ""int Program.c1.<>c__DisplayClass1_0.a""
   IL_0019:  ldarg.0
   IL_001a:  call       ""bool Program.c1.T()""
-  IL_001f:  brfalse.s  IL_004d
-  IL_0021:  newobj     ""Program.c1.<>c__DisplayClass1_1..ctor()""
-  IL_0026:  dup
-  IL_0027:  ldloc.2
-  IL_0028:  stfld      ""Program.c1.<>c__DisplayClass1_0 Program.c1.<>c__DisplayClass1_1.CS$<>8__locals1""
-  IL_002d:  dup
-  IL_002e:  ldc.i4.4
-  IL_002f:  stfld      ""int Program.c1.<>c__DisplayClass1_1.b""
-  IL_0034:  ldftn      ""int Program.c1.<>c__DisplayClass1_1.<Test>b__0()""
-  IL_003a:  newobj     ""System.Func<int>..ctor(object, System.IntPtr)""
-  IL_003f:  stloc.0
-  IL_0040:  ldarg.0
-  IL_0041:  ldftn      ""int Program.c1.<Test>b__1_1()""
-  IL_0047:  newobj     ""System.Func<int>..ctor(object, System.IntPtr)""
-  IL_004c:  stloc.1
-  IL_004d:  ldloc.0
-  IL_004e:  callvirt   ""int System.Func<int>.Invoke()""
-  IL_0053:  ldloc.1
-  IL_0054:  callvirt   ""int System.Func<int>.Invoke()""
-  IL_0059:  add
-  IL_005a:  call       ""void System.Console.WriteLine(int)""
-  IL_005f:  ret
+  IL_001f:  brfalse.s  IL_0042
+  IL_0021:  ldloc.2
+  IL_0022:  ldc.i4.4
+  IL_0023:  stfld      ""int Program.c1.<>c__DisplayClass1_0.b""
+  IL_0028:  ldloc.2
+  IL_0029:  ldftn      ""int Program.c1.<>c__DisplayClass1_0.<Test>b__0()""
+  IL_002f:  newobj     ""System.Func<int>..ctor(object, System.IntPtr)""
+  IL_0034:  stloc.0
+  IL_0035:  ldarg.0
+  IL_0036:  ldftn      ""int Program.c1.<Test>b__1_1()""
+  IL_003c:  newobj     ""System.Func<int>..ctor(object, System.IntPtr)""
+  IL_0041:  stloc.1
+  IL_0042:  ldloc.0
+  IL_0043:  callvirt   ""int System.Func<int>.Invoke()""
+  IL_0048:  ldloc.1
+  IL_0049:  callvirt   ""int System.Func<int>.Invoke()""
+  IL_004e:  add
+  IL_004f:  call       ""void System.Console.WriteLine(int)""
+  IL_0054:  ret
 }");
         }
 

src/Dependencies/PooledObjects/ArrayBuilder.cs

@@ -176,6 +176,11 @@ public int FindIndex(int startIndex, int count, Predicate<T> match)
             return -1;
         }
 
+        public bool Remove(T element)
+        {
+            return _builder.Remove(element);
+        }
+
         public void RemoveAt(int index)
         {
             _builder.RemoveAt(index);

src/Test/Utilities/Portable/CompilationVerifier.cs

@@ -152,6 +152,15 @@ void listMethodsInType(ICSharpCode.Decompiler.TypeSystem.ITypeDefinition type, D
             }
         }
 
+        /// <summary>
+		/// Asserts that the emitted IL for a type is the same as the expected IL.
+		/// Many core library types are in different assemblies on .Net Framework, and .Net Core.
+		/// Therefore this test is likely to fail unless you  only run it only only on one of these frameworks,
+		/// or you run it on both, but provide a different expected output string for each.
+		/// See <see cref="ExecutionConditionUtil"/>.
+		/// </summary>
+		/// <param name="typeName">The non-fully-qualified name of the type</param>
+		/// <param name="expected">The expected IL</param>
         public void VerifyTypeIL(string typeName, string expected)
         {
             var output = new ICSharpCode.Decompiler.PlainTextOutput();