From: Anubhav Srivastava Date: Fri, 13 Dec 2019 03:14:19 +0000 (-0800) Subject: Miscellaneous performance fixes for Crossgen2 (#758) X-Git-Tag: submit/tizen/20210909.063632~10693 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d850517b5e525e88f2d2400fbbc2922b624efb2a;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Miscellaneous performance fixes for Crossgen2 (#758) * 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. --- diff --git a/src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs b/src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs index fe5b722..b479912 100644 --- a/src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs +++ b/src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs @@ -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 /// /// Dictionary given a mangled name for a given /// - private ImmutableDictionary _mangledTypeNames = ImmutableDictionary.Empty; + private Dictionary _mangledTypeNames = new Dictionary(); /// /// Given a set of names check if @@ -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 _mangledMethodNames = ImmutableDictionary.Empty; - private ImmutableDictionary _unqualifiedMangledMethodNames = ImmutableDictionary.Empty; + private Dictionary _mangledMethodNames = new Dictionary(); + private Dictionary _unqualifiedMangledMethodNames = new Dictionary(); 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 _mangledFieldNames = ImmutableDictionary.Empty; + private Dictionary _mangledFieldNames = new Dictionary(); 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 _mangledStringLiterals = ImmutableDictionary.Empty; + private Dictionary _mangledStringLiterals = new Dictionary(); 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; diff --git a/src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs b/src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs index c2b2566..0bb6f95 100644 --- a/src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs +++ b/src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs @@ -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; } diff --git a/src/coreclr/src/tools/ReadyToRun.SuperIlc/CommandLineOptions.cs b/src/coreclr/src/tools/ReadyToRun.SuperIlc/CommandLineOptions.cs index 99171fa..dc90efc 100644 --- a/src/coreclr/src/tools/ReadyToRun.SuperIlc/CommandLineOptions.cs +++ b/src/coreclr/src/tools/ReadyToRun.SuperIlc/CommandLineOptions.cs @@ -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()); - Option GenerateMapFile() => + Option Map() => new Option(new[] { "--map" }, "Generate a map file (Crossgen2)", new Argument()); Option DegreeOfParallelism() => diff --git a/src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs b/src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs index 7f14e4a..b65dd62 100644 --- a/src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs +++ b/src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs @@ -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"; } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedFieldRvaNode.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedFieldRvaNode.cs index cd4b3e7..25448fc 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedFieldRvaNode.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedFieldRvaNode.cs @@ -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(); MetadataReader metadataReader = _module.MetadataReader; BlobReader metadataBlob = new BlobReader(_module.PEReader.GetMetadata().Pointer, _module.PEReader.GetMetadata().Length); metadataBlob.Offset = metadataReader.GetTableMetadataOffset(TableIndex.FieldRva); - ImmutableArray 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; } diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs index d5f828c..be9fbec 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs @@ -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; diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs index 1136952..3cd4c3b 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/GenericLookupSignature.cs @@ -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; diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs index d037cac..88b3ec6 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs @@ -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; diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs index cef3ab7..d5f645a 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodFixupSignature.cs @@ -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; diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleToken.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleToken.cs index 3e86bfb..af2ea0b 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleToken.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ModuleToken.cs @@ -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); diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs index 86b115a..7bf36ed 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs @@ -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; diff --git a/src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs b/src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs index 2d63bdd..d82deef4 100644 --- a/src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs +++ b/src/coreclr/src/tools/crossgen2/crossgen2/CommandLineOptions.cs @@ -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; } diff --git a/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs b/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs index ee017ca..bfb7a22 100644 --- a/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs +++ b/src/coreclr/src/tools/crossgen2/crossgen2/Program.cs @@ -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)