Skip to content
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.

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

Determinism fixes #306

Merged
merged 10 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.IO;
using System.Diagnostics;
using System.Collections.Generic;
using System.Linq;
using System.Text;

// Managed mirror of NativeFormatWriter.h/.cpp
Expand Down Expand Up @@ -2041,13 +2042,9 @@ void ComputeLayout()
// we can use the ordering to terminate the lookup prematurely.
uint mask = ((_nBuckets - 1) << 8) | 0xFF;

// sort it by hashcode
_Entries.Sort(
(a, b) =>
{
return (int)(a.Hashcode & mask) - (int)(b.Hashcode & mask);
}
);
// Sort by hashcode. This sort must be stable since we need determinism even if two entries have
// the same hashcode. This is deterministic if entries are added in a deterministic order.
_Entries = _Entries.OrderBy(entry => entry.Hashcode & mask).ToList();

// Start with maximum size entries
_entryIndexSize = 2;
Expand Down
13 changes: 8 additions & 5 deletions src/coreclr/src/tools/Common/TypeSystem/IL/EcmaMethodIL.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
using System.Collections.Immutable;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;

using System.Threading;
using Internal.TypeSystem;
using Internal.TypeSystem.Ecma;

Expand Down Expand Up @@ -64,8 +64,8 @@ public override byte[] GetILBytes()
if (_ilBytes != null)
return _ilBytes;

byte[] ilBytes = _methodBody.GetILBytes();
return (_ilBytes = ilBytes);
Interlocked.CompareExchange(ref _ilBytes, _methodBody.GetILBytes(), null);
return _ilBytes;
SrivastavaAnubhav marked this conversation as resolved.
Show resolved Hide resolved
}

public override bool IsInitLocals
Expand Down Expand Up @@ -97,7 +97,9 @@ public override LocalVariableDefinition[] GetLocals()

EcmaSignatureParser parser = new EcmaSignatureParser(_module, signatureReader);
LocalVariableDefinition[] locals = parser.ParseLocalsSignature();
return (_locals = locals);

Interlocked.CompareExchange(ref _locals, locals, null);
return _locals;
}

public override ILExceptionRegion[] GetExceptionRegions()
Expand Down Expand Up @@ -131,7 +133,8 @@ public override ILExceptionRegion[] GetExceptionRegions()
}
}

return (_ilExceptionRegions = ilExceptionRegions);
Interlocked.CompareExchange(ref _ilExceptionRegions, ilExceptionRegions, null);
return _ilExceptionRegions;
}

public override object GetObject(int token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.Metadata.Ecma335;

using Internal.Text;
using Internal.TypeSystem;
using Internal.TypeSystem.Ecma;

using Debug = System.Diagnostics.Debug;
Expand All @@ -36,9 +36,11 @@ public class ManifestMetadataTableNode : HeaderTableNode
private readonly Dictionary<string, int> _assemblyRefToModuleIdMap;

/// <summary>
/// Assembly references to store in the manifest metadata.
/// Map from module index to the AssemblyName for the module. This only contains modules
/// that were actually loaded and is populated by ModuleToIndex.
/// </summary>
private readonly List<AssemblyName> _manifestAssemblies;
private readonly Dictionary<int, AssemblyName> _moduleIdToAssemblyNameMap;


/// <summary>
/// Registered signature emitters.
Expand Down Expand Up @@ -69,7 +71,7 @@ public ManifestMetadataTableNode(EcmaModule inputModule, NodeFactory nodeFactory
: base(inputModule.Context.Target)
{
_assemblyRefToModuleIdMap = new Dictionary<string, int>();
_manifestAssemblies = new List<AssemblyName>();
_moduleIdToAssemblyNameMap = new Dictionary<int, AssemblyName>();
_signatureEmitters = new List<ISignatureEmitter>();
_nodeFactory = nodeFactory;

Expand All @@ -95,9 +97,22 @@ public void RegisterEmitter(ISignatureEmitter emitter)

public int ModuleToIndex(EcmaModule module)
{
if (!_nodeFactory.MarkingComplete)
{
// If we call this function before sorting is complete, we might have a determinism bug caused by
// compiling two functions in an arbitrary order and hence getting different module IDs.
throw new InvalidOperationException("Cannot get ModuleToIndex mapping until marking is complete.");
}

AssemblyName assemblyName = module.Assembly.GetName();
int assemblyRefIndex;
if (!_assemblyRefToModuleIdMap.TryGetValue(assemblyName.Name, out assemblyRefIndex))
{
assemblyRefIndex = _nextModuleId++;
_assemblyRefToModuleIdMap.Add(assemblyName.Name, assemblyRefIndex);
}

if (!_manifestAssemblies.Contains(assemblyName))
if (!_moduleIdToAssemblyNameMap.ContainsKey(assemblyRefIndex))
{
if (_emissionCompleted)
{
Expand All @@ -108,14 +123,9 @@ public int ModuleToIndex(EcmaModule module)
// the verification logic would be broken at runtime.
Debug.Assert(_nodeFactory.CompilationModuleGroup.VersionsWithModule(module));

_manifestAssemblies.Add(assemblyName);
if (!_assemblyRefToModuleIdMap.ContainsKey(assemblyName.Name))
_assemblyRefToModuleIdMap.Add(assemblyName.Name, _nextModuleId);

_nextModuleId++;
_moduleIdToAssemblyNameMap.Add(assemblyRefIndex, assemblyName);
}

return _assemblyRefToModuleIdMap[assemblyName.Name];
return assemblyRefIndex;
}

public override int ClassCode => 791828335;
Expand Down Expand Up @@ -166,8 +176,9 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
fieldList: MetadataTokens.FieldDefinitionHandle(1),
methodList: MetadataTokens.MethodDefinitionHandle(1));

foreach (AssemblyName assemblyName in _manifestAssemblies)
foreach (var idAndAssemblyName in _moduleIdToAssemblyNameMap.OrderBy(x => x.Key))
{
AssemblyName assemblyName = idAndAssemblyName.Value;
AssemblyFlags assemblyFlags = 0;
byte[] publicKeyOrToken;
if ((assemblyName.Flags & AssemblyNameFlags.PublicKey) != 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
ReadyToRunCodegenNodeFactory r2rFactory = (ReadyToRunCodegenNodeFactory)factory;
ObjectDataSignatureBuilder dataBuilder = new ObjectDataSignatureBuilder();
dataBuilder.AddSymbol(this);

EcmaModule targetModule = _signatureContext.GetTargetModule(_arrayType);
SignatureContext innerContext = dataBuilder.EmitFixup(r2rFactory, ReadyToRunFixupKind.NewArray, targetModule, _signatureContext);
dataBuilder.EmitTypeSignature(_arrayType, innerContext);
if (!relocsOnly)
{
dataBuilder.AddSymbol(this);

EcmaModule targetModule = _signatureContext.GetTargetModule(_arrayType);
SrivastavaAnubhav marked this conversation as resolved.
Show resolved Hide resolved
SignatureContext innerContext = dataBuilder.EmitFixup(r2rFactory, ReadyToRunFixupKind.NewArray, targetModule, _signatureContext);
dataBuilder.EmitTypeSignature(_arrayType, innerContext);
}

return dataBuilder.ToObjectData();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
ReadyToRunCodegenNodeFactory r2rFactory = (ReadyToRunCodegenNodeFactory)factory;
ObjectDataSignatureBuilder dataBuilder = new ObjectDataSignatureBuilder();
dataBuilder.AddSymbol(this);

dataBuilder.EmitFixup(r2rFactory, ReadyToRunFixupKind.StringHandle, _token.Module, _signatureContext);
dataBuilder.EmitUInt(_token.TokenRid);
if (!relocsOnly)
{
dataBuilder.AddSymbol(this);

dataBuilder.EmitFixup(r2rFactory, ReadyToRunFixupKind.StringHandle, _token.Module, _signatureContext);
dataBuilder.EmitUInt(_token.TokenRid);
}

return dataBuilder.ToObjectData();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public abstract class Compilation : ICompilation
protected readonly NodeFactory _nodeFactory;
protected readonly Logger _logger;
private readonly DevirtualizationManager _devirtualizationManager;
private ILCache _methodILCache;
protected ILCache _methodILCache;
private readonly HashSet<ModuleDesc> _modulesBeingInstrumented;


Expand Down Expand Up @@ -79,10 +79,6 @@ public bool CanInline(MethodDesc caller, MethodDesc callee)

public virtual MethodIL GetMethodIL(MethodDesc method)
{
// Flush the cache when it grows too big
if (_methodILCache.Count > 1000)
_methodILCache = new ILCache(_methodILCache.ILProvider, NodeFactory.CompilationModuleGroup);

return _methodILCache.GetOrCreateValue(method).MethodIL;
}

Expand All @@ -106,7 +102,7 @@ public bool IsModuleInstrumented(ModuleDesc module)
return _modulesBeingInstrumented.Contains(module);
}

private sealed class ILCache : LockFreeReaderHashtable<MethodDesc, ILCache.MethodILData>
public sealed class ILCache : LockFreeReaderHashtable<MethodDesc, ILCache.MethodILData>
{
public ILProvider ILProvider { get; }
private readonly CompilationModuleGroup _compilationModuleGroup;
Expand Down Expand Up @@ -146,7 +142,7 @@ protected override MethodILData CreateValueFromKey(MethodDesc key)
return new MethodILData() { Method = key, MethodIL = methodIL };
}

internal class MethodILData
public class MethodILData
{
public MethodDesc Method;
public MethodIL MethodIL;
Expand Down Expand Up @@ -301,6 +297,11 @@ protected override void ComputeDependencyNodeDependencies(List<DependencyNodeCor
}
}
}

if (_methodILCache.Count > 1000)
{
_methodILCache = new ILCache(_methodILCache.ILProvider, NodeFactory.CompilationModuleGroup);
}
}

public ISymbolNode GetFieldRvaData(FieldDesc field) => NodeFactory.CopiedFieldRva(field);
Expand Down
86 changes: 86 additions & 0 deletions src/coreclr/tests/src/readytorun/coreroot_determinism/Program.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;

internal class Program
{
public static bool IsTimestampByte(int i)
{
return i >= 136 && i < 140;
}

public static int CompareDLLs(string folder1, string folder2)
{
int result = 100;

// Check for files that failed compilation with one of the seeds but not the other
HashSet<string> uniqueFilenames = new HashSet<string>(Directory.GetFiles(folder1, "*.dll").Select(Path.GetFileName));
uniqueFilenames.SymmetricExceptWith(Directory.GetFiles(folder2, "*.dll").Select(Path.GetFileName));
foreach (string uniqueFilename in uniqueFilenames)
{
Console.WriteLine($"{uniqueFilename} was found in only one of the output folders.");
result = 1;
}

foreach (string filename in Directory.GetFiles(folder1, "*.dll").Select(Path.GetFileName))
{
if (uniqueFilenames.Contains(filename))
continue;

byte[] file1 = File.ReadAllBytes(Path.Combine(folder1, Path.GetFileName(filename)));
byte[] file2 = File.ReadAllBytes(Path.Combine(folder2, Path.GetFileName(filename)));

if (file1.Length != file2.Length)
{
Console.WriteLine(filename);
Console.WriteLine($"Expected ReadyToRun'd files to be identical but they have different sizes ({file1.Length} and {file2.Length})");
result = 1;
}

for (int i = 0; i < file1.Length; ++i)
{
if (file1[i] != file2[i] && !IsTimestampByte(i))
{
Console.WriteLine(filename);
Console.WriteLine($"Difference at non-timestamp byte {i}");
result = 1;
}
}

Console.WriteLine($"Files of length {file1.Length} were identical.");
}
return result;
}

public static string OSExeSuffix(string path) => (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? path + ".exe" : path);

public static void CompileWithSeed(int seed, string outDir)
{
string coreRootPath = Environment.GetEnvironmentVariable("CORE_ROOT");
string superIlcPath = Path.Combine(coreRootPath, "ReadyToRun.SuperIlc", OSExeSuffix("ReadyToRun.SuperIlc"));

Console.WriteLine($"================================== Compiling with seed {seed} ==================================");
Environment.SetEnvironmentVariable("CoreRT_DeterminismSeed", seed.ToString());
if (Directory.Exists(outDir))
{
Directory.Delete(outDir, true);
}
Directory.CreateDirectory(outDir);
ProcessStartInfo processStartInfo = new ProcessStartInfo(superIlcPath, $"compile-directory -cr {coreRootPath} -in {coreRootPath} --nojit --noexe --large-bubble --release --nocleanup -out {outDir}");
Process.Start(processStartInfo).WaitForExit();
}

public static int Main()
{
CompileWithSeed(1, "seed1");
CompileWithSeed(2, "seed2");
return CompareDLLs("seed1", "seed2");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="Current">
<PropertyGroup>
<OutputType>exe</OutputType>
<CLRTestKind>BuildAndRun</CLRTestKind>
<CLRTestPriority>0</CLRTestPriority>
<!-- Crossgen2 currently targets only x64 -->
<DisableProjectBuild Condition="'$(Platform)' != 'x64'">true</DisableProjectBuild>
<!-- Known not to work with GCStress for now: https://github.com/dotnet/coreclr/issues/26633 -->
<GCStressIncompatible>true</GCStressIncompatible>
<!-- This is an explicit crossgen test -->
<CrossGenTest>false</CrossGenTest>
<OldToolsVersion>2.0</OldToolsVersion>
</PropertyGroup>
<ItemGroup>
<Compile Include="Program.cs" />
</ItemGroup>
</Project>