Skip to content

Commit

Permalink
Avoid delegate allocation in generic cycle detector (#79842)
Browse files Browse the repository at this point in the history 
Removes 1,000,000 (one million) allocations while compiling a hello world. We were allocating a delegate and a closure each time we saw a reference from something generic to something generic.

This replaces the delegate with a manual closure allocated on the stack. The number of stack allocated entries was chosen arbitrarily. The highest recursion we see in CoreLib is for 3 elements, so 4 includes a bit of a buffer.

Maybe it's a tiny bit less readable, but not that much.
  • Loading branch information
@MichalStrehovsky
MichalStrehovsky authored Dec 22, 2022
1 parent 9fbb4ef commit e467a5f
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 38 deletions.
107 changes: 87 additions & 20 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.ForEach.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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 System.Diagnostics;

using Internal.TypeSystem;
Expand All @@ -21,39 +22,32 @@ private sealed partial class GraphBuilder
/// looking to see if another other generic type formals are referenced within that type expression.
///
/// This method also records bindings for any generic instances it finds inside the tree expression.
/// Sometimes, this side-effect is all that's wanted - in such cases, invoke this method with a null collector.
/// Sometimes, this side-effect is all that's wanted - in such cases, invoke this method with a default collector.
/// </summary>
private void ForEachEmbeddedGenericFormal(TypeDesc typeExpression, Instantiation typeContext, Instantiation methodContext, System.Action<EcmaGenericParameter, bool> collector = null)
private void ForEachEmbeddedGenericFormal(TypeDesc typeExpression, Instantiation typeContext, Instantiation methodContext, ref EmbeddingStateList collector)
{
System.Action<EcmaGenericParameter, int> wrappedCollector =
delegate(EcmaGenericParameter embedded, int depth)
{
bool isProperEmbedding = (depth > 0);
collector?.Invoke(embedded, isProperEmbedding);
return;
};
ForEachEmbeddedGenericFormalWorker(typeExpression, typeContext, methodContext, wrappedCollector, depth: 0);
ForEachEmbeddedGenericFormalWorker(typeExpression, typeContext, methodContext, ref collector, depth: 0);
}

private void ForEachEmbeddedGenericFormalWorker(TypeDesc type, Instantiation typeContext, Instantiation methodContext, System.Action<EcmaGenericParameter, int> collector, int depth)
private void ForEachEmbeddedGenericFormalWorker(TypeDesc type, Instantiation typeContext, Instantiation methodContext, ref EmbeddingStateList collector, int depth)
{
switch (type.Category)
{
case TypeFlags.Array:
case TypeFlags.SzArray:
case TypeFlags.ByRef:
case TypeFlags.Pointer:
ForEachEmbeddedGenericFormalWorker(((ParameterizedType)type).ParameterType, typeContext, methodContext, collector, depth + 1);
ForEachEmbeddedGenericFormalWorker(((ParameterizedType)type).ParameterType, typeContext, methodContext, ref collector, depth + 1);
return;
case TypeFlags.FunctionPointer:
return;
case TypeFlags.SignatureMethodVariable:
var methodParam = (EcmaGenericParameter)methodContext[((SignatureMethodVariable)type).Index];
collector(methodParam, depth);
collector.Collect(methodParam, depth);
return;
case TypeFlags.SignatureTypeVariable:
var typeParam = (EcmaGenericParameter)typeContext[((SignatureTypeVariable)type).Index];
collector(typeParam, depth);
collector.Collect(typeParam, depth);
return;
default:
Debug.Assert(type.IsDefType);
Expand All @@ -71,22 +65,95 @@ private void ForEachEmbeddedGenericFormalWorker(TypeDesc type, Instantiation typ
TypeDesc genericTypeArgument = genericTypeArguments[i];

int newDepth = depth + 1;
collector.Push(static delegate (in EmbeddingState state, GraphBuilder builder, EcmaGenericParameter embedded, int depth)
{
bool isProperEmbedding = (depth > state.NewDepth);
builder.RecordBinding(state.GenericTypeParameter, embedded, isProperEmbedding);
}, genericTypeParameter, newDepth);
ForEachEmbeddedGenericFormalWorker(
genericTypeArgument,
typeContext,
methodContext,
delegate (EcmaGenericParameter embedded, int depth2)
{
collector(embedded, depth2);
bool isProperEmbedding = (depth2 > newDepth);
RecordBinding(genericTypeParameter, embedded, isProperEmbedding);
},
ref collector,
newDepth
);
collector.Pop();
}
return;
}
}

private delegate void Collector(in EmbeddingState state, GraphBuilder builder, EcmaGenericParameter embedded, int depth);

private struct EmbeddingState
{
private readonly Collector _collector;
public readonly EcmaGenericParameter GenericTypeParameter;
public readonly int NewDepth;

public EmbeddingState(Collector collector, EcmaGenericParameter genericTypeParameter, int newDepth)
=> (_collector, GenericTypeParameter, NewDepth) = (collector, genericTypeParameter, newDepth);

public void Invoke(GraphBuilder builder, EcmaGenericParameter embedded, int depth2) => _collector(this, builder, embedded, depth2);
}

private struct EmbeddingStateList
{
private int _numItems;

private GraphBuilder _builder;

private EmbeddingState _item0;
private EmbeddingState _item1;
private EmbeddingState _item2;
private EmbeddingState _item3;

private List<EmbeddingState> _overflow;

public EmbeddingStateList(GraphBuilder builder) => _builder = builder;

public void Push(Collector collector, EcmaGenericParameter genericTypeParameter, int newDepth)
{
EmbeddingState state = new EmbeddingState(collector, genericTypeParameter, newDepth);
switch (_numItems)
{
case 0: _item0 = state; break;
case 1: _item1 = state; break;
case 2: _item2 = state; break;
case 3: _item3 = state; break;
default:
(_overflow ??= new List<EmbeddingState>()).Add(state);
break;
}

_numItems++;
}

public void Pop()
{
if (_numItems > 4)
_overflow.RemoveAt(_overflow.Count - 1);
_numItems--;
}

public void Collect(EcmaGenericParameter embedded, int depth2)
{
int numItems = _numItems;
if (numItems > 0)
_item0.Invoke(_builder, embedded, depth2);
if (numItems > 1)
_item1.Invoke(_builder, embedded, depth2);
if (numItems > 2)
_item2.Invoke(_builder, embedded, depth2);
if (numItems > 3)
_item3.Invoke(_builder, embedded, depth2);
if (numItems > 4)
{
foreach (EmbeddingState state in _overflow)
state.Invoke(_builder, embedded, depth2);
}
}
}
}
}
}
36 changes: 20 additions & 16 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.MethodCall.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,30 @@ private void ProcessMethodCall(MethodDesc target, Instantiation typeContext, Ins
// are we embedding it into a more complex type.
for (int i = 0; i < genericTypeParameters.Length; i++)
{
var stateList = new EmbeddingStateList(this);
stateList.Push(static delegate(in EmbeddingState state, GraphBuilder builder, EcmaGenericParameter embedded, int depth)
{
// If we got here, we found a method with generic arity (either from itself or its declaring type or both)
// that invokes a generic method. The caller is binding one of the target's generic formals to a type expression
// involving one of the caller's own formals.
//
// e.g.
//
// void Caller<G>()
// {
// Target<IList<G>>();
// return;
// }
//
bool isProperEmbedding = depth > 0;
builder.RecordBinding(state.GenericTypeParameter, embedded, isProperEmbedding);
}, (EcmaGenericParameter)genericTypeParameters[i], newDepth: 0);

ForEachEmbeddedGenericFormal(
genericTypeArguments[i],
typeContext,
methodContext,
delegate(EcmaGenericParameter embedded, bool isProperEmbedding)
{
// If we got here, we found a method with generic arity (either from itself or its declaring type or both)
// that invokes a generic method. The caller is binding one of the target's generic formals to a type expression
// involving one of the caller's own formals.
//
// e.g.
//
// void Caller<G>()
// {
// Target<IList<G>>();
// return;
// }
//
RecordBinding((EcmaGenericParameter)genericTypeParameters[i], embedded, isProperEmbedding);
}
ref stateList
);
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/LazyGenerics/GraphBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ private void WalkAncestorTypes(EcmaType declaringType)

private void ProcessAncestorType(TypeDesc ancestorType, Instantiation typeContext)
{
ForEachEmbeddedGenericFormal(ancestorType, typeContext, Instantiation.Empty);
var embeddingState = new EmbeddingStateList(this);
ForEachEmbeddedGenericFormal(ancestorType, typeContext, Instantiation.Empty, ref embeddingState);
}

private void LookForVirtualOverrides(EcmaMethod method)
Expand Down Expand Up @@ -281,7 +282,8 @@ private void WalkMethod(EcmaMethod method)
/// </summary>
private void ProcessTypeReference(TypeDesc typeReference, Instantiation typeContext, Instantiation methodContext)
{
ForEachEmbeddedGenericFormal(typeReference, typeContext, methodContext);
var embeddingState = new EmbeddingStateList(this);
ForEachEmbeddedGenericFormal(typeReference, typeContext, methodContext, ref embeddingState);
}

/// <summary>
Expand Down

0 comments on commit e467a5f

Please sign in to comment.