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

Add infinite codegen stress feature to crossgen2 and fix memory usage issues found while doing so #74956

Merged
merged 8 commits into from
Sep 20, 2022
54 changes: 39 additions & 15 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ public static IEnumerable<PgoSchemaElem> ConvertTypeHandleHistogramsToCompactTyp
Dictionary<object, IntPtr> objectToHandle = new Dictionary<object, IntPtr>();
Dictionary<IntPtr, object> handleToObject = new Dictionary<IntPtr, object>();

ComputeJitPgoInstrumentationSchema(LocalObjectToHandle, pgoData, out var nativeSchema, out var instrumentationData);
MemoryStream memoryStreamInstrumentationData = new MemoryStream();
ComputeJitPgoInstrumentationSchema(LocalObjectToHandle, pgoData, out var nativeSchema, memoryStreamInstrumentationData);
var instrumentationData = memoryStreamInstrumentationData.ToArray();

for (int i = 0; i < pgoData.Length; i++)
{
Expand Down Expand Up @@ -653,7 +655,7 @@ private void CompileMethodCleanup()
_pgoResults.Clear();
}

private Dictionary<object, IntPtr> _objectToHandle = new Dictionary<object, IntPtr>();
private Dictionary<object, IntPtr> _objectToHandle = new Dictionary<object, IntPtr>(new JitObjectComparer());
private List<object> _handleToObject = new List<object>();

private const int handleMultiplier = 8;
Expand Down Expand Up @@ -989,7 +991,24 @@ private uint getMethodAttribsInternal(MethodDesc method)
if (method.IsIntrinsic)
result |= CorInfoFlag.CORINFO_FLG_INTRINSIC;
if (method.IsVirtual)
{
result |= CorInfoFlag.CORINFO_FLG_VIRTUAL;

// The JIT only cares about the sealed flag if the method is virtual, or if
// it is a delegate.

// method or class might have the final bit
if (method.IsUnboxingThunk())
{
if (_compilation.IsEffectivelySealed(method.GetUnboxedMethod()))
result |= CorInfoFlag.CORINFO_FLG_FINAL;
}
else
{
if (_compilation.IsEffectivelySealed(method))
result |= CorInfoFlag.CORINFO_FLG_FINAL;
}
}
if (method.IsAbstract)
result |= CorInfoFlag.CORINFO_FLG_ABSTRACT;
if (method.IsConstructor || method.IsStaticConstructor)
Expand All @@ -1000,10 +1019,6 @@ private uint getMethodAttribsInternal(MethodDesc method)
// method body.
//

// method or class might have the final bit
if (_compilation.IsEffectivelySealed(method))
result |= CorInfoFlag.CORINFO_FLG_FINAL;

if (method.IsSharedByGenericInstantiations)
result |= CorInfoFlag.CORINFO_FLG_SHAREDINST;

Expand Down Expand Up @@ -2237,18 +2252,26 @@ private uint getClassGClayout(CORINFO_CLASS_STRUCT_* cls, byte* gcPtrs)
return result;
}

private Dictionary<TypeDesc, uint> _classNumInstanceFields = new();

private uint getClassNumInstanceFields(CORINFO_CLASS_STRUCT_* cls)
{
TypeDesc type = HandleToObject(cls);

uint result = 0;
var lookupType = type.GetTypeDefinition(); // The number of fields on an instantiation is the same as on the generic definition

if (_classNumInstanceFields.TryGetValue(lookupType, out uint numInstanceFields))
return numInstanceFields;

numInstanceFields = 0;
foreach (var field in type.GetFields())
{
if (!field.IsStatic)
result++;
numInstanceFields++;
}

return result;
_classNumInstanceFields.Add(lookupType, numInstanceFields);
return numInstanceFields;
}

private CORINFO_FIELD_STRUCT_* getFieldInClass(CORINFO_CLASS_STRUCT_* clsHnd, int num)
Expand Down Expand Up @@ -3799,11 +3822,13 @@ private PgoSchemaElem[] getPgoInstrumentationResults(MethodDesc method)
return _compilation.ProfileData[method]?.SchemaData;
}

public static void ComputeJitPgoInstrumentationSchema(Func<object, IntPtr> objectToHandle, PgoSchemaElem[] pgoResultsSchemas, out PgoInstrumentationSchema[] nativeSchemas, out byte[] instrumentationData, Func<TypeDesc, bool> typeFilter = null)
private MemoryStream _cachedMemoryStream = new MemoryStream();

public static void ComputeJitPgoInstrumentationSchema(Func<object, IntPtr> objectToHandle, PgoSchemaElem[] pgoResultsSchemas, out PgoInstrumentationSchema[] nativeSchemas, MemoryStream instrumentationData, Func<TypeDesc, bool> typeFilter = null)
{
nativeSchemas = new PgoInstrumentationSchema[pgoResultsSchemas.Length];
MemoryStream msInstrumentationData = new MemoryStream();
BinaryWriter bwInstrumentationData = new BinaryWriter(msInstrumentationData);
instrumentationData.SetLength(0);
BinaryWriter bwInstrumentationData = new BinaryWriter(instrumentationData);
for (int i = 0; i < nativeSchemas.Length; i++)
{
if ((bwInstrumentationData.BaseStream.Position % 8) == 4)
Expand Down Expand Up @@ -3865,8 +3890,6 @@ public static void ComputeJitPgoInstrumentationSchema(Func<object, IntPtr> objec
}

bwInstrumentationData.Flush();

instrumentationData = msInstrumentationData.ToArray();
}

private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref PgoInstrumentationSchema* pSchema, ref uint countSchemaItems, byte** pInstrumentationData,
Expand All @@ -3884,13 +3907,14 @@ private HRESULT getPgoInstrumentationResults(CORINFO_METHOD_STRUCT_* ftnHnd, ref
else
{
#pragma warning disable SA1001, SA1113, SA1115 // Commas should be spaced correctly
ComputeJitPgoInstrumentationSchema(ObjectToHandle, pgoResultsSchemas, out var nativeSchemas, out var instrumentationData
ComputeJitPgoInstrumentationSchema(ObjectToHandle, pgoResultsSchemas, out var nativeSchemas, _cachedMemoryStream
#if !READYTORUN
, _compilation.CanConstructType
#endif
);
#pragma warning restore SA1001, SA1113, SA1115 // Commas should be spaced correctly

var instrumentationData = _cachedMemoryStream.ToArray();
pgoResults.pInstrumentationData = (byte*)GetPin(instrumentationData);
pgoResults.countSchemaItems = (uint)nativeSchemas.Length;
pgoResults.pSchema = (PgoInstrumentationSchema*)GetPin(nativeSchemas);
Expand Down
45 changes: 45 additions & 0 deletions src/coreclr/tools/Common/JitInterface/IJitHashableOnly.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using Internal.TypeSystem;

namespace Internal.JitInterface
{
public class JitObjectComparer : IEqualityComparer<object>
{
public new bool Equals(object x, object y) => x == y;
public int GetHashCode(object obj)
{
if (obj is IJitHashableOnly jitHashable)
return jitHashable.GetJitVisibleHashCode();
return obj.GetHashCode();
}
}

public class JitObjectComparer<T> : IEqualityComparer<T> where T:class
{
public bool Equals(T x, T y) => x == y;
public int GetHashCode(T obj)
{
if (obj is IJitHashableOnly jitHashable)
return jitHashable.GetJitVisibleHashCode();
return obj.GetHashCode();
}
}

// Mark a type system object with this interface to indicate that it
// can be hashed, but only by using the IJitHashableOnly interface.
// Implementors of this should throw an exception in their implementation of
// ComputeHashCode so that the normal GetHashCode function does not work.
// This is used to prevent putting these objects into long lived storage.
//
// The goal here is to make it difficult to accidentally store a type into
// another hashtable that isn't associated with the JIT itself, but still
// allow the jit side code use standard collections.
public interface IJitHashableOnly
{
int GetJitVisibleHashCode();
}
}
11 changes: 10 additions & 1 deletion src/coreclr/tools/Common/JitInterface/UnboxingMethodDesc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ namespace Internal.JitInterface
/// This class is for internal purposes within the JitInterface. It's not expected
/// for it to escape the JitInterface.
/// </summary>
internal sealed class UnboxingMethodDesc : MethodDelegator
internal sealed class UnboxingMethodDesc : MethodDelegator, IJitHashableOnly
{
private readonly UnboxingMethodDescFactory _factory;
private readonly int _jitVisibleHashCode;

public MethodDesc Target => _wrappedMethod;

Expand All @@ -24,6 +25,7 @@ public UnboxingMethodDesc(MethodDesc wrappedMethod, UnboxingMethodDescFactory fa
Debug.Assert(wrappedMethod.OwningType.IsValueType);
Debug.Assert(!wrappedMethod.Signature.IsStatic);
_factory = factory;
_jitVisibleHashCode = HashCode.Combine(wrappedMethod.GetHashCode(), 401752602);
}

public override MethodDesc GetCanonMethodTarget(CanonicalFormKind kind)
Expand Down Expand Up @@ -74,6 +76,13 @@ protected override int CompareToImpl(MethodDesc other, TypeSystemComparer compar
{
throw new NotImplementedException();
}

protected override int ComputeHashCode()
{
throw new NotSupportedException("This method may not be stored as it is expected to only be used transiently in the JIT");
}

int IJitHashableOnly.GetJitVisibleHashCode() => _jitVisibleHashCode;
#endif
}

Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/InstantiatedType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -368,5 +368,16 @@ public override IEnumerable<MetadataType> GetNestedTypes()
// Return the result from the typical type definition.
return _typeDef.GetNestedTypes();
}

public override TypeDesc UnderlyingType
{
get
{
if (!IsEnum)
return this;
else
return _typeDef.UnderlyingType;
}
}
}
}
1 change: 0 additions & 1 deletion src/coreclr/tools/Common/TypeSystem/Common/TypeDesc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,6 @@ public virtual TypeDesc UnderlyingType
if (!this.IsEnum)
return this;

// TODO: Cache the result?
foreach (var field in this.GetFields())
{
if (!field.IsStatic)
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,24 @@ public override IEnumerable<FieldDesc> GetFields()
}
}

public override TypeDesc UnderlyingType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe delete the TODO here:

If we're doing overrides, we're unlikely to benefit from a cache there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've removed the comment.

{
get
{
if (!IsEnum)
return this;

foreach (var handle in _typeDefinition.GetFields())
{
var field = (EcmaField)_module.GetObject(handle);
if (!field.IsStatic)
return field.FieldType;
}

return base.UnderlyingType; // Use the base implementation to get consistent error behavior
}
}

public override FieldDesc GetField(string name)
{
var metadataReader = this.MetadataReader;
Expand Down
Loading