From aff061bab1b6d9ccd5731bd16fa8e89ad82ab75a Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Wed, 14 Feb 2024 15:00:43 -0800 Subject: [PATCH] Make sure ScopeStack doesn't box to IDisposable (#98396) 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. --- src/tools/illink/src/linker/BannedSymbols.txt | 2 + .../src/linker/Linker.Steps/MarkScopeStack.cs | 19 ++++++- .../src/linker/Linker.Steps/MarkStep.cs | 54 +++++++++---------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/tools/illink/src/linker/BannedSymbols.txt b/src/tools/illink/src/linker/BannedSymbols.txt index ada1bd0b4e105..f7e6db3c773b6 100644 --- a/src/tools/illink/src/linker/BannedSymbols.txt +++ b/src/tools/illink/src/linker/BannedSymbols.txt @@ -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 diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkScopeStack.cs b/src/tools/illink/src/linker/Linker.Steps/MarkScopeStack.cs index 00f5edec5379b..ab1ad448754fc 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkScopeStack.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkScopeStack.cs @@ -22,7 +22,7 @@ public Scope (in MessageOrigin origin) readonly Stack _scopeStack; - readonly struct LocalScope : IDisposable + internal readonly struct LocalScope : IDisposable { readonly MessageOrigin _origin; readonly MarkScopeStack _scopeStack; @@ -51,7 +51,7 @@ public void Dispose () } } - readonly struct ParentScope : IDisposable + internal readonly struct ParentScope : IDisposable { readonly Scope _parentScope; readonly Scope _childScope; @@ -78,6 +78,21 @@ public MarkScopeStack () _scopeStack = new Stack (); } + 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); diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index bb5e95e0a38dd..c7eb071913f59 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -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)); @@ -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; @@ -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 @@ -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 ()) { @@ -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: @@ -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); } } @@ -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); @@ -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--); @@ -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 (providerMember)) MarkDynamicDependency (dynamicDependency, providerMember); } @@ -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); @@ -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) { @@ -1607,7 +1607,7 @@ bool ProcessLateMarkedAttributes () } markOccurred = true; - using (ScopeStack.PushScope (scope)) { + using (ScopeStack.PushLocalScope (scope)) { MarkCustomAttribute (customAttribute, reason); } } @@ -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)); @@ -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); @@ -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 handleMarkType in MarkContext.MarkTypeActions) handleMarkType (type); @@ -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); } } @@ -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); @@ -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); } @@ -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)) @@ -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 || @@ -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); @@ -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); @@ -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)); @@ -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)); @@ -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); @@ -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));