Skip to content

Support for specifying max depth of generic cycle #63435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 7, 2022
Merged

Support for specifying max depth of generic cycle #63435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11f2cbe
Support for specifying max generic cycle
hez2010 Jan 6, 2022
c9a3c00
Make cutoffPoint optional
hez2010 Jan 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CompilerTypeSystemContext.Aot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ namespace ILCompiler
{
partial class CompilerTypeSystemContext
{
// Chosen rather arbitrarily. For the app that I was looking at, cutoff point of 7 compiled
// more than 10 minutes on a release build of the compiler, and I lost patience.
// Cutoff point of 5 produced an 1.7 GB object file.
// Cutoff point of 4 produced an 830 MB object file.
// Cutoff point of 3 produced an 470 MB object file.
// We want this to be high enough so that it doesn't cut off too early. But also not too
// high because things that are recursive often end up expanding laterally as well
// through various other generic code the deep code calls into.
public const int DefaultGenericCycleCutoffPoint = 4;

public SharedGenericsConfiguration GenericsConfig
{
get;
Expand All @@ -28,7 +38,7 @@ public SharedGenericsConfiguration GenericsConfig
private ArrayOfTRuntimeInterfacesAlgorithm _arrayOfTRuntimeInterfacesAlgorithm;
private MetadataType _arrayOfTType;

public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode genericsMode, DelegateFeature delegateFeatures)
public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode genericsMode, DelegateFeature delegateFeatures, int genericCycleCutoffPoint = DefaultGenericCycleCutoffPoint)
: base(details)
{
_genericsMode = genericsMode;
Expand All @@ -38,6 +48,8 @@ public CompilerTypeSystemContext(TargetDetails details, SharedGenericsMode gener

_delegateInfoHashtable = new DelegateInfoHashtable(delegateFeatures);

_genericCycleDetector = new LazyGenericsSupport.GenericCycleDetector(genericCycleCutoffPoint);

GenericsConfig = new SharedGenericsConfiguration();
}

Expand Down Expand Up @@ -181,7 +193,7 @@ internal DefType GetClosestDefType(TypeDesc type)
return (DefType)type;
}

private readonly LazyGenericsSupport.GenericCycleDetector _genericCycleDetector = new LazyGenericsSupport.GenericCycleDetector();
private readonly LazyGenericsSupport.GenericCycleDetector _genericCycleDetector;

public void DetectGenericCycles(TypeSystemEntity owner, TypeSystemEntity referent)
{
Expand Down
133 changes: 65 additions & 68 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/ModuleCycleInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,69 @@ public bool FormsCycle(TypeSystemEntity owner)
TypeDesc ownerType = (owner as EcmaMethod)?.OwningType;
return _entitiesInCycles.Contains(owner) || (ownerType != null && _entitiesInCycles.Contains(ownerType));
}
}

private class CycleInfoHashtable : LockFreeReaderHashtable<EcmaModule, ModuleCycleInfo>
{
protected override bool CompareKeyToValue(EcmaModule key, ModuleCycleInfo value) => key == value.Module;
protected override bool CompareValueToValue(ModuleCycleInfo value1, ModuleCycleInfo value2) => value1.Module == value2.Module;
protected override int GetKeyHashCode(EcmaModule key) => key.GetHashCode();
protected override int GetValueHashCode(ModuleCycleInfo value) => value.Module.GetHashCode();

protected override ModuleCycleInfo CreateValueFromKey(EcmaModule key)
{
GraphBuilder gb = new GraphBuilder(key);
Graph<EcmaGenericParameter> graph = gb.Graph;

var formalsNeedingLazyGenerics = graph.ComputeVerticesInvolvedInAFlaggedCycle();
var entitiesNeedingLazyGenerics = new HashSet<TypeSystemEntity>();

foreach (EcmaGenericParameter formal in formalsNeedingLazyGenerics)
{
var formalDefinition = key.MetadataReader.GetGenericParameter(formal.Handle);
if (formal.Kind == GenericParameterKind.Type)
{
entitiesNeedingLazyGenerics.Add(key.GetType(formalDefinition.Parent));
}
else
{
entitiesNeedingLazyGenerics.Add(key.GetMethod(formalDefinition.Parent));
}
}

return new ModuleCycleInfo(key, entitiesNeedingLazyGenerics);
}
}

internal class GenericCycleDetector
{
private readonly CycleInfoHashtable _hashtable = new CycleInfoHashtable();

private readonly struct EntityPair : IEquatable<EntityPair>
{
public readonly TypeSystemEntity Owner;
public readonly TypeSystemEntity Referent;
public EntityPair(TypeSystemEntity owner, TypeSystemEntity referent)
=> (Owner, Referent) = (owner, referent);
public bool Equals(EntityPair other) => Owner == other.Owner && Referent == other.Referent;
public override bool Equals(object obj) => obj is EntityPair p && Equals(p);
public override int GetHashCode() => HashCode.Combine(Owner.GetHashCode(), Referent.GetHashCode());
}

// This is a set of entities that had actual problems that caused us to abort compilation
// somewhere.
// Would prefer this to be a ConcurrentHashSet but there isn't any. ModuleCycleInfo can be looked up
// from the key, but since this is a key/value pair, might as well use the value too...
private readonly ConcurrentDictionary<EntityPair, ModuleCycleInfo> _actualProblems = new ConcurrentDictionary<EntityPair, ModuleCycleInfo>();

private readonly int _cutoffPoint;

public GenericCycleDetector(int cutoffPoint)
{
_cutoffPoint = cutoffPoint;
}

// Chosen rather arbitrarily. For the app that I was looking at, cutoff point of 7 compiled
// more than 10 minutes on a release build of the compiler, and I lost patience.
// Cutoff point of 5 produced an 1.7 GB object file.
// Cutoff point of 4 produced an 830 MB object file.
// Cutoff point of 3 produced an 470 MB object file.
// We want this to be high enough so that it doesn't cut off too early. But also not too
// high because things that are recursive often end up expanding laterally as well
// through various other generic code the deep code calls into.
private const int CutoffPoint = 4;

public bool IsDeepPossiblyCyclicInstantiation(TypeSystemEntity entity)
private bool IsDeepPossiblyCyclicInstantiation(TypeSystemEntity entity)
{
if (entity is TypeDesc type)
{
Expand All @@ -57,7 +108,7 @@ public bool IsDeepPossiblyCyclicInstantiation(TypeSystemEntity entity)
}
}

public bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, List<TypeDesc> seenTypes = null)
private bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, List<TypeDesc> seenTypes = null)
{
switch (type.Category)
{
Expand All @@ -80,7 +131,7 @@ public bool IsDeepPossiblyCyclicInstantiation(TypeDesc type, List<TypeDesc> seen
count++;
}

if (count > CutoffPoint)
if (count > _cutoffPoint)
{
return true;
}
Expand Down Expand Up @@ -112,60 +163,6 @@ public bool IsDeepPossiblyCyclicInstantiation(MethodDesc method)
{
return IsDeepPossiblyCyclicInstantiation(method.Instantiation) || IsDeepPossiblyCyclicInstantiation(method.OwningType);
}
}

private class CycleInfoHashtable : LockFreeReaderHashtable<EcmaModule, ModuleCycleInfo>
{
protected override bool CompareKeyToValue(EcmaModule key, ModuleCycleInfo value) => key == value.Module;
protected override bool CompareValueToValue(ModuleCycleInfo value1, ModuleCycleInfo value2) => value1.Module == value2.Module;
protected override int GetKeyHashCode(EcmaModule key) => key.GetHashCode();
protected override int GetValueHashCode(ModuleCycleInfo value) => value.Module.GetHashCode();

protected override ModuleCycleInfo CreateValueFromKey(EcmaModule key)
{
GraphBuilder gb = new GraphBuilder(key);
Graph<EcmaGenericParameter> graph = gb.Graph;

var formalsNeedingLazyGenerics = graph.ComputeVerticesInvolvedInAFlaggedCycle();
var entitiesNeedingLazyGenerics = new HashSet<TypeSystemEntity>();

foreach (EcmaGenericParameter formal in formalsNeedingLazyGenerics)
{
var formalDefinition = key.MetadataReader.GetGenericParameter(formal.Handle);
if (formal.Kind == GenericParameterKind.Type)
{
entitiesNeedingLazyGenerics.Add(key.GetType(formalDefinition.Parent));
}
else
{
entitiesNeedingLazyGenerics.Add(key.GetMethod(formalDefinition.Parent));
}
}

return new ModuleCycleInfo(key, entitiesNeedingLazyGenerics);
}
}

internal class GenericCycleDetector
{
private readonly CycleInfoHashtable _hashtable = new CycleInfoHashtable();

private readonly struct EntityPair : IEquatable<EntityPair>
{
public readonly TypeSystemEntity Owner;
public readonly TypeSystemEntity Referent;
public EntityPair(TypeSystemEntity owner, TypeSystemEntity referent)
=> (Owner, Referent) = (owner, referent);
public bool Equals(EntityPair other) => Owner == other.Owner && Referent == other.Referent;
public override bool Equals(object obj) => obj is EntityPair p && Equals(p);
public override int GetHashCode() => HashCode.Combine(Owner.GetHashCode(), Referent.GetHashCode());
}

// This is a set of entities that had actual problems that caused us to abort compilation
// somewhere.
// Would prefer this to be a ConcurrentHashSet but there isn't any. ModuleCycleInfo can be looked up
// from the key, but since this is a key/value pair, might as well use the value too...
private readonly ConcurrentDictionary<EntityPair, ModuleCycleInfo> _actualProblems = new ConcurrentDictionary<EntityPair, ModuleCycleInfo>();

public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
{
Expand Down Expand Up @@ -196,7 +193,7 @@ public void DetectCycle(TypeSystemEntity owner, TypeSystemEntity referent)
{
// Just the presence of a cycle is not a problem, but once we start getting too deep,
// we need to cut our losses.
if (cycleInfo.IsDeepPossiblyCyclicInstantiation(referent))
if (IsDeepPossiblyCyclicInstantiation(referent))
{
_actualProblems.TryAdd(new EntityPair(owner, referent), cycleInfo);

Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/tools/aot/ILCompiler/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ internal class Program
private int _parallelism = Environment.ProcessorCount;
private string _instructionSet;
private string _guard;
private int _maxGenericCycle = CompilerTypeSystemContext.DefaultGenericCycleCutoffPoint;

private string _singleMethodTypeName;
private string _singleMethodName;
Expand Down Expand Up @@ -216,7 +217,7 @@ private ArgumentSyntax ParseCommandLine(string[] args)
syntax.DefineOptionList("nosinglewarnassembly", ref _singleWarnDisabledAssemblies, "Expand AOT/trimming warnings for given assembly");
syntax.DefineOptionList("directpinvoke", ref _directPInvokes, "PInvoke to call directly");
syntax.DefineOptionList("directpinvokelist", ref _directPInvokeLists, "File with list of PInvokes to call directly");

syntax.DefineOption("maxgenericcycle", ref _maxGenericCycle, "Max depth of generic cycle");
syntax.DefineOptionList("root", ref _rootedAssemblies, "Fully generate given assembly");
syntax.DefineOptionList("conditionalroot", ref _conditionallyRootedAssemblies, "Fully generate given assembly if it's used");
syntax.DefineOptionList("trim", ref _trimmedAssemblies, "Trim the specified assembly");
Expand Down Expand Up @@ -460,7 +461,7 @@ private int Run(string[] args)
var targetAbi = TargetAbi.CoreRT;
var targetDetails = new TargetDetails(_targetArchitecture, _targetOS, targetAbi, simdVectorLength);
CompilerTypeSystemContext typeSystemContext =
new CompilerTypeSystemContext(targetDetails, genericsMode, supportsReflection ? DelegateFeature.All : 0);
new CompilerTypeSystemContext(targetDetails, genericsMode, supportsReflection ? DelegateFeature.All : 0, _maxGenericCycle);

//
// TODO: To support our pre-compiled test tree, allow input files that aren't managed assemblies since
Expand Down