From fccd43f0c224c6bb48cb63524b408f21047795cb Mon Sep 17 00:00:00 2001 From: Sebastien Pouliot Date: Wed, 21 Jul 2021 09:03:25 -0400 Subject: [PATCH] [dotnet][linker] Remove unused backing fields (#12001) Problems: * `Dispose` set the generated backing fields to `null` which means the linker will mark every backing fields, even if not used elsewhere (generally properties) inside the class * Backing fields increase the memory footprint of the managed peer instance (for the type and all it's subclasses) * Backing fields also increase the app size. Not a huge problem as they are all declared _weakly_ as `NSObject` but still... Solution: * When the linker process a `Dispose` method of an `NSObject` subclass with the _optimizable_ attribute then we remove the method body. This way the linker cannot mark the fields. * Before saving back the assemblies we replace the cached method body and NOP every field that were not marked by something else than the `Dispose` method. ```diff --- a.cs 2021-06-22 16:56:57.000000000 -0400 +++ b.cs 2021-06-22 16:57:00.000000000 -0400 @@ -3107,8 +3107,6 @@ private static readonly IntPtr class_ptr = Class.GetHandle("UIApplication"); - private object __mt_WeakDelegate_var; - public override IntPtr ClassHandle => class_ptr; [DllImport("__Internal")] @@ -3141,9 +3139,8 @@ protected override void Dispose(bool P_0) { base.Dispose(P_0); - if (base.Handle == IntPtr.Zero) + if (!(base.Handle == IntPtr.Zero)) { - __mt_WeakDelegate_var = null; } } } @@ -3209,10 +3206,6 @@ { private static readonly IntPtr class_ptr = Class.GetHandle("UIScreen"); - private object __mt_FocusedItem_var; - - private object __mt_FocusedView_var; - public override IntPtr ClassHandle => class_ptr; public virtual CGRect Bounds @@ -3242,10 +3235,8 @@ protected override void Dispose(bool P_0) { base.Dispose(P_0); - if (base.Handle == IntPtr.Zero) + if (!(base.Handle == IntPtr.Zero)) { - __mt_FocusedItem_var = null; - __mt_FocusedView_var = null; } } } @@ -3254,10 +3245,6 @@ { private static readonly IntPtr class_ptr = Class.GetHandle("UIView"); - private object __mt_ParentFocusEnvironment_var; - - private object __mt_PreferredFocusedView_var; - public override IntPtr ClassHandle => class_ptr; public virtual CGRect Bounds @@ -3303,10 +3290,8 @@ protected override void Dispose(bool P_0) { base.Dispose(P_0); - if (base.Handle == IntPtr.Zero) + if (!(base.Handle == IntPtr.Zero)) { - __mt_ParentFocusEnvironment_var = null; - __mt_PreferredFocusedView_var = null; } } } @@ -3315,12 +3300,6 @@ { private static readonly IntPtr class_ptr = Class.GetHandle("UIViewController"); - private object __mt_ParentFocusEnvironment_var; - - private object __mt_PreferredFocusedView_var; - - private object __mt_WeakTransitioningDelegate_var; - public override IntPtr ClassHandle => class_ptr; public virtual UIView View @@ -3363,11 +3342,8 @@ protected override void Dispose(bool P_0) { base.Dispose(P_0); - if (base.Handle == IntPtr.Zero) + if (!(base.Handle == IntPtr.Zero)) { - __mt_ParentFocusEnvironment_var = null; - __mt_PreferredFocusedView_var = null; - __mt_WeakTransitioningDelegate_var = null; } } } @@ -3376,8 +3352,6 @@ { private static readonly IntPtr class_ptr = Class.GetHandle("UIWindow"); - private object __mt_WindowScene_var; - public override IntPtr ClassHandle => class_ptr; public virtual UIViewController RootViewController @@ -3411,9 +3385,8 @@ protected override void Dispose(bool P_0) { base.Dispose(P_0); - if (base.Handle == IntPtr.Zero) + if (!(base.Handle == IntPtr.Zero)) { - __mt_WindowScene_var = null; } } } ``` * Do not consider bindings with `[Dispose (...)]` as optimizable Injected code makes it impossible for `bgen` to decide if it's optimizable (or not) Filed https://github.com/xamarin/xamarin-macios/issues/12150 with more details (and for other similar attributes) --- dotnet/targets/Xamarin.Shared.Sdk.targets | 1 + src/generator.cs | 13 ++- .../dotnet-linker/BackingFieldDelayHandler.cs | 105 ++++++++++++++++++ .../Steps/PreOutputDispatcher.cs | 5 +- tools/linker/CoreOptimizeGeneratedCode.cs | 4 +- 5 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 tools/dotnet-linker/BackingFieldDelayHandler.cs diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 86745895a1..53fd67e4de 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -459,6 +459,7 @@ --> <_TrimmerCustomSteps Include="$(_AdditionalTaskAssembly)" Condition="'$(_LinkMode)' != 'None'" Type="Xamarin.Linker.Steps.PreserveBlockCodeHandler" /> <_TrimmerCustomSteps Include="$(_AdditionalTaskAssembly)" Condition="'$(_LinkMode)' != 'None'" Type="Xamarin.Linker.OptimizeGeneratedCodeHandler" /> + <_TrimmerCustomSteps Include="$(_AdditionalTaskAssembly)" Condition="'$(_LinkMode)' != 'None'" Type="Xamarin.Linker.BackingFieldDelayHandler" /> <_TrimmerCustomSteps Include="$(_AdditionalTaskAssembly)" Condition="'$(_LinkMode)' != 'None'" Type="Xamarin.Linker.Steps.MarkDispatcher" /> <_TrimmerCustomSteps Include="$(_AdditionalTaskAssembly)" Condition="'$(_LinkMode)' != 'None'" Type="Xamarin.Linker.Steps.PreserveSmartEnumConversionsHandler" /> diff --git a/src/generator.cs b/src/generator.cs index b6871cd042..ee60a12ae0 100644 --- a/src/generator.cs +++ b/src/generator.cs @@ -3143,11 +3143,14 @@ public partial class Generator : IMemberGatherer { // this attribute allows the linker to be more clever in removing unused code in bindings - without risking breaking user code // only generate those for monotouch now since we can ensure they will be linked away before reaching the devices - public void GeneratedCode (StreamWriter sw, int tabs) + public void GeneratedCode (StreamWriter sw, int tabs, bool optimizable = true) { for (int i=0; i < tabs; i++) sw.Write ('\t'); - sw.WriteLine ("[BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]"); + sw.Write ("[BindingImpl (BindingImplOptions.GeneratedCode"); + if (optimizable) + sw.Write (" | BindingImplOptions.Optimizable"); + sw.WriteLine (")]"); } static void WriteIsDirectBindingCondition (StreamWriter sw, ref int tabs, bool? is_direct_binding, string is_direct_binding_value, Func trueCode, Func falseCode) @@ -3184,9 +3187,9 @@ public partial class Generator : IMemberGatherer { } } - public void print_generated_code () + public void print_generated_code (bool optimizable = true) { - GeneratedCode (sw, indent); + GeneratedCode (sw, indent, optimizable); } public void print (string format) @@ -7422,7 +7425,7 @@ public partial class Generator : IMemberGatherer { if (!is_static_class){ var disposeAttr = AttributeManager.GetCustomAttributes (type); if (disposeAttr.Length > 0 || instance_fields_to_clear_on_dispose.Count > 0){ - print_generated_code (); + print_generated_code (optimizable: disposeAttr.Length == 0); print ("protected override void Dispose (bool disposing)"); print ("{"); indent++; diff --git a/tools/dotnet-linker/BackingFieldDelayHandler.cs b/tools/dotnet-linker/BackingFieldDelayHandler.cs new file mode 100644 index 0000000000..d96496d317 --- /dev/null +++ b/tools/dotnet-linker/BackingFieldDelayHandler.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using Mono.Cecil; +using Mono.Cecil.Cil; +using Mono.Linker; +using Mono.Linker.Steps; +using Xamarin.Tuner; + +namespace Xamarin.Linker { + + // Problems: + // * `Dispose` set the generated backing fields to `null` which means + // the linker will mark every backing fields, even if not used + // elsewhere (generally properties) inside the class + // * Backing fields increase the memory footprint of the managed peer + // instance (for the type and all it's subclasses) + // * Backing fields also increase the app size. Not a huge problem as + // they are all declared _weakly_ as `NSObject` but still... + // + // Solution: + // * When the linker process a `Dispose` method of an `NSObject` + // subclass with the _optimizable_ attribute then we remove the + // method body. This way the linker cannot mark the fields. + // * Before saving back the assemblies we replace the cached method + // body and NOP every field that were not marked by something else + // than the `Dispose` method. + public class BackingFieldDelayHandler : ConfigurationAwareMarkHandler { + + protected override string Name { get; } = "Backing Fields Optimizer"; + protected override int ErrorCode { get; } = 2400; + + public override void Initialize (LinkContext context, MarkContext markContext) + { + base.Initialize (context); + markContext.RegisterMarkMethodAction (ProcessMethod); + } + + // cache `Dispose` body of optimization NSObject subclasses + static Dictionary dispose = new (); + + protected override void Process (MethodDefinition method) + { + if (!method.HasParameters || !method.IsVirtual || !method.HasBody) + return; + if (method.Name != "Dispose") + return; + // only process methods that are marked as optimizable + if (!method.IsBindingImplOptimizableCode (LinkContext)) + return; + var t = method.DeclaringType; + if (!t.IsNSObject (LinkContext)) + return; + + // keep original for later (if needed) + dispose.Add (method, method.Body); + + // setting body to null will only cause it to be reloaded again + // same if we don't get a new IL processor + // and we do not want that (as it would mark the fields) + var body = new MethodBody (method); + var il = body.GetILProcessor (); + il.Emit (OpCodes.Ret); + method.Body = body; + } + + public static void ReapplyDisposedFields (DerivedLinkContext context, string operation) + { + // note: all methods in the dictionary are marked (since they were added from an IMarkHandler) + foreach ((var method, var body) in dispose) { + foreach (var ins in body.Instructions) { + switch (ins.OpCode.OperandType) { + case OperandType.InlineField: + var field = (ins.Operand as FieldReference)?.Resolve (); + if (!context.Annotations.IsMarked (field)) { + var store_field = ins; + var load_null = ins.Previous; + var load_this = ins.Previous.Previous; + if (OptimizeGeneratedCodeHandler.ValidateInstruction (method, store_field, operation, Code.Stfld) && + OptimizeGeneratedCodeHandler.ValidateInstruction (method, load_null, operation, Code.Ldnull) && + OptimizeGeneratedCodeHandler.ValidateInstruction (method, load_this, operation, Code.Ldarg_0)) { + store_field.OpCode = OpCodes.Nop; + load_null.OpCode = OpCodes.Nop; + load_this.OpCode = OpCodes.Nop; + } + } + break; + } + } + method.Body = body; + } + } + } + + public class BackingFieldReintroductionSubStep : ExceptionalSubStep { + public override SubStepTargets Targets => SubStepTargets.Assembly; + protected override string Name => "Backing Field Reintroduction"; + protected override int ErrorCode { get; } = 2410; + + public override void Initialize (LinkContext context) + { + base.Initialize (context); + BackingFieldDelayHandler.ReapplyDisposedFields (LinkContext, Name); + } + } +} diff --git a/tools/dotnet-linker/Steps/PreOutputDispatcher.cs b/tools/dotnet-linker/Steps/PreOutputDispatcher.cs index 82693df4b3..9dbee29cba 100644 --- a/tools/dotnet-linker/Steps/PreOutputDispatcher.cs +++ b/tools/dotnet-linker/Steps/PreOutputDispatcher.cs @@ -4,7 +4,10 @@ using Xamarin.Linker; namespace Xamarin.Linker.Steps { class PreOutputDispatcher : SubStepsDispatcher { public PreOutputDispatcher () - : base (new [] { new RemoveUserResourcesSubStep () }) + : base (new BaseSubStep [] { + new RemoveUserResourcesSubStep (), + new BackingFieldReintroductionSubStep (), + }) { } } diff --git a/tools/linker/CoreOptimizeGeneratedCode.cs b/tools/linker/CoreOptimizeGeneratedCode.cs index 9ceb0defb1..926e8a7e45 100644 --- a/tools/linker/CoreOptimizeGeneratedCode.cs +++ b/tools/linker/CoreOptimizeGeneratedCode.cs @@ -218,7 +218,7 @@ namespace Xamarin.Linker { ins.Operand = null; } - protected static bool ValidateInstruction (MethodDefinition caller, Instruction ins, string operation, Code expected) + internal static bool ValidateInstruction (MethodDefinition caller, Instruction ins, string operation, Code expected) { if (ins.OpCode.Code != expected) { Driver.Log (1, "Could not {0} in {1} at offset {2}, expected {3} got {4}", operation, caller, ins.Offset, expected, ins); @@ -228,7 +228,7 @@ namespace Xamarin.Linker { return true; } - protected static bool ValidateInstruction (MethodDefinition caller, Instruction ins, string operation, params Code [] expected) + internal static bool ValidateInstruction (MethodDefinition caller, Instruction ins, string operation, params Code [] expected) { foreach (var code in expected) { if (ins.OpCode.Code == code)