Merge pull request #2496 from MichalStrehovsky/nativeLayoutCleanup

Cleanup around ExactMethodInstantiation and Generics hashtable

3 files changed, 46 insertions, 81 deletions

diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs
index 9c6ed6029..124b1854f 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ExactMethodInstantiationsNode.cs
@@ -5,7 +5,6 @@
 using System;
 using System.IO;
 using System.Diagnostics;
-using System.Collections.Generic;
 
 using Internal.Text;
 using Internal.TypeSystem;
@@ -21,16 +20,10 @@ namespace ILCompiler.DependencyAnalysis
         private ObjectAndOffsetSymbolNode _endSymbol;
         private ExternalReferencesTableNode _externalReferences;
 
-        private HashSet<MethodDesc> _visitedMethods;
-        private List<MethodDesc> _exactMethodInstantiationsList;
-
         public ExactMethodInstantiationsNode(ExternalReferencesTableNode externalReferences)
         {
             _endSymbol = new ObjectAndOffsetSymbolNode(this, 0, "__exact_method_instantiations_End", true);
             _externalReferences = externalReferences;
-
-            _visitedMethods = new HashSet<MethodDesc>();
-            _exactMethodInstantiationsList = new List<MethodDesc>();
         }
 
         public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
@@ -51,21 +44,20 @@ namespace ILCompiler.DependencyAnalysis
             if (relocsOnly)
                 return new ObjectData(Array.Empty<byte>(), Array.Empty<Relocation>(), 1, new ISymbolNode[] { this });
 
-            // Zero out the hashset so that we AV if someone tries to insert after we're done.
-            _visitedMethods = null;
-
             // Ensure the native layout data has been saved, in order to get valid Vertex offsets for the signature Vertices
             factory.MetadataManager.NativeLayoutInfo.SaveNativeLayoutInfoWriter(factory);
 
-
             NativeWriter nativeWriter = new NativeWriter();
             VertexHashtable hashtable = new VertexHashtable();
             Section nativeSection = nativeWriter.NewSection();
             nativeSection.Place(hashtable);
 
 
-            foreach (MethodDesc method in _exactMethodInstantiationsList)
+            foreach (MethodDesc method in factory.MetadataManager.GetCompiledMethods())
             {
+                if (!IsMethodEligibleForTracking(method))
+                    continue;
+
                 // Get the method pointer vertex
 
                 bool getUnboxingStub = method.OwningType.IsValueType && !method.Signature.IsStatic;
@@ -143,18 +135,6 @@ namespace ILCompiler.DependencyAnalysis
             return dependencies;
         }
 
-        public void AddEntryIfEligible(NodeFactory factory, MethodDesc method)
-        {
-            // Check if we already saw this method
-            if (!_visitedMethods.Add(method))
-                return;
-
-            if (!IsMethodEligibleForTracking(method))
-                return;
-
-            _exactMethodInstantiationsList.Add(method);
-        }
-
         private static bool IsMethodEligibleForTracking(MethodDesc method)
         {
             // Runtime determined methods should never show up here.
diff --git a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericsHashtableNode.cs b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericsHashtableNode.cs
index 5511c4114..fd5315207 100644
--- a/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericsHashtableNode.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/GenericsHashtableNode.cs
@@ -4,8 +4,6 @@
 
 using System;
 using System.IO;
-using System.Diagnostics;
-using System.Collections.Generic;
 
 using Internal.Text;
 using Internal.TypeSystem;
@@ -21,23 +19,10 @@ namespace ILCompiler.DependencyAnalysis
         private ObjectAndOffsetSymbolNode _endSymbol;
         private ExternalReferencesTableNode _externalReferences;
 
-        private NativeWriter _writer;
-        private Section _tableSection;
-        private VertexHashtable _hashtable;
-
-        private HashSet<TypeDesc> _genericTypeInstantiations;
-
         public GenericsHashtableNode(ExternalReferencesTableNode externalReferences)
         {
             _endSymbol = new ObjectAndOffsetSymbolNode(this, 0, "__generics_hashtable_End", true);
             _externalReferences = externalReferences;
-
-            _writer = new NativeWriter();
-            _hashtable = new VertexHashtable();
-            _tableSection = _writer.NewSection();
-            _tableSection.Place(_hashtable);
-
-            _genericTypeInstantiations = new HashSet<TypeDesc>();
         }
 
         public void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
@@ -52,34 +37,32 @@ namespace ILCompiler.DependencyAnalysis
         public override bool StaticDependenciesAreComputed => true;
         protected override string GetName() => this.GetMangledName();
 
-        public void AddEntryIfEligible(NodeFactory factory, TypeDesc type)
-        {
-            // If this is an instantiated non-canonical generic type, add it to the generic instantiations hashtable
-            if (!type.HasInstantiation || type.IsGenericDefinition || type.IsCanonicalSubtype(CanonicalFormKind.Any))
-                return;
-
-            // Already added?
-            if (!_genericTypeInstantiations.Add(type))
-                return;
-
-            var typeSymbol = factory.NecessaryTypeSymbol(type);
-            uint instantiationId = _externalReferences.GetIndex(typeSymbol);
-            Vertex hashtableEntry = _writer.GetUnsignedConstant(instantiationId);
-
-            _hashtable.Append((uint)type.GetHashCode(), _tableSection.Place(hashtableEntry));
-        }
-
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
         {
             // This node does not trigger generation of other nodes.
             if (relocsOnly)
                 return new ObjectData(Array.Empty<byte>(), Array.Empty<Relocation>(), 1, new ISymbolNode[] { this });
 
-            // Zero out the hashset so that we AV if someone tries to insert after we're done.
-            _genericTypeInstantiations = null;
+            NativeWriter nativeWriter = new NativeWriter();
+            VertexHashtable hashtable = new VertexHashtable();
+            Section nativeSection = nativeWriter.NewSection();
+            nativeSection.Place(hashtable);
+
+            foreach (var type in factory.MetadataManager.GetTypesWithEETypes())
+            {
+                // If this is an instantiated non-canonical generic type, add it to the generic instantiations hashtable
+                if (!type.HasInstantiation || type.IsGenericDefinition || type.IsCanonicalSubtype(CanonicalFormKind.Any))
+                    continue;
+
+                var typeSymbol = factory.NecessaryTypeSymbol(type);
+                uint instantiationId = _externalReferences.GetIndex(typeSymbol);
+                Vertex hashtableEntry = nativeWriter.GetUnsignedConstant(instantiationId);
+
+                hashtable.Append((uint)type.GetHashCode(), nativeSection.Place(hashtableEntry));
+            }
 
             MemoryStream stream = new MemoryStream();
-            _writer.Save(stream);
+            nativeWriter.Save(stream);
             byte[] streamBytes = stream.ToArray();
 
             _endSymbol.SetSymbolOffset(streamBytes.Length);
diff --git a/src/ILCompiler.Compiler/src/Compiler/MetadataGeneration.cs b/src/ILCompiler.Compiler/src/Compiler/MetadataGeneration.cs
index 31a832f0e..ff39e1b51 100644
--- a/src/ILCompiler.Compiler/src/Compiler/MetadataGeneration.cs
+++ b/src/ILCompiler.Compiler/src/Compiler/MetadataGeneration.cs
@@ -46,12 +46,6 @@ namespace ILCompiler
 
         private Dictionary<DynamicInvokeMethodSignature, MethodDesc> _dynamicInvokeThunks = new Dictionary<DynamicInvokeMethodSignature, MethodDesc>();
 
-        private ExternalReferencesTableNode _commonFixupsTableNode;
-        private ExternalReferencesTableNode _nativeReferencesTableNode;
-
-        private GenericsHashtableNode _genericsHashtable;
-        private ExactMethodInstantiationsNode _exactMethodInstantiations;
-
         internal NativeLayoutInfoNode NativeLayoutInfo { get; private set; }
 
         public MetadataGeneration(NodeFactory factory)
@@ -76,8 +70,8 @@ namespace ILCompiler
             var metadataNode = new MetadataNode();
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.EmbeddedMetadata), metadataNode, metadataNode, metadataNode.EndSymbol);
 
-            _commonFixupsTableNode = new ExternalReferencesTableNode("CommonFixupsTable");
-            _nativeReferencesTableNode = new ExternalReferencesTableNode("NativeReferences");
+            var commonFixupsTableNode = new ExternalReferencesTableNode("CommonFixupsTable");
+            var nativeReferencesTableNode = new ExternalReferencesTableNode("NativeReferences");
 
             var resourceDataNode = new ResourceDataNode();
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.BlobIdResourceData), resourceDataNode, resourceDataNode, resourceDataNode.EndSymbol);
@@ -85,34 +79,34 @@ namespace ILCompiler
             var resourceIndexNode = new ResourceIndexNode(resourceDataNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.BlobIdResourceIndex), resourceIndexNode, resourceIndexNode, resourceIndexNode.EndSymbol);
           
-            var typeMapNode = new TypeMetadataMapNode(_commonFixupsTableNode);
+            var typeMapNode = new TypeMetadataMapNode(commonFixupsTableNode);
 
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.TypeMap), typeMapNode, typeMapNode, typeMapNode.EndSymbol);
 
-            var cctorContextMapNode = new ClassConstructorContextMap(_commonFixupsTableNode);
+            var cctorContextMapNode = new ClassConstructorContextMap(commonFixupsTableNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.CCtorContextMap), cctorContextMapNode, cctorContextMapNode, cctorContextMapNode.EndSymbol);
 
-            var invokeMapNode = new ReflectionInvokeMapNode(_commonFixupsTableNode);
+            var invokeMapNode = new ReflectionInvokeMapNode(commonFixupsTableNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.InvokeMap), invokeMapNode, invokeMapNode, invokeMapNode.EndSymbol);
 
-            var arrayMapNode = new ArrayMapNode(_commonFixupsTableNode);
+            var arrayMapNode = new ArrayMapNode(commonFixupsTableNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.ArrayMap), arrayMapNode, arrayMapNode, arrayMapNode.EndSymbol);
 
-            var fieldMapNode = new ReflectionFieldMapNode(_commonFixupsTableNode);
+            var fieldMapNode = new ReflectionFieldMapNode(commonFixupsTableNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.FieldAccessMap), fieldMapNode, fieldMapNode, fieldMapNode.EndSymbol);
 
-            NativeLayoutInfo = new NativeLayoutInfoNode(_nativeReferencesTableNode);
+            NativeLayoutInfo = new NativeLayoutInfoNode(nativeReferencesTableNode);
             header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.NativeLayoutInfo), NativeLayoutInfo, NativeLayoutInfo, NativeLayoutInfo.EndSymbol);
 
-            _exactMethodInstantiations = new ExactMethodInstantiationsNode(_nativeReferencesTableNode);
-            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.ExactMethodInstantiationsHashtable), _exactMethodInstantiations, _exactMethodInstantiations, _exactMethodInstantiations.EndSymbol);
+            var exactMethodInstantiations = new ExactMethodInstantiationsNode(nativeReferencesTableNode);
+            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.ExactMethodInstantiationsHashtable), exactMethodInstantiations, exactMethodInstantiations, exactMethodInstantiations.EndSymbol);
 
-            _genericsHashtable = new GenericsHashtableNode(_nativeReferencesTableNode);
-            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.GenericsHashtable), _genericsHashtable, _genericsHashtable, _genericsHashtable.EndSymbol);
+            var genericsHashtable = new GenericsHashtableNode(nativeReferencesTableNode);
+            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.GenericsHashtable), genericsHashtable, genericsHashtable, genericsHashtable.EndSymbol);
 
             // This one should go last
-            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.CommonFixupsTable), _commonFixupsTableNode, _commonFixupsTableNode, _commonFixupsTableNode.EndSymbol);
-            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.NativeReferences), _nativeReferencesTableNode, _nativeReferencesTableNode, _nativeReferencesTableNode.EndSymbol);
+            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.CommonFixupsTable), commonFixupsTableNode, commonFixupsTableNode, commonFixupsTableNode.EndSymbol);
+            header.Add(BlobIdToReadyToRunSection(ReflectionMapBlob.NativeReferences), nativeReferencesTableNode, nativeReferencesTableNode, nativeReferencesTableNode.EndSymbol);
         }
 
         private void Graph_NewMarkedNode(DependencyNodeCore<NodeFactory> obj)
@@ -122,7 +116,6 @@ namespace ILCompiler
             {
                 _typesWithEETypesGenerated.Add(eetypeNode.Type);
                 AddGeneratedType(eetypeNode.Type);
-                _genericsHashtable.AddEntryIfEligible(_nodeFactory, eetypeNode.Type);
                 return;
             }
 
@@ -140,7 +133,6 @@ namespace ILCompiler
                 }
 
                 AddGeneratedType(method.OwningType);
-                _exactMethodInstantiations.AddEntryIfEligible(_nodeFactory, method);
                 _methodDefinitionsGenerated.Add(method.GetTypicalMethodDefinition());
                 _methodsGenerated.Add(method);
                 return;
@@ -392,11 +384,21 @@ namespace ILCompiler
             return _arrayTypesGenerated;
         }
 
+        internal IEnumerable<MethodDesc> GetCompiledMethods()
+        {
+            return _methodsGenerated;
+        }
+
         internal bool TypeGeneratesEEType(TypeDesc type)
         {
             return _typesWithEETypesGenerated.Contains(type);
         }
 
+        internal IEnumerable<TypeDesc> GetTypesWithEETypes()
+        {
+            return _typesWithEETypesGenerated;
+        }
+
         private struct DummyMetadataPolicy : IMetadataPolicy
         {
             private MetadataGeneration _parent;