Skip to content

Commit

Permalink
Miscellaneous performance fixes for Crossgen2 (#758)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
SrivastavaAnubhav authored and Fadi Hanna committed Dec 13, 2019
1 parent b65649e commit d850517
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 57 deletions.
76 changes: 42 additions & 34 deletions src/coreclr/src/tools/Common/Compiler/CoreRTNameMangler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Security.Cryptography;
using System.Text;

Expand Down Expand Up @@ -144,7 +143,7 @@ private string SanitizeNameWithHash(string literal)
/// <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"/>
Expand All @@ -166,11 +165,14 @@ private string DisambiguateName(string origName, ISet<string> set)

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_" : "<";
Expand Down Expand Up @@ -273,12 +275,11 @@ private string ComputeMangledTypeName(TypeDesc type)
// 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;
Expand Down Expand Up @@ -351,14 +352,14 @@ private string ComputeMangledTypeName(TypeDesc type)
{
// 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)
{
Expand All @@ -381,7 +382,7 @@ public override Utf8String GetMangledMethodName(MethodDesc method)
lock (this)
{
if (!_mangledMethodNames.ContainsKey(method))
_mangledMethodNames = _mangledMethodNames.Add(method, utf8MangledName);
_mangledMethodNames.Add(method, utf8MangledName);
}

return utf8MangledName;
Expand All @@ -390,11 +391,14 @@ public override Utf8String GetMangledMethodName(MethodDesc method)

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)
Expand Down Expand Up @@ -481,12 +485,11 @@ private Utf8String ComputeUnqualifiedMangledMethodName(MethodDesc method)
name = DisambiguateName(name, deduplicator);
deduplicator.Add(name);

_unqualifiedMangledMethodNames = _unqualifiedMangledMethodNames.Add(m, name);
_unqualifiedMangledMethodNames.Add(m, name);
}
}
return _unqualifiedMangledMethodNames[method];
}

return _unqualifiedMangledMethodNames[method];
}

Utf8String utf8MangledName;
Expand Down Expand Up @@ -550,22 +553,25 @@ private Utf8String ComputeUnqualifiedMangledMethodName(MethodDesc method)
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)
Expand Down Expand Up @@ -594,12 +600,11 @@ private Utf8String ComputeMangledFieldName(FieldDesc field)
if (prependTypeName != null)
name = prependTypeName + "__" + name;

_mangledFieldNames = _mangledFieldNames.Add(f, name);
_mangledFieldNames.Add(f, name);
}
}
return _mangledFieldNames[field];
}

return _mangledFieldNames[field];
}


Expand All @@ -613,26 +618,29 @@ private Utf8String ComputeMangledFieldName(FieldDesc field)
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;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/tools/ReadyToRun.SuperIlc/BuildOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class BuildOptions
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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Command CompileFolder() =>
NoExe(),
NoEtw(),
NoCleanup(),
GenerateMapFile(),
Map(),
DegreeOfParallelism(),
Sequential(),
Framework(),
Expand Down Expand Up @@ -71,7 +71,7 @@ Command CompileSubtree() =>
NoExe(),
NoEtw(),
NoCleanup(),
GenerateMapFile(),
Map(),
DegreeOfParallelism(),
Sequential(),
Framework(),
Expand Down Expand Up @@ -181,7 +181,7 @@ Option NoEtw() =>
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() =>
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/tools/ReadyToRun.SuperIlc/CpaotRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ protected override IEnumerable<string> BuildCommandLineArguments(string assembly
// Todo: Allow control of some of these
yield return "--targetarch=x64";

if (_options.GenerateMapFile)
if (_options.Map)
{
yield return "--map";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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;
Expand Down Expand Up @@ -55,14 +56,11 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
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();
Expand All @@ -76,18 +74,20 @@ private unsafe byte[] GetRvaData(int targetPointerSize)
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ protected override string GetName(NodeFactory factory)
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public bool Equals(ModuleToken moduleToken)

public int CompareTo(ModuleToken other)
{
int result = Token.CompareTo(other.Token);
int result = ((int)Token).CompareTo((int)other.Token);
if (result != 0)
return result;

Expand All @@ -79,7 +79,7 @@ public SignatureContext SignatureContext(ModuleTokenResolver resolver)
return new SignatureContext(Module, resolver);
}

public MetadataReader MetadataReader => Module.PEReader.GetMetadataReader();
public MetadataReader MetadataReader => Module.MetadataReader;

public CorTokenType TokenType => SignatureBuilder.TypeFromToken(Token);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilde
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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class CommandLineOptions
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; }


Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/tools/crossgen2/crossgen2/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ private int Run()
builder
.UseIbcTuning(_commandLineOptions.Tuning)
.UseResilience(_commandLineOptions.Resilient)
.UseMapFile(_commandLineOptions.GenerateMapFile)
.UseMapFile(_commandLineOptions.Map)
.UseParallelism(_commandLineOptions.Parallelism)
.UseILProvider(ilProvider)
.UseJitPath(_commandLineOptions.JitPath)
Expand Down

0 comments on commit d850517

Please sign in to comment.