Merge pull request #5732 from DustinCampbell/issue-3313

Initial work to clean up lifetime of keyed Code Model elements

src/VisualStudio/CSharp/Impl/CodeModel/CSharpCodeModelService.cs

@@ -483,7 +483,7 @@ public override string AssemblyAttributeString
         }
 
         /// <summary>
-        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.CreateCodeElement{T}(SyntaxNode)"/>
+        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.GetOrCreateCodeElement{T}(SyntaxNode)"/>
         /// </summary>
         public override EnvDTE.CodeElement CreateInternalCodeElement(
             CodeModelState state,

src/VisualStudio/Core/Def/Implementation/Interop/CleanableWeakComHandleTable.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;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
+using Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
+
+namespace Microsoft.VisualStudio.LanguageServices.Implementation.Interop
+{
+    /// <summary>
+    /// Special collection for storing a table of COM objects weakly that provides
+    /// logic for cleaning up dead references in a time-sliced way. Public members of this
+    /// collection are affinitized to the foreground thread.
+    /// </summary>
+    internal class CleanableWeakComHandleTable<TKey, TValue> : ForegroundThreadAffinitizedObject
+        where TValue : class
+    {
+        private const int DefaultCleanUpThreshold = 25;
+        private static readonly TimeSpan s_defaultCleanUpTimeSlice = TimeSpan.FromMilliseconds(15);
+
+        private readonly Dictionary<TKey, WeakComHandle<TValue, TValue>> _table;
+        private readonly HashSet<TKey> _deadKeySet;
+
+        /// <summary>
+        /// The upper limit of items that the collection will store before clean up is recommended.
+        /// </summary>
+        public int CleanUpThreshold { get; }
+
+        /// <summary>
+        /// The amount of time that can pass during clean up it returns.
+        /// </summary>
+        public TimeSpan CleanUpTimeSlice { get; }
+
+        private int _itemsAddedSinceLastCleanUp;
+        private bool _needsCleanUp;
+        private IEnumerator<KeyValuePair<TKey, WeakComHandle<TValue, TValue>>> _cleanUpEnumerator;
+
+        private enum CleanUpState { Initial, CollectingDeadKeys, RemovingDeadKeys }
+        private CleanUpState _cleanUpState;
+
+        public bool NeedsCleanUp => _needsCleanUp;
+
+        public CleanableWeakComHandleTable(int? cleanUpThreshold = null, TimeSpan? cleanUpTimeSlice = null)
+        {
+            _table = new Dictionary<TKey, WeakComHandle<TValue, TValue>>();
+            _deadKeySet = new HashSet<TKey>();
+
+            CleanUpThreshold = cleanUpThreshold ?? DefaultCleanUpThreshold;
+            CleanUpTimeSlice = cleanUpTimeSlice ?? s_defaultCleanUpTimeSlice;
+            _cleanUpState = CleanUpState.Initial;
+        }
+
+        private void InvalidateEnumerator()
+        {
+            if (_cleanUpEnumerator != null)
+            {
+                _cleanUpEnumerator.Dispose();
+                _cleanUpEnumerator = null;
+            }
+        }
+
+        private bool CollectDeadKeys(TimeSlice timeSlice)
+        {
+            Debug.Assert(_cleanUpState == CleanUpState.CollectingDeadKeys);
+            Debug.Assert(_cleanUpEnumerator != null);
+
+            while (_cleanUpEnumerator.MoveNext())
+            {
+                var pair = _cleanUpEnumerator.Current;
+
+                if (!pair.Value.IsAlive())
+                {
+                    _deadKeySet.Add(pair.Key);
+                }
+
+                if (timeSlice.IsOver)
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        private bool RemoveDeadKeys(TimeSlice timeSlice)
+        {
+            Debug.Assert(_cleanUpEnumerator == null);
+
+            while (_deadKeySet.Count > 0)
+            {
+                var key = _deadKeySet.First();
+
+                _deadKeySet.Remove(key);
+
+                Debug.Assert(_table.ContainsKey(key), "Key not found in table.");
+                _table.Remove(key);
+
+                if (timeSlice.IsOver)
+                {
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        /// <summary>
+        /// Cleans up references to dead objects in the table. This operation will return if it takes
+        /// longer than <see cref="CleanUpTimeSlice"/>. Calling <see cref="CleanUpDeadObjects"/> further
+        /// times will continue the process.
+        /// </summary>
+        public void CleanUpDeadObjects()
+        {
+            this.AssertIsForeground();
+
+            if (!_needsCleanUp)
+            {
+                return;
+            }
+
+            var timeSlice = new TimeSlice(CleanUpTimeSlice);
+
+            if (_cleanUpState == CleanUpState.Initial)
+            {
+                _cleanUpEnumerator = _table.GetEnumerator();
+                _cleanUpState = CleanUpState.CollectingDeadKeys;
+            }
+
+            if (_cleanUpState == CleanUpState.CollectingDeadKeys)
+            {
+                if (_cleanUpEnumerator == null)
+                {
+                    // The enumerator got reset while we were collecting dead keys.
+                    // Go ahead and remove the dead keys that were already collected before
+                    // collecting more.
+                    if (!RemoveDeadKeys(timeSlice))
+                    {
+                        return;
+                    }
+
+                    _cleanUpEnumerator = _table.GetEnumerator();
+                }
+
+                if (!CollectDeadKeys(timeSlice))
+                {
+                    return;
+                }
+
+                InvalidateEnumerator();
+                _cleanUpState = CleanUpState.RemovingDeadKeys;
+            }
+
+            if (_cleanUpState == CleanUpState.RemovingDeadKeys)
+            {
+                if (!RemoveDeadKeys(timeSlice))
+                {
+                    return;
+                }
+
+                _cleanUpState = CleanUpState.Initial;
+            }
+
+            _needsCleanUp = false;
+        }
+
+        public void Add(TKey key, TValue value)
+        {
+            this.AssertIsForeground();
+
+            if (_table.ContainsKey(key))
+            {
+                throw new InvalidOperationException("Key already exists in table.");
+            }
+
+            _itemsAddedSinceLastCleanUp++;
+            if (_itemsAddedSinceLastCleanUp >= CleanUpThreshold)
+            {
+                _needsCleanUp = true;
+                _itemsAddedSinceLastCleanUp = 0;
+            }
+
+            InvalidateEnumerator();
+
+            this._table.Add(key, new WeakComHandle<TValue, TValue>(value));
+        }
+
+        public TValue Remove(TKey key)
+        {
+            this.AssertIsForeground();
+
+            InvalidateEnumerator();
+
+            if (_deadKeySet.Contains(key))
+            {
+                _deadKeySet.Remove(key);
+            }
+
+            TValue value;
+            if (TryGetValue(key, out value))
+            {
+                _table.Remove(key);
+                return value;
+            }
+
+            return null;
+        }
+
+        public bool TryGetValue(TKey key, out TValue value)
+        {
+            this.AssertIsForeground();
+
+            WeakComHandle<TValue, TValue> handle;
+            if (_table.TryGetValue(key, out handle))
+            {
+                value = handle.ComAggregateObject;
+                return value != null;
+            }
+
+            value = null;
+            return false;
+        }
+
+        public IEnumerable<TValue> Values
+        {
+            get
+            {
+                foreach (var keyValuePair in _table)
+                {
+                    yield return keyValuePair.Value.ComAggregateObject;
+                }
+            }
+        }
+    }
+}

src/VisualStudio/Core/Def/ServicesVisualStudio.csproj

@@ -43,6 +43,7 @@
     <Compile Include="Implementation\EditAndContinue\Interop\NativeMethods.cs" />
     <Compile Include="Implementation\AnalyzerDependency\IIgnorableAssemblyList.cs" />
     <Compile Include="Implementation\AnalyzerDependency\IBindingRedirectionService.cs" />
+    <Compile Include="Implementation\Interop\CleanableWeakComHandleTable.cs" />
     <Compile Include="Implementation\LanguageService\AbstractLanguageService`2.IVsLanguageBlock.cs" />
     <Compile Include="Implementation\Library\FindResults\TreeItems\AbstractSourceTreeItem.cs" />
     <Compile Include="Implementation\Library\FindResults\TreeItems\MetadataDefinitionTreeItem.cs" />

src/VisualStudio/Core/Impl/CodeModel/AbstractCodeModelService.cs

@@ -267,7 +267,7 @@ public EnvDTE.CodeElement CreateExternalCodeElement(CodeModelState state, Projec
         }
 
         /// <summary>
-        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.CreateCodeElement{T}(SyntaxNode)"/>
+        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.GetOrCreateCodeElement{T}(SyntaxNode)"/>
         /// </summary>
         public abstract EnvDTE.CodeElement CreateInternalCodeElement(
             CodeModelState state,
@@ -362,7 +362,7 @@ protected EnvDTE.CodeFunction CreateInternalCodeAccessorFunction(CodeModelState
                 throw new InvalidOperationException();
             }
 
-            var parent = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+            var parent = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
             var parentObj = ComAggregate.GetManagedObject<AbstractCodeMember>(parent);
             var accessorKind = GetAccessorKind(node);
 
@@ -377,7 +377,7 @@ protected EnvDTE.CodeAttribute CreateInternalCodeAttribute(CodeModelState state,
 
             if (IsParameterNode(parentNode))
             {
-                var parentElement = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+                var parentElement = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
                 parentObject = ComAggregate.GetManagedObject<AbstractCodeElement>(parentElement);
             }
             else
@@ -397,7 +397,7 @@ protected EnvDTE.CodeAttribute CreateInternalCodeAttribute(CodeModelState state,
                 {
                     parentNode = fileCodeModel.LookupNode(nodeKey);
 
-                    var parentElement = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+                    var parentElement = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
                     parentObject = ComAggregate.GetManagedObject<AbstractCodeElement>(parentElement);
                 }
             }
@@ -419,7 +419,7 @@ protected EnvDTE80.CodeImport CreateInternalCodeImport(CodeModelState state, Fil
             AbstractCodeElement parentObj = null;
             if (parentNode != null)
             {
-                var parent = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+                var parent = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
                 parentObj = ComAggregate.GetManagedObject<AbstractCodeElement>(parent);
             }
 
@@ -439,7 +439,7 @@ protected EnvDTE.CodeParameter CreateInternalCodeParameter(CodeModelState state,
 
             string name = GetParameterName(node);
 
-            var parent = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+            var parent = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
             var parentObj = ComAggregate.GetManagedObject<AbstractCodeMember>(parent);
 
             return CodeParameter.Create(state, parentObj, name);
@@ -469,7 +469,7 @@ protected EnvDTE80.CodeElement2 CreateInternalCodeInheritsStatement(CodeModelSta
             int ordinal;
             GetInheritsNamespaceAndOrdinal(parentNode, node, out namespaceName, out ordinal);
 
-            var parent = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+            var parent = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
             var parentObj = ComAggregate.GetManagedObject<AbstractCodeMember>(parent);
 
             return CodeInheritsStatement.Create(state, parentObj, namespaceName, ordinal);
@@ -490,7 +490,7 @@ protected EnvDTE80.CodeElement2 CreateInternalCodeImplementsStatement(CodeModelS
             int ordinal;
             GetImplementsNamespaceAndOrdinal(parentNode, node, out namespaceName, out ordinal);
 
-            var parent = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(parentNode);
+            var parent = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(parentNode);
             var parentObj = ComAggregate.GetManagedObject<AbstractCodeMember>(parent);
 
             return CodeImplementsStatement.Create(state, parentObj, namespaceName, ordinal);

src/VisualStudio/Core/Impl/CodeModel/Collections/InheritsImplementsCollection.cs

@@ -75,7 +75,7 @@ protected override bool TryGetItemByIndex(int index, out EnvDTE.CodeElement elem
             if (index < currentIndex + inheritsNodeCount)
             {
                 var child = inheritsNodes.ElementAt(index - currentIndex);
-                element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                 return true;
             }
 
@@ -87,7 +87,7 @@ protected override bool TryGetItemByIndex(int index, out EnvDTE.CodeElement elem
             if (index < currentIndex + implementsNodeCount)
             {
                 var child = implementsNodes.ElementAt(index - currentIndex);
-                element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                 return true;
             }
 
@@ -107,7 +107,7 @@ protected override bool TryGetItemByName(string name, out EnvDTE.CodeElement ele
                 CodeModelService.GetInheritsNamespaceAndOrdinal(node, child, out childName, out ordinal);
                 if (childName == name)
                 {
-                    element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                    element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                     return true;
                 }
             }
@@ -120,7 +120,7 @@ protected override bool TryGetItemByName(string name, out EnvDTE.CodeElement ele
                 CodeModelService.GetImplementsNamespaceAndOrdinal(node, child, out childName, out ordinal);
                 if (childName == name)
                 {
-                    element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                    element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                     return true;
                 }
             }

src/VisualStudio/Core/Impl/CodeModel/Collections/NamespaceCollection.cs

@@ -155,7 +155,7 @@ protected override bool TryGetItemByIndex(int index, out EnvDTE.CodeElement elem
             if (index < currentIndex + memberNodeCount)
             {
                 var child = memberNodes.ElementAt(index - currentIndex);
-                element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                 return true;
             }
 
@@ -213,7 +213,7 @@ protected override bool TryGetItemByName(string name, out EnvDTE.CodeElement ele
                 var childName = CodeModelService.GetName(child);
                 if (childName == name)
                 {
-                    element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                    element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                     return true;
                 }
             }

src/VisualStudio/Core/Impl/CodeModel/Collections/NodeSnapshot.cs

@@ -109,7 +109,7 @@ public override int Count
                 }
 
                 // The node must be something that the FileCodeModel can create.
-                return this.FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(node);
+                return this.FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(node);
             }
         }
     }

src/VisualStudio/Core/Impl/CodeModel/Collections/TypeCollection.cs

@@ -71,7 +71,7 @@ protected override bool TryGetItemByIndex(int index, out EnvDTE.CodeElement elem
             if (index >= 0 && index < memberNodes.Count())
             {
                 var child = memberNodes.ElementAt(index);
-                element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                 return true;
             }
 
@@ -88,7 +88,7 @@ protected override bool TryGetItemByName(string name, out EnvDTE.CodeElement ele
                 var childName = CodeModelService.GetName(child);
                 if (childName == name)
                 {
-                    element = FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(child);
+                    element = FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(child);
                     return true;
                 }
             }

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel.CleanableWeakComHandleTable.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;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Linq;
-using Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
-using CodeElementWeakComAggregateHandle =
-    Microsoft.VisualStudio.LanguageServices.Implementation.Interop.WeakComHandle<EnvDTE.CodeElement, EnvDTE.CodeElement>;
-
-namespace Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel
-{
-    public sealed partial class FileCodeModel
-    {
-        /// <summary>
-        /// A wrapper around a collection containing weak references that can clean
-        /// itself out of dead weak references in a timesliced way.
-        /// The class holds an enumerator over inner collection and a queue of dead elements.
-        /// On every 25 element added the cleanup is initiated, which is performed when
-        /// CleanupWeakComHandles() method is called (FCM calls it on idle).
-        /// The cleanup works in timesliced way.
-        /// The cleanup first scans the inner collection adding each element with dead weak
-        /// ref to the dead queue. When scan is finished (or new element is added/removed,
-        /// which invalidates the enumerator), cleanup processes the dead queue removing
-        /// elements from the inner collection.
-        /// We keep dead queue alive even when the enumerator got invalidated, removing elements
-        /// from the dead queue when they are removed from the collection via external call.
-        /// </summary>
-        private class CleanableWeakComHandleTable
-        {
-            // TODO: Move these to options
-            private static readonly TimeSpan s_timeSliceForFileCodeModelCleanup = TimeSpan.FromMilliseconds(15);
-            private const int ElementsAddedBeforeFileCodeModelCleanup = 25;
-
-            private readonly Dictionary<SyntaxNodeKey, CodeElementWeakComAggregateHandle> _elements;
-            private bool _needCleanup;
-            private int _elementsAddedSinceLastCleanup;
-
-            // Dead queue is a hash set so we could effectively remove element from
-            // the dead queue when it's being removed externally
-            private readonly HashSet<SyntaxNodeKey> _deadQueue;
-            private IEnumerator<KeyValuePair<SyntaxNodeKey, CodeElementWeakComAggregateHandle>> _cleanupEnumerator;
-
-            private enum State
-            {
-                Initial,
-                Checking,
-                ProcessingDeadQueue
-            }
-
-            private State _state;
-
-            public CleanableWeakComHandleTable()
-            {
-                _elements = new Dictionary<SyntaxNodeKey, CodeElementWeakComAggregateHandle>();
-                _deadQueue = new HashSet<SyntaxNodeKey>();
-                _state = State.Initial;
-            }
-
-            private void InvalidateEnumerator()
-            {
-                if (_cleanupEnumerator != null)
-                {
-                    _cleanupEnumerator.Dispose();
-                    _cleanupEnumerator = null;
-                }
-            }
-
-            public bool NeedCleanup
-            {
-                get { return _needCleanup; }
-            }
-
-            public void Add(SyntaxNodeKey key, CodeElementWeakComAggregateHandle value)
-            {
-                TriggerCleanup();
-                InvalidateEnumerator();
-                _elements.Add(key, value);
-            }
-
-            public void Remove(SyntaxNodeKey key)
-            {
-                InvalidateEnumerator();
-
-                if (_deadQueue.Contains(key))
-                {
-                    _deadQueue.Remove(key);
-                }
-
-                _elements.Remove(key);
-            }
-
-            public bool TryGetValue(SyntaxNodeKey key, out CodeElementWeakComAggregateHandle value)
-            {
-                return _elements.TryGetValue(key, out value);
-            }
-
-            public bool ContainsKey(SyntaxNodeKey key)
-            {
-                return _elements.ContainsKey(key);
-            }
-
-            public IEnumerable<CodeElementWeakComAggregateHandle> Values
-            {
-                get { return _elements.Values; }
-            }
-
-            private void TriggerCleanup()
-            {
-                _elementsAddedSinceLastCleanup++;
-                if (_elementsAddedSinceLastCleanup >= ElementsAddedBeforeFileCodeModelCleanup)
-                {
-                    _needCleanup = true;
-                    _elementsAddedSinceLastCleanup = 0;
-                }
-            }
-
-            private bool CheckWeakComHandles(TimeSlice timeSlice)
-            {
-                Debug.Assert(_cleanupEnumerator != null);
-                Debug.Assert(_state == State.Checking);
-
-                while (_cleanupEnumerator.MoveNext())
-                {
-                    if (!_cleanupEnumerator.Current.Value.IsAlive())
-                    {
-                        _deadQueue.Add(_cleanupEnumerator.Current.Key);
-                    }
-
-                    if (timeSlice.IsOver)
-                    {
-                        return false;
-                    }
-                }
-
-                return true;
-            }
-
-            private bool ProcessDeadQueue(TimeSlice timeSlice)
-            {
-                Debug.Assert(_cleanupEnumerator == null, "We should never process dead queue when enumerator is alive.");
-
-                while (_deadQueue.Count > 0)
-                {
-                    var key = _deadQueue.First();
-
-                    _deadQueue.Remove(key);
-                    Debug.Assert(_elements.ContainsKey(key), "How come the key is in the dead queue, but not in the dictionary?");
-
-                    _elements.Remove(key);
-
-                    if (timeSlice.IsOver)
-                    {
-                        return false;
-                    }
-                }
-
-                return true;
-            }
-
-            public void CleanupWeakComHandles()
-            {
-                if (!_needCleanup)
-                {
-                    return;
-                }
-
-                var timeSlice = new TimeSlice(s_timeSliceForFileCodeModelCleanup);
-
-                if (_state == State.Initial)
-                {
-                    _cleanupEnumerator = _elements.GetEnumerator();
-                    _state = State.Checking;
-                }
-
-                if (_state == State.Checking)
-                {
-                    if (_cleanupEnumerator == null)
-                    {
-                        // The enumerator got reset while we were checking, need to process dead queue
-                        // before starting checking over again
-                        if (!ProcessDeadQueue(timeSlice))
-                        {
-                            // Need more time to finish processing dead queue, continue next time
-                            return;
-                        }
-
-                        _cleanupEnumerator = _elements.GetEnumerator();
-                    }
-
-                    if (!CheckWeakComHandles(timeSlice))
-                    {
-                        // Need more time to check for dead elements, continue next time
-                        return;
-                    }
-
-                    // Done with checking, now process dead queue
-                    InvalidateEnumerator();
-                    _state = State.ProcessingDeadQueue;
-                }
-
-                if (_state == State.ProcessingDeadQueue)
-                {
-                    if (!ProcessDeadQueue(timeSlice))
-                    {
-                        // Need more time to finish processing dead queue, continue next time
-                        return;
-                    }
-
-                    // Done with cleanup
-                    _state = State.Initial;
-                    _needCleanup = false;
-                }
-            }
-        }
-    }
-}

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel.ElementTable.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;
-using System.Collections.Generic;
-using System.Diagnostics;
-using Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel.InternalElements;
-using CodeElementWeakComAggregateHandle =
-    Microsoft.VisualStudio.LanguageServices.Implementation.Interop.WeakComHandle<EnvDTE.CodeElement, EnvDTE.CodeElement>;
-
-namespace Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel
-{
-    public sealed partial class FileCodeModel
-    {
-        private class ElementTable : IEnumerable<EnvDTE.CodeElement>
-        {
-            // Since CodeElements have a strong ref on the FileCodeModel, we should use
-            // weak references on the CodeElements to prevent a cycle. This class should
-            // keep weak refs to the CodeElements so that we get a graph like this:
-            //  => - Strong reference           -> - Weak reference
-            //
-            //      FileCodeModel =>
-            //          ElementTable->     // we prevent a cycle with a native object in the mix by using a weak ref here.
-            //              RCW =>
-            //                  NativeComAggregate =>
-            //                      CCW =>
-            //                          CodeElement =>
-            //                              FileCodeModel
-            //
-            // See Dev10 Bug 785889 and 799848.
-
-            private readonly CleanableWeakComHandleTable _elementWeakComHandles = new CleanableWeakComHandleTable();
-
-            public void Add(SyntaxNodeKey key, EnvDTE.CodeElement element)
-            {
-                // This code deals with a weird case of interaction with WinForms: The same
-                // element can be added multiple times. What we have to do is bump
-                // the conflicting element to a free [KeyName, Ordinal] slot
-                // by incrementing [Ordinal]
-                // Elements with the same node path can also be added multiple times in 
-                // normal editing scenarios, e.g. when a code element is changing preserving its name
-                // (e.g. a class becomes an interface) or user just removes a block of code and 
-                // then readds it back before code elements for removed code were garbage collected.
-                CodeElementWeakComAggregateHandle existingElementHandle;
-                if (_elementWeakComHandles.TryGetValue(key, out existingElementHandle))
-                {
-                    int newOrdinal = key.Ordinal;
-                    while (true)
-                    {
-                        newOrdinal++;
-                        var currentKey = new SyntaxNodeKey(key.Name, newOrdinal);
-                        if (!_elementWeakComHandles.ContainsKey(currentKey))
-                        {
-                            // We found a free "slot": use it and release the previous one
-                            AbstractKeyedCodeElement existingElement = null;
-                            EnvDTE.CodeElement existingElementManagedObject;
-                            if (existingElementHandle.TryGetManagedObjectWithoutCaringWhetherNativeObjectIsAlive(out existingElementManagedObject))
-                            {
-                                existingElement = existingElementManagedObject as AbstractKeyedCodeElement;
-                            }
-
-                            _elementWeakComHandles.Remove(key);
-
-                            if (existingElementHandle.ComAggregateObject == null)
-                            {
-                                // The native object has already been released.
-                                // There's no need to re-add this handle.
-                                break;
-                            }
-
-                            Debug.Assert(existingElement != null, "The ComAggregate is alive. Why isn't the actual managed object?");
-
-                            _elementWeakComHandles.Add(currentKey, existingElementHandle);
-                            existingElement.NodeKey = currentKey;
-                            break;
-                        }
-                    }
-                }
-
-                Debug.Assert(!_elementWeakComHandles.ContainsKey(key),
-                             "All right, we got it wrong. We should have a free entry in the table!");
-
-                _elementWeakComHandles.Add(key, new CodeElementWeakComAggregateHandle(element));
-            }
-
-            public void Remove(SyntaxNodeKey nodeKey, out EnvDTE.CodeElement removedElement)
-            {
-                var elementHandleInTable = RemoveImpl(nodeKey);
-                removedElement = elementHandleInTable.ComAggregateObject;
-            }
-
-            public void Remove(SyntaxNodeKey nodeKey)
-            {
-                RemoveImpl(nodeKey);
-            }
-
-            private CodeElementWeakComAggregateHandle RemoveImpl(SyntaxNodeKey nodeKey)
-            {
-                CodeElementWeakComAggregateHandle elementHandleInTable;
-                if (!_elementWeakComHandles.TryGetValue(nodeKey, out elementHandleInTable))
-                {
-                    Debug.Fail("Can't find code element being removed");
-                    throw new InvalidOperationException();
-                }
-
-                _elementWeakComHandles.Remove(nodeKey);
-                return elementHandleInTable;
-            }
-
-            public EnvDTE.CodeElement TryGetValue(SyntaxNodeKey nodeKey)
-            {
-                CodeElementWeakComAggregateHandle resultWeakComHandle;
-                if (_elementWeakComHandles.TryGetValue(nodeKey, out resultWeakComHandle))
-                {
-                    return resultWeakComHandle.ComAggregateObject;
-                }
-
-                return null;
-            }
-
-            private IEnumerator<EnvDTE.CodeElement> GetEnumeratorForElementsThatAreAlive()
-            {
-                // We only want to iterate over the comHandles that are still alive.
-                var handlesThatAreAlive = new List<EnvDTE.CodeElement>();
-                foreach (var handle in _elementWeakComHandles.Values)
-                {
-                    var element = handle.ComAggregateObject;
-                    if (element == null)
-                    {
-                        // The ComAggregateObject for this element has been released.
-                        continue;
-                    }
-
-                    handlesThatAreAlive.Add(element);
-                }
-
-                return handlesThatAreAlive.GetEnumerator();
-            }
-
-            public IEnumerator<EnvDTE.CodeElement> GetEnumerator()
-            {
-                return GetEnumeratorForElementsThatAreAlive();
-            }
-
-            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
-            {
-                return GetEnumeratorForElementsThatAreAlive();
-            }
-
-            public void Cleanup(out bool needMoreTime)
-            {
-                _elementWeakComHandles.CleanupWeakComHandles();
-                needMoreTime = _elementWeakComHandles.NeedCleanup;
-            }
-        }
-    }
-}

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel.cs

@@ -47,7 +47,7 @@ public sealed partial class FileCodeModel : AbstractCodeModelObject, EnvDTE.File
 
         private readonly ITextManagerAdapter _textManagerAdapter;
 
-        private readonly ElementTable _elementTable;
+        private readonly CleanableWeakComHandleTable<SyntaxNodeKey, EnvDTE.CodeElement> _codeElementTable;
 
         // These are used during batching.
         private bool _batchMode;
@@ -74,7 +74,7 @@ public sealed partial class FileCodeModel : AbstractCodeModelObject, EnvDTE.File
             _documentId = documentId;
             _textManagerAdapter = textManagerAdapter;
 
-            _elementTable = new ElementTable();
+            _codeElementTable = new CleanableWeakComHandleTable<SyntaxNodeKey, EnvDTE.CodeElement>();
 
             _batchMode = false;
             _batchDocument = null;
@@ -175,86 +175,50 @@ internal DocumentId GetDocumentId()
             throw Exceptions.ThrowEUnexpected();
         }
 
-        private void AddElement(SyntaxNodeKey nodeKey, EnvDTE.CodeElement element)
+        internal void UpdateCodeElementNodeKey(AbstractKeyedCodeElement keyedElement, SyntaxNodeKey oldNodeKey, SyntaxNodeKey newNodeKey)
         {
-            _elementTable.Add(nodeKey, element);
-        }
-
-        private void RemoveElement(SyntaxNodeKey nodeKey)
-        {
-            _elementTable.Remove(nodeKey);
-        }
+            var codeElement = _codeElementTable.Remove(oldNodeKey);
 
-        /// <summary>
-        /// This function re-adds a code element to the table, taking care not to duplicate
-        /// an element (i.e., making sure that no two elements with the same key-ordinal
-        /// appear in the same element chain in the table).  To resolve any conflict, each
-        /// node with the given key is examined positionally, and the existing order is
-        /// maintained as closely as possible -- but it is still possible the code element
-        /// references to duplicate elements can get "bumped" by odd edits -- nothing we
-        /// can do about this.
-        /// </summary>
-        internal void ResetElementNodeKey(AbstractKeyedCodeElement element, SyntaxNodeKey nodeKey)
-        {
-            EnvDTE.CodeElement elementInTable;
-            _elementTable.Remove(element.NodeKey, out elementInTable);
-
-            var abstractElementInTable = ComAggregate.GetManagedObject<AbstractKeyedCodeElement>(elementInTable);
-            if (!object.Equals(abstractElementInTable, element))
+            var managedElement = ComAggregate.GetManagedObject<AbstractKeyedCodeElement>(codeElement);
+            if (!object.Equals(managedElement, keyedElement))
             {
-                Debug.Fail("Found a different element with the same key!");
-                throw new InvalidOperationException();
+                throw new InvalidOperationException($"Unexpected failure in Code Model while updating node keys {oldNodeKey} -> {newNodeKey}");
             }
 
-            abstractElementInTable.NodeKey = nodeKey;
-
-            _elementTable.Add(nodeKey, elementInTable);
+            _codeElementTable.Add(newNodeKey, codeElement);
         }
 
-        internal void OnElementCreated(SyntaxNodeKey nodeKey, EnvDTE.CodeElement element)
+        internal void OnCodeElementCreated(SyntaxNodeKey nodeKey, EnvDTE.CodeElement element)
         {
-            AddElement(nodeKey, element);
+            _codeElementTable.Add(nodeKey, element);
         }
 
-        internal T CreateCodeElement<T>(SyntaxNode node)
+        internal T GetOrCreateCodeElement<T>(SyntaxNode node)
         {
             var nodeKey = CodeModelService.TryGetNodeKey(node);
 
             if (!nodeKey.IsEmpty)
             {
-                // Check if the node exists in the parse tree.
-                // Note that in designer spew the nodes don't get created right away so we skip this check.
-                if (!IsBatchOpen && CodeModelService.LookupNode(nodeKey, GetSyntaxTree()) == null)
-                {
-                    throw Exceptions.ThrowEFail();
-                }
-
-                // See if the element exists.
-                var previousElement = _elementTable.TryGetValue(nodeKey);
+                // Since the node already has a key, check to see if a code element already
+                // exists for it. If so, return that element it it's still valid; otherwise,
+                // remove it from the table.
 
-                // Here's our element... possibly.  It must be valid -- if it isn't,
-                // we need to remove it.
-                if (previousElement != null)
+                EnvDTE.CodeElement codeElement;
+                if (_codeElementTable.TryGetValue(nodeKey, out codeElement))
                 {
-                    var previousElementImpl = ComAggregate.TryGetManagedObject<AbstractCodeElement>(previousElement);
-
-                    if (previousElementImpl.IsValidNode())
+                    var element = ComAggregate.TryGetManagedObject<AbstractCodeElement>(codeElement);
+                    if (element.IsValidNode())
                     {
-                        if (previousElement is T)
+                        if (codeElement is T)
                         {
-                            return (T)previousElement;
-                        }
-                        else
-                        {
-                            Debug.Fail("Called asked for the wrong type!");
-                            throw new InvalidOperationException();
+                            return (T)codeElement;
                         }
+
+                        throw new InvalidOperationException($"Found a valid code element for {nodeKey}, but it is not of type, {typeof(T).ToString()}");
                     }
                     else
                     {
-                        // This guy is no longer valid, so yank it out.  No sense
-                        // continuing to look for a match, either.
-                        RemoveElement(nodeKey);
+                        _codeElementTable.Remove(nodeKey);
                     }
                 }
             }
@@ -541,6 +505,7 @@ private int GetPositionFromTextPoint(EnvDTE.TextPoint point)
             var column = point.LineCharOffset - 1;
             var line = GetDocument().GetTextAsync(CancellationToken.None).WaitAndGetResult(CancellationToken.None).Lines[lineNumber];
             var position = line.Start + column;
+
             return position;
         }
 
@@ -615,15 +580,12 @@ internal EnvDTE.CodeElement CodeElementFromPosition(int position, EnvDTE.vsCMEle
                 return CodeModelService.CreateInternalCodeElement(State, this, node);
             }
 
-            return CreateCodeElement<EnvDTE.CodeElement>(node);
+            return GetOrCreateCodeElement<EnvDTE.CodeElement>(node);
         }
 
         public EnvDTE.CodeElements CodeElements
         {
-            get
-            {
-                return NamespaceCollection.Create(this.State, this, this, SyntaxNodeKey.Empty);
-            }
+            get { return NamespaceCollection.Create(this.State, this, this, SyntaxNodeKey.Empty); }
         }
 
         public EnvDTE.ProjectItem Parent
@@ -706,8 +668,8 @@ int IVBFileCodeModelEvents.EndEdit()
                         foreach (var elementAndPath in elementAndPaths)
                         {
                             // make sure the element is there.
-                            var existingElement = _elementTable.TryGetValue(elementAndPath.Item1.NodeKey);
-                            if (existingElement != null)
+                            EnvDTE.CodeElement existingElement;
+                            if (_codeElementTable.TryGetValue(elementAndPath.Item1.NodeKey, out existingElement))
                             {
                                 elementAndPath.Item1.ReacquireNodeKey(elementAndPath.Item2, CancellationToken.None);
                             }
@@ -784,10 +746,9 @@ internal List<GlobalNodeKey> GetCurrentNodeKeys()
         {
             var currentNodeKeys = new List<GlobalNodeKey>();
 
-            foreach (var element in _elementTable)
+            foreach (var element in _codeElementTable.Values)
             {
                 var keyedElement = ComAggregate.TryGetManagedObject<AbstractKeyedCodeElement>(element);
-
                 if (keyedElement == null)
                 {
                     continue;
@@ -797,7 +758,7 @@ internal List<GlobalNodeKey> GetCurrentNodeKeys()
                 if (keyedElement.TryLookupNode(out node))
                 {
                     var nodeKey = keyedElement.NodeKey;
-                    currentNodeKeys.Add(new GlobalNodeKey(nodeKey, new Roslyn.Utilities.SyntaxPath(node)));
+                    currentNodeKeys.Add(new GlobalNodeKey(nodeKey, new SyntaxPath(node)));
                 }
             }
 
@@ -814,13 +775,17 @@ internal void ResetElementKeys(List<GlobalNodeKey> globalNodeKeys)
 
         private void ResetElementKey(GlobalNodeKey globalNodeKey)
         {
-            var element = _elementTable.TryGetValue(globalNodeKey.NodeKey);
-
             // Failure to find the element is not an error -- it just means the code
             // element didn't exist...
-            if (element != null)
+
+            EnvDTE.CodeElement element;
+            if (_codeElementTable.TryGetValue(globalNodeKey.NodeKey, out element))
             {
-                ComAggregate.GetManagedObject<AbstractKeyedCodeElement>(element).ReacquireNodeKey(globalNodeKey.Path, default(CancellationToken));
+                var keyedElement = ComAggregate.GetManagedObject<AbstractKeyedCodeElement>(element);
+                if (keyedElement != null)
+                {
+                    keyedElement.ReacquireNodeKey(globalNodeKey.Path, default(CancellationToken));
+                }
             }
         }
     }

src/VisualStudio/Core/Impl/CodeModel/FileCodeModel_Events.cs

@@ -20,7 +20,8 @@ public bool FireEvents()
         {
             var needMoreTime = false;
 
-            _elementTable.Cleanup(out needMoreTime);
+            _codeElementTable.CleanUpDeadObjects();
+            needMoreTime = _codeElementTable.NeedsCleanUp;
 
             if (this.IsZombied)
             {
@@ -165,7 +166,7 @@ internal void GetElementsForCodeModelEvent(CodeModelEvent codeModelEvent, out En
                 }
                 else
                 {
-                    element = this.CreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.Node);
+                    element = this.GetOrCreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.Node);
                 }
             }
 
@@ -184,14 +185,14 @@ private object GetParentElementForCodeModelEvent(CodeModelEvent codeModelEvent)
             {
                 if (codeModelEvent.ParentNode != null)
                 {
-                    return this.CreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
+                    return this.GetOrCreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
                 }
             }
             else if (this.CodeModelService.IsAttributeNode(codeModelEvent.Node))
             {
                 if (codeModelEvent.ParentNode != null)
                 {
-                    return this.CreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
+                    return this.GetOrCreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
                 }
                 else
                 {
@@ -203,7 +204,7 @@ private object GetParentElementForCodeModelEvent(CodeModelEvent codeModelEvent)
                 if (codeModelEvent.ParentNode != null &&
                     codeModelEvent.ParentNode.Parent != null)
                 {
-                    return this.CreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
+                    return this.GetOrCreateCodeElement<EnvDTE.CodeElement>(codeModelEvent.ParentNode);
                 }
                 else
                 {

src/VisualStudio/Core/Impl/CodeModel/ICodeModelService.cs

@@ -74,7 +74,7 @@ internal interface ICodeModelService : ICodeModelNavigationPointService
         string AssemblyAttributeString { get; }
 
         /// <summary>
-        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.CreateCodeElement{T}(SyntaxNode)"/>
+        /// Do not use this method directly! Instead, go through <see cref="FileCodeModel.GetOrCreateCodeElement{T}(SyntaxNode)"/>
         /// </summary>
         EnvDTE.CodeElement CreateInternalCodeElement(CodeModelState state, FileCodeModel fileCodeModel, SyntaxNode node);
         EnvDTE.CodeElement CreateExternalCodeElement(CodeModelState state, ProjectId projectId, ISymbol symbol);

src/VisualStudio/Core/Impl/CodeModel/InternalElements/AbstractCodeMember.cs

@@ -47,7 +47,7 @@ public override object Parent
                     throw Exceptions.ThrowEUnexpected();
                 }
 
-                return FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(containingTypeNode);
+                return FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(containingTypeNode);
             }
         }
 

src/VisualStudio/Core/Impl/CodeModel/InternalElements/AbstractCodeType.cs

@@ -68,7 +68,7 @@ public override object Parent
                 var containingNamespaceOrType = GetNamespaceOrTypeNode();
 
                 return containingNamespaceOrType != null
-                    ? (object)FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(containingNamespaceOrType)
+                    ? (object)FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(containingNamespaceOrType)
                     : this.FileCodeModel;
             }
         }
@@ -133,7 +133,7 @@ public EnvDTE.CodeNamespace Namespace
                 var namespaceNode = GetNamespaceNode();
 
                 return namespaceNode != null
-                    ? FileCodeModel.CreateCodeElement<EnvDTE.CodeNamespace>(namespaceNode)
+                    ? FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeNamespace>(namespaceNode)
                     : null;
             }
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/AbstractKeyedCodeElement.cs

@@ -4,14 +4,13 @@
 using System.Diagnostics;
 using System.Threading;
 using Microsoft.CodeAnalysis;
-using Microsoft.VisualStudio.LanguageServices.Implementation.Interop;
 using Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
 using Roslyn.Utilities;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.CodeModel.InternalElements
 {
     /// <summary>
-    /// This is the base class of all code elements located with a SyntaxNodeKey.
+    /// This is the base class of all code elements identified by a SyntaxNodeKey.
     /// </summary>
     public abstract class AbstractKeyedCodeElement : AbstractCodeElement
     {
@@ -44,7 +43,6 @@ public abstract class AbstractKeyedCodeElement : AbstractCodeElement
         internal SyntaxNodeKey NodeKey
         {
             get { return _nodeKey; }
-            set { _nodeKey = value; }
         }
 
         internal bool IsUnknown
@@ -75,9 +73,11 @@ internal void ReacquireNodeKey(SyntaxPath syntaxPath, CancellationToken cancella
                 throw Exceptions.ThrowEFail();
             }
 
-            var nodeKey = CodeModelService.GetNodeKey(node);
+            var newNodeKey = CodeModelService.GetNodeKey(node);
 
-            FileCodeModel.ResetElementNodeKey(this, nodeKey);
+            FileCodeModel.UpdateCodeElementNodeKey(this, _nodeKey, newNodeKey);
+
+            _nodeKey = newNodeKey;
         }
 
         protected void UpdateNodeAndReacquireNodeKey<T>(Action<SyntaxNode, T> updater, T value, bool trackKinds = true)

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeClass.cs

@@ -28,7 +28,7 @@ public sealed class CodeClass : AbstractCodeType, EnvDTE.CodeClass, EnvDTE80.Cod
             var element = new CodeClass(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeClass)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeDelegate.cs

@@ -20,7 +20,7 @@ public sealed partial class CodeDelegate : AbstractCodeType, ICodeElementContain
             var element = new CodeDelegate(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeDelegate)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeEnum.cs

@@ -18,7 +18,7 @@ public sealed partial class CodeEnum : AbstractCodeType, EnvDTE.CodeEnum
             var element = new CodeEnum(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeEnum)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeEvent.cs

@@ -21,7 +21,7 @@ public sealed partial class CodeEvent : AbstractCodeMember, EnvDTE80.CodeEvent
             var element = new CodeEvent(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE80.CodeEvent)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeFunction.cs

@@ -24,7 +24,7 @@ public partial class CodeFunction : AbstractCodeMember, ICodeElementContainer<Co
             var element = new CodeFunction(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeFunction)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeFunctionDeclareDecl.cs

@@ -23,7 +23,7 @@ public sealed class CodeFunctionDeclareDecl : CodeFunction
             var element = new CodeFunctionDeclareDecl(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeFunction)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeFunctionWithEventHandler.cs

@@ -18,7 +18,7 @@ public sealed class CodeFunctionWithEventHandler : CodeFunction, Interop.IEventH
             var element = new CodeFunctionWithEventHandler(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeFunction)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeInterface.cs

@@ -19,7 +19,7 @@ public sealed class CodeInterface : AbstractCodeType, EnvDTE.CodeInterface, EnvD
             var element = new CodeInterface(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeInterface)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeNamespace.cs

@@ -23,7 +23,7 @@ public sealed class CodeNamespace : AbstractKeyedCodeElement, EnvDTE.CodeNamespa
             var element = new CodeNamespace(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeNamespace)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }
@@ -68,7 +68,7 @@ public override object Parent
                 var namespaceNode = GetNamespaceNode();
 
                 return namespaceNode != null
-                    ? (object)FileCodeModel.CreateCodeElement<EnvDTE.CodeNamespace>(namespaceNode)
+                    ? (object)FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeNamespace>(namespaceNode)
                     : this.FileCodeModel;
             }
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeProperty.cs

@@ -22,7 +22,7 @@ public sealed partial class CodeProperty : AbstractCodeMember, ICodeElementConta
             var element = new CodeProperty(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeProperty)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }
@@ -109,7 +109,7 @@ public EnvDTE.CodeElement Parent2
                     throw Exceptions.ThrowEUnexpected();
                 }
 
-                return FileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(containingTypeNode);
+                return FileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(containingTypeNode);
             }
         }
 

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeStruct.cs

@@ -20,7 +20,7 @@ public sealed class CodeStruct : AbstractCodeType, EnvDTE.CodeStruct, EnvDTE80.C
             var element = new CodeStruct(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeStruct)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/InternalElements/CodeVariable.cs

@@ -20,7 +20,7 @@ public sealed class CodeVariable : AbstractCodeMember, EnvDTE.CodeVariable, EnvD
             var element = new CodeVariable(state, fileCodeModel, nodeKey, nodeKind);
             var result = (EnvDTE.CodeVariable)ComAggregate.CreateAggregatedObject(element);
 
-            fileCodeModel.OnElementCreated(nodeKey, (EnvDTE.CodeElement)result);
+            fileCodeModel.OnCodeElementCreated(nodeKey, (EnvDTE.CodeElement)result);
 
             return result;
         }

src/VisualStudio/Core/Impl/CodeModel/SyntaxNodeKey.cs

@@ -58,7 +58,7 @@ public override int GetHashCode()
 
         public override string ToString()
         {
-            return string.Format("{{{0}, {1}}}", _name, _ordinal);
+            return $"{{{_name}, {_ordinal}}}";
         }
 
         public string Name

src/VisualStudio/Core/Impl/RoslynVisualStudioWorkspace.cs

@@ -281,7 +281,7 @@ internal override object GetBrowseObject(SymbolListItem symbolListItem)
 
                 if (syntaxNode != null)
                 {
-                    var codeElement = fileCodeModel.CreateCodeElement<EnvDTE.CodeElement>(syntaxNode);
+                    var codeElement = fileCodeModel.GetOrCreateCodeElement<EnvDTE.CodeElement>(syntaxNode);
                     if (codeElement != null)
                     {
                         return codeElement;

src/VisualStudio/Core/Impl/ServicesVisualStudioImpl.csproj

@@ -247,9 +247,7 @@
     <Compile Include="CodeModel\ExternalElements\ExternalCodeStruct.cs" />
     <Compile Include="CodeModel\ExternalElements\ExternalCodeUnknown.cs" />
     <Compile Include="CodeModel\ExternalElements\ExternalCodeVariable.cs" />
-    <Compile Include="CodeModel\FileCodeModel.CleanableWeakComHandleTable.cs" />
     <Compile Include="CodeModel\FileCodeModel.cs" />
-    <Compile Include="CodeModel\FileCodeModel.ElementTable.cs" />
     <Compile Include="CodeModel\FileCodeModel_CodeGen.cs" />
     <Compile Include="CodeModel\FileCodeModel_Events.cs" />
     <Compile Include="CodeModel\FileCodeModel_Refactoring.cs" />

src/VisualStudio/VisualBasic/Impl/CodeModel/VisualBasicCodeModelService.vb

@@ -521,7 +521,7 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.CodeModel
         End Property
 
         ''' <summary>
-        ''' Do not use this method directly! Instead, go through <see cref="FileCodeModel.CreateCodeElement(Of T)(SyntaxNode)"/>
+        ''' Do not use this method directly! Instead, go through <see cref="FileCodeModel.GetOrCreateCodeElement(Of T)(SyntaxNode)"/>
         ''' </summary>
         Public Overloads Overrides Function CreateInternalCodeElement(
             state As CodeModelState,