Skip to content

Commit

Permalink
Stabilize summary blobs (#67086)
Browse files Browse the repository at this point in the history
I was trying to repro #66191 and assumed determinism is the root cause. I used `CoreRT_DeterminismSeed` to perturb the dependency node evaluation and it found differences.

Now, I don't think the differences really matter. We already seem to be deterministic in the multithreaded compilation (the outputs are always the same no matter how many times I try).

`CoreRT_DeterminismSeed` perturbs the order in which node dependencies are evaluated and that exposes more ordering issues than we practically have.

In normal multithreaded compilation we grab nodes in different order from the `NodeFactory` as part of method compilation, but the order in which those nodes are evaluated is still fixed. This means the nodes we grabbed are still getting marked in a deterministic order and we get stable ordering even without an explicit sort in this spot.

But maybe as part of https://github.com/dotnet/runtimelab/issues/1631 we should enable `CoreRT_Determinism` in our testing and for that we need this extra sorting.
  • Loading branch information
MichalStrehovsky committed Mar 25, 2022
1 parent 000e1a8 commit 343d8ee
Show file tree
Hide file tree
Showing 19 changed files with 40 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace ILCompiler.DependencyAnalysis
{
public class CompilerComparer : TypeSystemComparer, IComparer<ISortableNode>
{
public static new CompilerComparer Instance { get; } = new CompilerComparer();

public int Compare(ISortableNode x, ISortableNode y)
{
if (x == y)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;

using Debug = System.Diagnostics.Debug;

namespace Internal.TypeSystem
Expand All @@ -23,8 +25,10 @@ namespace Internal.TypeSystem
// to sort itself with respect to other instances of the same type.
// Comparisons between different categories of types are centralized to a single location that
// can provide rules to sort them.
public class TypeSystemComparer
public class TypeSystemComparer : IComparer<TypeDesc>, IComparer<MethodDesc>, IComparer<FieldDesc>, IComparer<MethodSignature>
{
public static TypeSystemComparer Instance { get; } = new TypeSystemComparer();

public int Compare(TypeDesc x, TypeDesc y)
{
if (x == y)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ private void ComputeLayout()
layout[index++] = entry;
}

var comparer = new GenericLookupResult.Comparer(new TypeSystemComparer());
var comparer = new GenericLookupResult.Comparer(TypeSystemComparer.Instance);
Array.Sort(layout, comparer.Compare);

// Only publish after the full layout is computed. Races are fine.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,9 @@ public int GetIdForMethod(MethodDesc dynamicInvokeMethod, NodeFactory factory)

private void BuildMethodToIdMap(NodeFactory factory)
{
// Get a sorted list of generated stubs
List<MethodDesc> methods = new List<MethodDesc>(factory.MetadataManager.GetDynamicInvokeTemplateMethods());

// Sort the stubs
var typeSystemComparer = new TypeSystemComparer();
methods.Sort((first, second) => typeSystemComparer.Compare(first, second));

// Assign each stub an ID
var methodToTemplateIndex = new Dictionary<MethodDesc, int>();
foreach (MethodDesc method in methods)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected override void GetElementDataForNodes(ref ObjectDataBuilder builder, No
private class DispatchCellComparer : IComparer<InterfaceDispatchCellNode>
{
private readonly NodeFactory _factory;
private readonly TypeSystemComparer _comparer = new TypeSystemComparer();
private readonly TypeSystemComparer _comparer = TypeSystemComparer.Instance;

public DispatchCellComparer(NodeFactory factory)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1089,12 +1089,12 @@ public string GetSymbolAlternateName(ISymbolNode node)
public ArrayOfEmbeddedPointersNode<GCStaticsNode> GCStaticsRegion = new ArrayOfEmbeddedPointersNode<GCStaticsNode>(
"__GCStaticRegionStart",
"__GCStaticRegionEnd",
new SortableDependencyNode.ObjectNodeComparer(new CompilerComparer()));
new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance));

public ArrayOfEmbeddedDataNode<ThreadStaticsNode> ThreadStaticsRegion = new ArrayOfEmbeddedDataNode<ThreadStaticsNode>(
"__ThreadStaticRegionStart",
"__ThreadStaticRegionEnd",
new SortableDependencyNode.EmbeddedObjectNodeComparer(new CompilerComparer()));
new SortableDependencyNode.EmbeddedObjectNodeComparer(CompilerComparer.Instance));

public ArrayOfEmbeddedPointersNode<IMethodNode> EagerCctorTable = new ArrayOfEmbeddedPointersNode<IMethodNode>(
"__EagerCctorStart",
Expand All @@ -1104,12 +1104,12 @@ public string GetSymbolAlternateName(ISymbolNode node)
public ArrayOfEmbeddedPointersNode<InterfaceDispatchMapNode> DispatchMapTable = new ArrayOfEmbeddedPointersNode<InterfaceDispatchMapNode>(
"__DispatchMapTableStart",
"__DispatchMapTableEnd",
new SortableDependencyNode.ObjectNodeComparer(new CompilerComparer()));
new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance));

public ArrayOfEmbeddedDataNode<EmbeddedObjectNode> FrozenSegmentRegion = new ArrayOfFrozenObjectsNode<EmbeddedObjectNode>(
"__FrozenSegmentRegionStart",
"__FrozenSegmentRegionEnd",
new SortableDependencyNode.EmbeddedObjectNodeComparer(new CompilerComparer()));
new SortableDependencyNode.EmbeddedObjectNodeComparer(CompilerComparer.Instance));

internal ModuleInitializerListNode ModuleInitializerList = new ModuleInitializerListNode();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public TypeGVMEntriesNode(TypeDesc associatedType)
_associatedType = associatedType;
}

public TypeDesc AssociatedType => _associatedType;

public override bool HasConditionalStaticDependencies => false;
public override bool HasDynamicDependencies => false;
public override bool InterestingForDynamicDependencyAnalysis => false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,18 @@ public abstract class MetadataManager : ICompilationRootProvider
protected readonly ManifestResourceBlockingPolicy _resourceBlockingPolicy;
protected readonly DynamicInvokeThunkGenerationPolicy _dynamicInvokeThunkGenerationPolicy;

private List<NonGCStaticsNode> _cctorContextsGenerated = new List<NonGCStaticsNode>();
private readonly HashSet<TypeDesc> _typesWithEETypesGenerated = new HashSet<TypeDesc>();
private readonly HashSet<TypeDesc> _typesWithConstructedEETypesGenerated = new HashSet<TypeDesc>();
private HashSet<MethodDesc> _methodsGenerated = new HashSet<MethodDesc>();
private HashSet<MethodDesc> _reflectableMethods = new HashSet<MethodDesc>();
private HashSet<GenericDictionaryNode> _genericDictionariesGenerated = new HashSet<GenericDictionaryNode>();
private HashSet<IMethodBodyNode> _methodBodiesGenerated = new HashSet<IMethodBodyNode>();
private List<TypeGVMEntriesNode> _typeGVMEntries = new List<TypeGVMEntriesNode>();
private HashSet<DefType> _typesWithDelegateMarshalling = new HashSet<DefType>();
private HashSet<DefType> _typesWithStructMarshalling = new HashSet<DefType>();
private HashSet<MethodDesc> _dynamicInvokeTemplates = new HashSet<MethodDesc>();
private readonly SortedSet<NonGCStaticsNode> _cctorContextsGenerated = new SortedSet<NonGCStaticsNode>(CompilerComparer.Instance);
private readonly SortedSet<TypeDesc> _typesWithEETypesGenerated = new SortedSet<TypeDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<TypeDesc> _typesWithConstructedEETypesGenerated = new SortedSet<TypeDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<MethodDesc> _methodsGenerated = new SortedSet<MethodDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<MethodDesc> _reflectableMethods = new SortedSet<MethodDesc>(TypeSystemComparer.Instance);
private readonly SortedSet<GenericDictionaryNode> _genericDictionariesGenerated = new SortedSet<GenericDictionaryNode>(CompilerComparer.Instance);
private readonly SortedSet<IMethodBodyNode> _methodBodiesGenerated = new SortedSet<IMethodBodyNode>(CompilerComparer.Instance);
private readonly SortedSet<TypeGVMEntriesNode> _typeGVMEntries
= new SortedSet<TypeGVMEntriesNode>(Comparer<TypeGVMEntriesNode>.Create((a, b) => TypeSystemComparer.Instance.Compare(a.AssociatedType, b.AssociatedType)));
private readonly SortedSet<DefType> _typesWithDelegateMarshalling = new SortedSet<DefType>(TypeSystemComparer.Instance);
private readonly SortedSet<DefType> _typesWithStructMarshalling = new SortedSet<DefType>(TypeSystemComparer.Instance);
private readonly SortedSet<MethodDesc> _dynamicInvokeTemplates = new SortedSet<MethodDesc>(TypeSystemComparer.Instance);
private HashSet<NativeLayoutTemplateMethodSignatureVertexNode> _templateMethodEntries = new HashSet<NativeLayoutTemplateMethodSignatureVertexNode>();

internal NativeLayoutInfoNode NativeLayoutInfo { get; private set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
definedSymbols: new ISymbolDefinitionNode[] { this });
}

_methods.MergeSort(new CompilerComparer());
_methods.MergeSort(CompilerComparer.Instance);
GCRefMapBuilder builder = new GCRefMapBuilder(factory.Target, relocsOnly);
builder.Builder.RequireInitialAlignment(4);
builder.Builder.AddSymbol(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class ImportSectionNode : EmbeddedObjectNode
{
private class ImportTable : ArrayOfEmbeddedDataNode<Import>
{
public ImportTable(string startSymbol, string endSymbol) : base(startSymbol, endSymbol, nodeSorter: new EmbeddedObjectNodeComparer(new CompilerComparer())) {}
public ImportTable(string startSymbol, string endSymbol) : base(startSymbol, endSymbol, nodeSorter: new EmbeddedObjectNodeComparer(CompilerComparer.Instance)) {}

public override bool ShouldSkipEmittingObjectNode(NodeFactory factory) => false;

Expand Down Expand Up @@ -45,7 +45,7 @@ public ImportSectionNode(string name, CorCompileImportType importType, CorCompil
_emitGCRefMap = emitGCRefMap;

_imports = new ImportTable(_name + "_ImportBegin", _name + "_ImportEnd");
_signatures = new ArrayOfEmbeddedPointersNode<Signature>(_name + "_SigBegin", _name + "_SigEnd", new EmbeddedObjectNodeComparer(new CompilerComparer()));
_signatures = new ArrayOfEmbeddedPointersNode<Signature>(_name + "_SigBegin", _name + "_SigEnd", new EmbeddedObjectNodeComparer(CompilerComparer.Instance));
_signatureList = new List<Signature>();
_gcRefMap = _emitGCRefMap ? new GCRefMapNode(this) : null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public class ImportSectionsTableNode : ArrayOfEmbeddedDataNode<ImportSectionNode
private bool _materializedSignature;

public ImportSectionsTableNode(NodeFactory r2rFactory)
: base("ImportSectionsTableStart", "ImportSectionsTableEnd", new EmbeddedObjectNodeComparer(new CompilerComparer()))
: base("ImportSectionsTableStart", "ImportSectionsTableEnd", new EmbeddedObjectNodeComparer(CompilerComparer.Instance))
{
_r2rFactory = r2rFactory;
_r2rFactory.ManifestMetadataTable.RegisterEmitter(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
public class ProfileDataSectionNode : ArrayOfEmbeddedDataNode<ProfileDataNode>
{
public ProfileDataSectionNode()
: base("ProfileDataSectionNode_Begin", "ProfileDataSectionNode_End", new EmbeddedObjectNodeComparer(new CompilerComparer()))
: base("ProfileDataSectionNode_Begin", "ProfileDataSectionNode_End", new EmbeddedObjectNodeComparer(CompilerComparer.Instance))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
public class RuntimeFunctionsGCInfoNode : ArrayOfEmbeddedDataNode<MethodGCInfoNode>
{
public RuntimeFunctionsGCInfoNode()
: base("RuntimeFunctionsGCInfo_Begin", "RuntimeFunctionsGCInfo_End", new EmbeddedObjectNodeComparer(new CompilerComparer()))
: base("RuntimeFunctionsGCInfo_Begin", "RuntimeFunctionsGCInfo_End", new EmbeddedObjectNodeComparer(CompilerComparer.Instance))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ public void AttachToDependencyGraph(DependencyAnalyzerBase<NodeFactory> graph)
if (methodsToInsertInstrumentationDataFor.Count != 0)
{
MethodDesc[] methodsToInsert = methodsToInsertInstrumentationDataFor.ToArray();
methodsToInsert.MergeSort(new TypeSystemComparer().Compare);
methodsToInsert.MergeSort(TypeSystemComparer.Instance.Compare);
InstrumentationDataTable = new InstrumentationDataTableNode(this, methodsToInsert, ProfileDataManager);
Header.Add(Internal.Runtime.ReadyToRunSectionType.PgoInstrumentationData, InstrumentationDataTable, InstrumentationDataTable);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ private void RewriteComponentFile(string inputFile, string outputFile, string ow
win32Resources: new Win32Resources.ResourceData(inputModule),
flags);

IComparer<DependencyNodeCore<NodeFactory>> comparer = new SortableDependencyNode.ObjectNodeComparer(new CompilerComparer());
IComparer<DependencyNodeCore<NodeFactory>> comparer = new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance);
DependencyAnalyzerBase<NodeFactory> componentGraph = new DependencyAnalyzer<NoLogStrategy<NodeFactory>, NodeFactory>(componentFactory, comparer);

componentGraph.AddRoot(componentFactory.Header, "Component module R2R header");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public override ICompilation ToCompilation()

factory.CompositeImageSettings = _compositeImageSettings;

IComparer<DependencyNodeCore<NodeFactory>> comparer = new SortableDependencyNode.ObjectNodeComparer(new CompilerComparer());
IComparer<DependencyNodeCore<NodeFactory>> comparer = new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance);
DependencyAnalyzerBase<NodeFactory> graph = CreateDependencyGraph(factory, comparer);

List<CorJitFlag> corJitFlags = new List<CorJitFlag> { CorJitFlag.CORJIT_FLAG_DEBUG_INFO };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public ImmutableArray<DependencyNodeCore<NodeFactory>> ApplyProfilerGuidedMethod
}

var newNodesArray = nodes.ToArray();
newNodesArray.MergeSortAllowDuplicates(new SortableDependencyNode.ObjectNodeComparer(new CompilerComparer()));
newNodesArray.MergeSortAllowDuplicates(new SortableDependencyNode.ObjectNodeComparer(CompilerComparer.Instance));
return newNodesArray.ToImmutableArray();

void ApplySortToDependencies(DependencyNodeCore<NodeFactory> node, int depth)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public IEnumerable<IMethodNode> GetCompiledMethods(EcmaModule moduleToEnumerate,
{
if (!_sortedMethods)
{
CompilerComparer comparer = new CompilerComparer();
CompilerComparer comparer = CompilerComparer.Instance;
SortableDependencyNode.ObjectNodeComparer objectNodeComparer = new SortableDependencyNode.ObjectNodeComparer(comparer);
Comparison<IMethodNode> sortHelper = (x, y) =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public override ICompilation ToCompilation()
var factory = new RyuJitNodeFactory(_context, _compilationGroup, _metadataManager, _interopStubManager, _nameMangler, _vtableSliceProvider, _dictionaryLayoutProvider, GetPreinitializationManager());

JitConfigProvider.Initialize(_context.Target, jitFlagBuilder.ToArray(), _ryujitOptions, _jitPath);
DependencyAnalyzerBase<NodeFactory> graph = CreateDependencyGraph(factory, new ObjectNode.ObjectNodeComparer(new CompilerComparer()));
DependencyAnalyzerBase<NodeFactory> graph = CreateDependencyGraph(factory, new ObjectNode.ObjectNodeComparer(CompilerComparer.Instance));
return new RyuJitCompilation(graph, factory, _compilationRoots, _ilProvider, _debugInformationProvider, _logger, _devirtualizationManager, _inliningPolicy ?? _compilationGroup, _instructionSetSupport, _profileDataManager, _methodImportationErrorProvider, options, _parallelism);
}
}
Expand Down

0 comments on commit 343d8ee

Please sign in to comment.