Miscellaneous performance fixes for Crossgen2 (#758)
authorAnubhav Srivastava <SrivastavaAnubhav@users.noreply.github.com>
Fri, 13 Dec 2019 03:14:19 +0000 (19:14 -0800)
committerFadi Hanna <fadim@microsoft.com>
Fri, 13 Dec 2019 03:14:19 +0000 (22:14 -0500)
* Miscellaneous performance fixes.
Create comparison functions for Enum (Enum.CompareTo boxes since it takes an Object).
In ModuleToken, use the existing MetadataReader instead of making a new one each time.
Use Dictionary instead of ImmutableDictionary in CoreRTNameMangler to reduce allocations.
In CopiedFieldRvaNode, only read as much of the section as we need to copy.

13 files changed:
src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs
src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs
src/coreclr/src/tools/ReadyToRun.SuperIlc/CommandLineOptions.cs
src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedFieldRvaNode.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleToken.cs
src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs
src/coreclr/src/tools/crossgen2/crossgen2/Program.cs

index fe5b722..b479912 100644 (file)
@@ -4,7 +4,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.Security.Cryptography;
 using System.Text;
 
@@ -144,7 +143,7 @@ namespace ILCompiler
         /// <summary>
         /// Dictionary given a mangled name for a given <see cref="TypeDesc"/>
         /// </summary>
-        private ImmutableDictionary<TypeDesc, string> _mangledTypeNames = ImmutableDictionary<TypeDesc, string>.Empty;
+        private Dictionary<TypeDesc, string> _mangledTypeNames = new Dictionary<TypeDesc, string>();
 
         /// <summary>
         /// Given a set of names <param name="set"/> check if <param name="origName"/>
@@ -166,11 +165,14 @@ namespace ILCompiler
 
         public override string GetMangledTypeName(TypeDesc type)
         {
-            string mangledName;
-            if (_mangledTypeNames.TryGetValue(type, out mangledName))
-                return mangledName;
+            lock (this)
+            {
+                string mangledName;
+                if (_mangledTypeNames.TryGetValue(type, out mangledName))
+                    return mangledName;
 
-            return ComputeMangledTypeName(type);
+                return ComputeMangledTypeName(type);
+            }
         }
 
         private string EnterNameScopeSequence => _mangleForCplusPlus ? "_A_" : "<";
@@ -273,12 +275,11 @@ namespace ILCompiler
                             // Ensure that name is unique and update our tables accordingly.
                             name = DisambiguateName(name, deduplicator);
                             deduplicator.Add(name);
-                            _mangledTypeNames = _mangledTypeNames.Add(t, name);
+                            _mangledTypeNames.Add(t, name);
                         }
                     }
+                    return _mangledTypeNames[type];
                 }
-
-                return _mangledTypeNames[type];
             }
 
             string mangledName;
@@ -351,14 +352,14 @@ namespace ILCompiler
             {
                 // Ensure that name is unique and update our tables accordingly.
                 if (!_mangledTypeNames.ContainsKey(type))
-                    _mangledTypeNames = _mangledTypeNames.Add(type, mangledName);
+                    _mangledTypeNames.Add(type, mangledName);
             }
 
             return mangledName;
         }
 
-        private ImmutableDictionary<MethodDesc, Utf8String> _mangledMethodNames = ImmutableDictionary<MethodDesc, Utf8String>.Empty;
-        private ImmutableDictionary<MethodDesc, Utf8String> _unqualifiedMangledMethodNames = ImmutableDictionary<MethodDesc, Utf8String>.Empty;
+        private Dictionary<MethodDesc, Utf8String> _mangledMethodNames = new Dictionary<MethodDesc, Utf8String>();
+        private Dictionary<MethodDesc, Utf8String> _unqualifiedMangledMethodNames = new Dictionary<MethodDesc, Utf8String>();
 
         public override Utf8String GetMangledMethodName(MethodDesc method)
         {
@@ -381,7 +382,7 @@ namespace ILCompiler
                 lock (this)
                 {
                     if (!_mangledMethodNames.ContainsKey(method))
-                        _mangledMethodNames = _mangledMethodNames.Add(method, utf8MangledName);
+                        _mangledMethodNames.Add(method, utf8MangledName);
                 }
 
                 return utf8MangledName;
@@ -390,11 +391,14 @@ namespace ILCompiler
 
         private Utf8String GetUnqualifiedMangledMethodName(MethodDesc method)
         {
-            Utf8String mangledName;
-            if (_unqualifiedMangledMethodNames.TryGetValue(method, out mangledName))
-                return mangledName;
+            lock (this)
+            {
+                Utf8String mangledName;
+                if (_unqualifiedMangledMethodNames.TryGetValue(method, out mangledName))
+                    return mangledName;
 
-            return ComputeUnqualifiedMangledMethodName(method);
+                return ComputeUnqualifiedMangledMethodName(method);
+            }
         }
 
         private Utf8String GetPrefixMangledTypeName(IPrefixMangledType prefixMangledType)
@@ -481,12 +485,11 @@ namespace ILCompiler
                             name = DisambiguateName(name, deduplicator);
                             deduplicator.Add(name);
 
-                            _unqualifiedMangledMethodNames = _unqualifiedMangledMethodNames.Add(m, name);
+                            _unqualifiedMangledMethodNames.Add(m, name);
                         }
                     }
+                    return _unqualifiedMangledMethodNames[method];
                 }
-
-                return _unqualifiedMangledMethodNames[method];
             }
 
             Utf8String utf8MangledName;
@@ -550,22 +553,25 @@ namespace ILCompiler
                 lock (this)
                 {
                     if (!_unqualifiedMangledMethodNames.ContainsKey(method))
-                        _unqualifiedMangledMethodNames = _unqualifiedMangledMethodNames.Add(method, utf8MangledName);
+                        _unqualifiedMangledMethodNames.Add(method, utf8MangledName);
                 }
             }
 
             return utf8MangledName;
         }
 
-        private ImmutableDictionary<FieldDesc, Utf8String> _mangledFieldNames = ImmutableDictionary<FieldDesc, Utf8String>.Empty;
+        private Dictionary<FieldDesc, Utf8String> _mangledFieldNames = new Dictionary<FieldDesc, Utf8String>();
 
         public override Utf8String GetMangledFieldName(FieldDesc field)
         {
-            Utf8String mangledName;
-            if (_mangledFieldNames.TryGetValue(field, out mangledName))
-                return mangledName;
+            lock (this)
+            {
+                Utf8String mangledName;
+                if (_mangledFieldNames.TryGetValue(field, out mangledName))
+                    return mangledName;
 
-            return ComputeMangledFieldName(field);
+                return ComputeMangledFieldName(field);
+            }
         }
 
         private Utf8String ComputeMangledFieldName(FieldDesc field)
@@ -594,12 +600,11 @@ namespace ILCompiler
                             if (prependTypeName != null)
                                 name = prependTypeName + "__" + name;
 
-                            _mangledFieldNames = _mangledFieldNames.Add(f, name);
+                            _mangledFieldNames.Add(f, name);
                         }
                     }
+                    return _mangledFieldNames[field];
                 }
-
-                return _mangledFieldNames[field];
             }
 
 
@@ -613,26 +618,29 @@ namespace ILCompiler
             lock (this)
             {
                 if (!_mangledFieldNames.ContainsKey(field))
-                    _mangledFieldNames = _mangledFieldNames.Add(field, utf8MangledName);
+                    _mangledFieldNames.Add(field, utf8MangledName);
             }
 
             return utf8MangledName;
         }
 
-        private ImmutableDictionary<string, string> _mangledStringLiterals = ImmutableDictionary<string, string>.Empty;
+        private Dictionary<string, string> _mangledStringLiterals = new Dictionary<string, string>();
 
         public override string GetMangledStringName(string literal)
         {
             string mangledName;
-            if (_mangledStringLiterals.TryGetValue(literal, out mangledName))
-                return mangledName;
+            lock (this)
+            {
+                if (_mangledStringLiterals.TryGetValue(literal, out mangledName))
+                    return mangledName;
+            }
 
             mangledName = SanitizeNameWithHash(literal);
 
             lock (this)
             {
                 if (!_mangledStringLiterals.ContainsKey(literal))
-                    _mangledStringLiterals = _mangledStringLiterals.Add(literal, mangledName);
+                    _mangledStringLiterals.Add(literal, mangledName);
             }
 
             return mangledName;
index c2b2566..0bb6f95 100644 (file)
@@ -21,7 +21,7 @@ namespace ReadyToRun.SuperIlc
         public bool NoExe { get; set; }
         public bool NoEtw { get; set; }
         public bool NoCleanup { get; set; }
-        public bool GenerateMapFile { get; set; }
+        public bool Map { get; set; }
         public FileInfo PackageList { get; set; }
         public int DegreeOfParallelism { get; set; }
         public bool Sequential { get; set; }
index 99171fa..dc90efc 100644 (file)
@@ -38,7 +38,7 @@ namespace ReadyToRun.SuperIlc
                         NoExe(),
                         NoEtw(),
                         NoCleanup(),
-                        GenerateMapFile(),
+                        Map(),
                         DegreeOfParallelism(),
                         Sequential(),
                         Framework(),
@@ -71,7 +71,7 @@ namespace ReadyToRun.SuperIlc
                         NoExe(),
                         NoEtw(),
                         NoCleanup(),
-                        GenerateMapFile(),
+                        Map(),
                         DegreeOfParallelism(),
                         Sequential(),
                         Framework(),
@@ -181,7 +181,7 @@ namespace ReadyToRun.SuperIlc
             Option NoCleanup() =>
                 new Option(new[] { "--nocleanup" }, "Don't clean up compilation artifacts after test runs", new Argument<bool>());
 
-            Option GenerateMapFile() =>
+            Option Map() =>
                 new Option(new[] { "--map" }, "Generate a map file (Crossgen2)", new Argument<bool>());
 
             Option DegreeOfParallelism() =>
index 7f14e4a..b65dd62 100644 (file)
@@ -49,7 +49,7 @@ namespace ReadyToRun.SuperIlc
             // Todo: Allow control of some of these
             yield return "--targetarch=x64";
 
-            if (_options.GenerateMapFile)
+            if (_options.Map)
             {
                 yield return "--map";
             }
index cd4b3e7..25448fc 100644 (file)
@@ -6,6 +6,7 @@ using System;
 using System.Collections.Immutable;
 using System.Reflection.Metadata;
 using System.Reflection.Metadata.Ecma335;
+using System.Reflection.PortableExecutable;
 using Internal.Text;
 using Internal.TypeSystem;
 using Internal.TypeSystem.Ecma;
@@ -55,14 +56,11 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         private unsafe byte[] GetRvaData(int targetPointerSize)
         {
             int size = 0;
-            byte[] result = Array.Empty<byte>();
 
             MetadataReader metadataReader = _module.MetadataReader;
             BlobReader metadataBlob = new BlobReader(_module.PEReader.GetMetadata().Pointer, _module.PEReader.GetMetadata().Length);
             metadataBlob.Offset = metadataReader.GetTableMetadataOffset(TableIndex.FieldRva);
 
-            ImmutableArray<byte> memBlock = _module.PEReader.GetSectionData(_rva).GetContent();
-
             for (int i = 1; i <= metadataReader.GetTableRowCount(TableIndex.FieldRva); i++)
             {
                 int currentFieldRva = metadataBlob.ReadInt32();
@@ -76,18 +74,20 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 int currentSize = field.FieldType.GetElementSize().AsInt;
                 if (currentSize > size)
                 {
-                    if (currentSize > memBlock.Length)
-                        throw new BadImageFormatException();
-
                     // We need to handle overlapping fields by reusing blobs based on the rva, and just update
                     // the size and contents
                     size = currentSize;
-                    result = new byte[AlignmentHelper.AlignUp(size, targetPointerSize)];
-                    memBlock.CopyTo(0, result, 0, size);
                 }
             }
 
             Debug.Assert(size > 0);
+
+            PEMemoryBlock block = _module.PEReader.GetSectionData(_rva);
+            if (block.Length < size)
+                throw new BadImageFormatException();
+
+            byte[] result = new byte[AlignmentHelper.AlignUp(size, targetPointerSize)];
+            block.GetContent(0, size).CopyTo(result);
             return result;
         }
 
index d5f828c..be9fbec 100644 (file)
@@ -59,7 +59,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             FieldFixupSignature otherNode = (FieldFixupSignature)other;
-            int result = _fixupKind.CompareTo(otherNode._fixupKind);
+            int result = ((int)_fixupKind).CompareTo((int)otherNode._fixupKind);
             if (result != 0)
                 return result;
 
index 1136952..3cd4c3b 100644 (file)
@@ -188,11 +188,11 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             GenericLookupSignature otherNode = (GenericLookupSignature)other;
-            int result = _runtimeLookupKind.CompareTo(otherNode._runtimeLookupKind);
+            int result = ((int)_runtimeLookupKind).CompareTo((int)otherNode._runtimeLookupKind);
             if (result != 0)
                 return result;
 
-            result = _fixupKind.CompareTo(otherNode._fixupKind);
+            result = ((int)_fixupKind).CompareTo((int)otherNode._fixupKind);
             if (result != 0)
                 return result;
 
index d037cac..88b3ec6 100644 (file)
@@ -74,7 +74,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             ImportThunk otherNode = (ImportThunk)other;
-            int result = _thunkKind.CompareTo(otherNode._thunkKind);
+            int result = ((int)_thunkKind).CompareTo((int)otherNode._thunkKind);
             if (result != 0)
                 return result;
 
index cef3ab7..d5f645a 100644 (file)
@@ -136,7 +136,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             MethodFixupSignature otherNode = (MethodFixupSignature)other;
-            int result = _fixupKind.CompareTo(otherNode._fixupKind);
+            int result = ((int)_fixupKind).CompareTo((int)otherNode._fixupKind);
             if (result != 0)
                 return result;
 
index 3e86bfb..af2ea0b 100644 (file)
@@ -67,7 +67,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         public int CompareTo(ModuleToken other)
         {
-            int result = Token.CompareTo(other.Token);
+            int result = ((int)Token).CompareTo((int)other.Token);
             if (result != 0)
                 return result;
 
@@ -79,7 +79,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             return new SignatureContext(Module, resolver);
         }
 
-        public MetadataReader MetadataReader => Module.PEReader.GetMetadataReader();
+        public MetadataReader MetadataReader => Module.MetadataReader;
 
         public CorTokenType TokenType => SignatureBuilder.TypeFromToken(Token);
 
index 86b115a..7bf36ed 100644 (file)
@@ -58,7 +58,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         public override int CompareToImpl(ISortableNode other, CompilerComparer comparer)
         {
             TypeFixupSignature otherNode = (TypeFixupSignature)other;
-            int result = _fixupKind.CompareTo(otherNode._fixupKind);
+            int result = ((int)_fixupKind).CompareTo((int)otherNode._fixupKind);
             if (result != 0)
                 return result;
 
index 2d63bdd..d82deef 100644 (file)
@@ -31,7 +31,7 @@ namespace ILCompiler
         public bool Tuning { get; set; }
         public bool Partial { get; set; }
         public bool Resilient { get; set; }
-        public bool GenerateMapFile { get; set; }
+        public bool Map { get; set; }
         public int Parallelism { get; set; }
 
 
index ee017ca..bfb7a22 100644 (file)
@@ -323,7 +323,7 @@ namespace ILCompiler
                     builder
                         .UseIbcTuning(_commandLineOptions.Tuning)
                         .UseResilience(_commandLineOptions.Resilient)
-                        .UseMapFile(_commandLineOptions.GenerateMapFile)
+                        .UseMapFile(_commandLineOptions.Map)
                         .UseParallelism(_commandLineOptions.Parallelism)
                         .UseILProvider(ilProvider)
                         .UseJitPath(_commandLineOptions.JitPath)