Skip to content

Commit

Permalink
Make sure ScopeStack doesn't box to IDisposable (dotnet#98396)
Browse files Browse the repository at this point in the history 
We allocate every time we encounter a using (ScopeStack.PushScope...) statement, which adds up to be a lot (~10mb in hello world, one of the most allocated objects). Instead, we can return the struct type directly so no boxing occurs.
  • Loading branch information
@jtschuster
jtschuster committed Feb 14, 2024
1 parent fbe2b06 commit aff061b
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 29 deletions.
2 changes: 2 additions & 0 deletions src/tools/illink/src/linker/BannedSymbols.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ P:Mono.Cecil.Cil.MethodBody.Instructions;Use LinkContext.GetMethodIL or BannedAp
P:Mono.Cecil.Cil.MethodBody.ExceptionHandlers;Use LinkContext.GetMethodIL or BannedApiExtensions.ExceptionHandlers(Mono.Linker.LinkContext) instead
P:Mono.Cecil.Cil.MethodBody.Variables;Use LinkContext.GetMethodIL or BannedApiExtensions.Variables(Mono.Linker.LinkContext) instead
M:Mono.Linker.Steps.ILProvider/MethodIL.Create;Use ILProvider GetMethodIL instead
M:Mono.Linker.Steps.MarkScopeStack.PushScope;Use PushLocalScope instead to avoid boxing
M:Mono.Linker.Steps.MarkScopeStack.PopToParent;Use PopToParentScope instead to avoid boxing
19 changes: 17 additions & 2 deletions src/tools/illink/src/linker/Linker.Steps/MarkScopeStack.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public Scope (in MessageOrigin origin)

readonly Stack<Scope> _scopeStack;

readonly struct LocalScope : IDisposable
internal readonly struct LocalScope : IDisposable
{
readonly MessageOrigin _origin;
readonly MarkScopeStack _scopeStack;
Expand Down Expand Up @@ -51,7 +51,7 @@ public void Dispose ()
}
}

readonly struct ParentScope : IDisposable
internal readonly struct ParentScope : IDisposable
{
readonly Scope _parentScope;
readonly Scope _childScope;
Expand All @@ -78,6 +78,21 @@ public MarkScopeStack ()
_scopeStack = new Stack<Scope> ();
}

internal LocalScope PushLocalScope (in MessageOrigin origin)
{
return new LocalScope (origin, this);
}

internal LocalScope PushLocalScope (in Scope scope)
{
return new LocalScope (scope, this);
}

internal ParentScope PopToParentScope ()
{
return new ParentScope (this);
}

public IDisposable PushScope (in MessageOrigin origin)
{
return new LocalScope (origin, this);
Expand Down
54 changes: 27 additions & 27 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ bool ProcessInternalsVisibleAttributes ()
Debug.Assert (attr.Provider is ModuleDefinition or AssemblyDefinition);
var assembly = (provider is ModuleDefinition module) ? module.Assembly : provider as AssemblyDefinition;

using var assemblyScope = ScopeStack.PushScope (new MessageOrigin (assembly));
using var assemblyScope = ScopeStack.PushLocalScope (new MessageOrigin (assembly));

if (!Annotations.IsMarked (attr.Attribute) && IsInternalsVisibleAttributeAssemblyMarked (attr.Attribute)) {
MarkCustomAttribute (attr.Attribute, new DependencyInfo (DependencyKind.AssemblyOrModuleAttribute, attr.Provider));
Expand Down Expand Up @@ -362,7 +362,7 @@ internal void MarkEntireType (TypeDefinition type, in DependencyInfo reason)

// Prevent cases where there's nothing on the stack (can happen when marking entire assemblies)
// In which case we would generate warnings with no source (hard to debug)
using var _ = ScopeStack.CurrentScope.Origin.Provider == null ? ScopeStack.PushScope (new MessageOrigin (type)) : null;
using MarkScopeStack.LocalScope? _ = ScopeStack.CurrentScope.Origin.Provider == null ? ScopeStack.PushLocalScope (new MessageOrigin (type)) : null;

if (!_entireTypesMarked.Add (type))
return;
Expand Down Expand Up @@ -451,7 +451,7 @@ bool MarkFullyPreservedAssemblies ()

// Setup empty scope - there has to be some scope setup since we're doing marking below
// but there's no "origin" right now (command line is the origin really)
using var localScope = ScopeStack.PushScope (new MessageOrigin ((ICustomAttributeProvider?) null));
using var localScope = ScopeStack.PushLocalScope (new MessageOrigin ((ICustomAttributeProvider?) null));

// Beware: this works on loaded assemblies, not marked assemblies, so it should not be tied to marking.
// We could further optimize this to only iterate through assemblies if the last mark iteration loaded
Expand Down Expand Up @@ -488,7 +488,7 @@ bool ProcessPrimaryQueue ()

bool ProcessMarkedPending ()
{
using var emptyScope = ScopeStack.PushScope (new MessageOrigin (null as ICustomAttributeProvider));
using var emptyScope = ScopeStack.PushLocalScope (new MessageOrigin (null as ICustomAttributeProvider));

bool marked = false;
foreach (var pending in Annotations.GetMarkedPending ()) {
Expand All @@ -498,7 +498,7 @@ bool ProcessMarkedPending ()
if (Annotations.IsProcessed (pending.Key))
continue;

using var localScope = ScopeStack.PushScope (pending.Value);
using var localScope = ScopeStack.PushLocalScope (pending.Value);

switch (pending.Key) {
case TypeDefinition type:
Expand Down Expand Up @@ -572,7 +572,7 @@ protected virtual void EnqueueMethod (MethodDefinition method, in DependencyInfo
void ProcessVirtualMethods ()
{
foreach ((var method, var scope) in _virtual_methods) {
using (ScopeStack.PushScope (scope)) {
using (ScopeStack.PushLocalScope (scope)) {
ProcessVirtualMethod (method);
}
}
Expand All @@ -597,7 +597,7 @@ void ProcessMarkedTypesWithInterfaces ()
// UnusedInterfaces optimization is turned off mark all interface implementations
bool unusedInterfacesOptimizationEnabled = Context.IsOptimizationEnabled (CodeOptimizations.UnusedInterfaces, type);

using (ScopeStack.PushScope (scope)) {
using (ScopeStack.PushLocalScope (scope)) {
if (Annotations.IsInstantiated (type) || Annotations.IsRelevantToVariantCasting (type) ||
!unusedInterfacesOptimizationEnabled) {
MarkInterfaceImplementations (type);
Expand Down Expand Up @@ -685,7 +685,7 @@ void ProcessPendingBodies ()
for (int i = 0; i < _unreachableBodies.Count; i++) {
(var body, var scope) = _unreachableBodies[i];
if (Annotations.IsInstantiated (body.Method.DeclaringType)) {
using (ScopeStack.PushScope (scope))
using (ScopeStack.PushLocalScope (scope))
MarkMethodBody (body);

_unreachableBodies.RemoveAt (i--);
Expand Down Expand Up @@ -874,7 +874,7 @@ void MarkCustomAttributes (ICustomAttributeProvider provider, in DependencyInfo
return;

IMemberDefinition providerMember = (IMemberDefinition) provider; ;
using (ScopeStack.PushScope (new MessageOrigin (providerMember)))
using (ScopeStack.PushLocalScope (new MessageOrigin (providerMember)))
foreach (var dynamicDependency in Annotations.GetLinkerAttributes<DynamicDependency> (providerMember))
MarkDynamicDependency (dynamicDependency, providerMember);
}
Expand Down Expand Up @@ -1407,7 +1407,7 @@ protected virtual void MarkAssembly (AssemblyDefinition assembly, DependencyInfo
if (CheckProcessed (assembly))
return;

using var assemblyScope = ScopeStack.PushScope (new MessageOrigin (assembly));
using var assemblyScope = ScopeStack.PushLocalScope (new MessageOrigin (assembly));

EmbeddedXmlInfo.ProcessDescriptors (assembly, Context);

Expand Down Expand Up @@ -1537,7 +1537,7 @@ bool ProcessLazyAttributes ()
Debug.Assert (provider is ModuleDefinition or AssemblyDefinition);
var assembly = (provider is ModuleDefinition module) ? module.Assembly : provider as AssemblyDefinition;

using var assemblyScope = ScopeStack.PushScope (new MessageOrigin (assembly));
using var assemblyScope = ScopeStack.PushLocalScope (new MessageOrigin (assembly));

var resolved = Context.Resolve (customAttribute.Constructor);
if (resolved == null) {
Expand Down Expand Up @@ -1607,7 +1607,7 @@ bool ProcessLateMarkedAttributes ()
}

markOccurred = true;
using (ScopeStack.PushScope (scope)) {
using (ScopeStack.PushLocalScope (scope)) {
MarkCustomAttribute (customAttribute, reason);
}
}
Expand Down Expand Up @@ -1788,7 +1788,7 @@ void MarkField (FieldDefinition field, in DependencyInfo reason, in MessageOrigi
// Use the original scope for marking the declaring type - it provides better warning message location
MarkType (field.DeclaringType, new DependencyInfo (DependencyKind.DeclaringType, field));

using var fieldScope = ScopeStack.PushScope (new MessageOrigin (field));
using var fieldScope = ScopeStack.PushLocalScope (new MessageOrigin (field));
MarkType (field.FieldType, new DependencyInfo (DependencyKind.FieldType, field));
MarkCustomAttributes (field, new DependencyInfo (DependencyKind.CustomAttribute, field));
MarkMarshalSpec (field, new DependencyInfo (DependencyKind.FieldMarshalSpec, field));
Expand Down Expand Up @@ -2007,7 +2007,7 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in
if (reference == null)
return null;

using var localScope = origin.HasValue ? ScopeStack.PushScope (origin.Value) : null;
using MarkScopeStack.LocalScope? localScope = origin.HasValue ? ScopeStack.PushLocalScope (origin.Value) : null;

(reference, reason) = GetOriginalType (reference, reason);

Expand Down Expand Up @@ -2053,7 +2053,7 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in
if (type.Scope is ModuleDefinition module)
MarkModule (module, new DependencyInfo (DependencyKind.ScopeOfType, type));

using var typeScope = ScopeStack.PushScope (new MessageOrigin (type));
using var typeScope = ScopeStack.PushLocalScope (new MessageOrigin (type));

foreach (Action<TypeDefinition> handleMarkType in MarkContext.MarkTypeActions)
handleMarkType (type);
Expand Down Expand Up @@ -2141,7 +2141,7 @@ internal void MarkStaticConstructorVisibleToReflection (TypeDefinition type, in
}
}
if (ShouldMarkTypeStaticConstructor (type) && reason.Kind != DependencyKind.TriggersCctorForCalledMethod) {
using (ScopeStack.PopToParent ())
using (ScopeStack.PopToParentScope ())
MarkStaticConstructor (type, new DependencyInfo (DependencyKind.CctorForType, type), ScopeStack.CurrentScope.Origin);
}
}
Expand Down Expand Up @@ -2440,7 +2440,7 @@ void MarkNamedProperty (TypeDefinition type, string property_name, in Dependency
if (property.Name != property_name)
continue;

using (ScopeStack.PushScope (new MessageOrigin (property))) {
using (ScopeStack.PushLocalScope (new MessageOrigin (property))) {
// This marks methods directly without reporting the property.
MarkMethod (property.GetMethod, reason, ScopeStack.CurrentScope.Origin);
MarkMethod (property.SetMethod, reason, ScopeStack.CurrentScope.Origin);
Expand Down Expand Up @@ -2804,7 +2804,7 @@ void MarkGenericArguments (IGenericInstance instance)
// The only two implementations of IGenericInstance both derive from MemberReference
Debug.Assert (instance is MemberReference);

using var _ = ScopeStack.CurrentScope.Origin.Provider == null ? ScopeStack.PushScope (new MessageOrigin (((MemberReference) instance).Resolve ())) : null;
using MarkScopeStack.LocalScope? _ = ScopeStack.CurrentScope.Origin.Provider == null ? ScopeStack.PushLocalScope (new MessageOrigin (((MemberReference) instance).Resolve ())) : null;
var scanner = new GenericArgumentDataFlow (Context, this, ScopeStack.CurrentScope.Origin);
scanner.ProcessGenericArgumentDataFlow (parameter, argument);
}
Expand Down Expand Up @@ -2832,7 +2832,7 @@ void MarkGenericArguments (IGenericInstance instance)

void ApplyPreserveInfo (TypeDefinition type)
{
using var typeScope = ScopeStack.PushScope (new MessageOrigin (type));
using var typeScope = ScopeStack.PushLocalScope (new MessageOrigin (type));

if (Annotations.TryGetPreserve (type, out TypePreserve preserve)) {
if (!Annotations.SetAppliedPreserve (type, preserve))
Expand Down Expand Up @@ -3203,8 +3203,8 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
throw new InternalErrorException ($"Unsupported method dependency {reason.Kind}");
#endif
ScopeStack.AssertIsEmpty ();
using var parentScope = ScopeStack.PushScope (new MarkScopeStack.Scope (origin));
using var methodScope = ScopeStack.PushScope (new MessageOrigin (method));
using var parentScope = ScopeStack.PushLocalScope (new MarkScopeStack.Scope (origin));
using var methodScope = ScopeStack.PushLocalScope (new MessageOrigin (method));

bool markedForCall =
reason.Kind == DependencyKind.DirectCall ||
Expand Down Expand Up @@ -3360,7 +3360,7 @@ protected virtual void MarkRequirementsForInstantiatedTypes (TypeDefinition type

Annotations.MarkInstantiated (type);

using var typeScope = ScopeStack.PushScope (new MessageOrigin (type));
using var typeScope = ScopeStack.PushLocalScope (new MessageOrigin (type));

MarkInterfaceImplementations (type);

Expand Down Expand Up @@ -3450,7 +3450,7 @@ bool MarkDisablePrivateReflectionAttribute ()
if (disablePrivateReflection == null)
throw new LinkerFatalErrorException (MessageContainer.CreateErrorMessage (null, DiagnosticId.CouldNotFindType, "System.Runtime.CompilerServices.DisablePrivateReflectionAttribute"));

using (ScopeStack.PushScope (new MessageOrigin (null as ICustomAttributeProvider))) {
using (ScopeStack.PushLocalScope (new MessageOrigin (null as ICustomAttributeProvider))) {
MarkType (disablePrivateReflection, DependencyInfo.DisablePrivateReflectionRequirement);

var ctor = MarkMethodIf (disablePrivateReflection.Methods, MethodDefinitionExtensions.IsDefaultConstructor, new DependencyInfo (DependencyKind.DisablePrivateReflectionRequirement, disablePrivateReflection), ScopeStack.CurrentScope.Origin);
Expand Down Expand Up @@ -3570,7 +3570,7 @@ protected internal void MarkProperty (PropertyDefinition prop, in DependencyInfo
if (!Annotations.MarkProcessed (prop, reason))
return;

using var propertyScope = ScopeStack.PushScope (new MessageOrigin (prop));
using var propertyScope = ScopeStack.PushLocalScope (new MessageOrigin (prop));

// Consider making this more similar to MarkEvent method?
MarkCustomAttributes (prop, new DependencyInfo (DependencyKind.CustomAttribute, prop));
Expand All @@ -3582,7 +3582,7 @@ protected internal virtual void MarkEvent (EventDefinition evt, in DependencyInf
if (!Annotations.MarkProcessed (evt, reason))
return;

using var eventScope = ScopeStack.PushScope (new MessageOrigin (evt));
using var eventScope = ScopeStack.PushLocalScope (new MessageOrigin (evt));

MarkCustomAttributes (evt, new DependencyInfo (DependencyKind.CustomAttribute, evt));

Expand Down Expand Up @@ -3681,7 +3681,7 @@ bool MarkAndCheckRequiresReflectionMethodBodyScanner (MethodIL methodIL)

requiresReflectionMethodBodyScanner =
ReflectionMethodBodyScanner.RequiresReflectionMethodBodyScannerForMethodBody (Context, methodIL.Method);
using var _ = ScopeStack.PushScope (new MessageOrigin (methodIL.Method));
using var _ = ScopeStack.PushLocalScope (new MessageOrigin (methodIL.Method));
foreach (Instruction instruction in methodIL.Instructions)
MarkInstruction (instruction, methodIL.Method, ref requiresReflectionMethodBodyScanner);

Expand Down Expand Up @@ -3832,7 +3832,7 @@ protected internal virtual void MarkInterfaceImplementation (InterfaceImplementa
return;
Annotations.MarkProcessed (iface, reason ?? new DependencyInfo (DependencyKind.InterfaceImplementationOnType, ScopeStack.CurrentScope.Origin.Provider));

using var localScope = origin.HasValue ? ScopeStack.PushScope (origin.Value) : null;
using MarkScopeStack.LocalScope? localScope = origin.HasValue ? ScopeStack.PushLocalScope (origin.Value) : null;

// Blame the type that has the interfaceimpl, expecting the type itself to get marked for other reasons.
MarkCustomAttributes (iface, new DependencyInfo (DependencyKind.CustomAttribute, iface));
Expand Down

0 comments on commit aff061b

Please sign in to comment.