Mark instance method bodies lazily

Instance method bodies do not need to be marked until an instance of the type could exist.

* Add MethodConvertedToThrow callback

* Update link xml preserve test expected behavior

src/linker/Linker.Steps/CodeRewriterStep.cs

@@ -45,16 +45,17 @@ void ProcessMethod (MethodDefinition method)
 			}
 		}
 
-		void RewriteBodyToLinkedAway (MethodDefinition method)
+		protected virtual void RewriteBodyToLinkedAway (MethodDefinition method)
 		{
 			method.ImplAttributes &= ~(MethodImplAttributes.AggressiveInlining | MethodImplAttributes.Synchronized);
 			method.ImplAttributes |= MethodImplAttributes.NoInlining;
 
 			method.Body = CreateThrowLinkedAwayBody (method);
+
 			ClearDebugInformation (method);
 		}
 
-		void RewriteBodyToStub (MethodDefinition method)
+		protected virtual void RewriteBodyToStub (MethodDefinition method)
 		{
 			if (!method.IsIL)
 				throw new NotImplementedException ();
@@ -64,7 +65,7 @@ void RewriteBodyToStub (MethodDefinition method)
 			ClearDebugInformation (method);
 		}
 
-		void RewriteBodyToFalse (MethodDefinition method)
+		protected virtual void RewriteBodyToFalse (MethodDefinition method)
 		{
 			if (!method.IsIL)
 				throw new NotImplementedException ();

src/linker/Linker.Steps/MarkStep.cs

@@ -46,6 +46,7 @@ public partial class MarkStep : IStep {
 		protected Queue<AttributeProviderPair> _assemblyLevelAttributes;
 		protected Queue<AttributeProviderPair> _lateMarkedAttributes;
 		protected List<TypeDefinition> _typesWithInterfaces;
+		protected List<MethodBody> _unreachableBodies;
 
 		public AnnotationStore Annotations {
 			get { return _context.Annotations; }
@@ -64,6 +65,7 @@ public MarkStep ()
 			_assemblyLevelAttributes = new Queue<AttributeProviderPair> ();
 			_lateMarkedAttributes = new Queue<AttributeProviderPair> ();
 			_typesWithInterfaces = new List<TypeDefinition> ();
+			_unreachableBodies = new List<MethodBody> ();
 		}
 
 		public virtual void Process (LinkContext context)
@@ -72,6 +74,7 @@ public virtual void Process (LinkContext context)
 
 			Initialize ();
 			Process ();
+			Complete ();
 		}
 
 		void Initialize ()
@@ -93,6 +96,13 @@ protected virtual void InitializeAssembly (AssemblyDefinition assembly)
 			}
 		}
 
+		void Complete ()
+		{
+			foreach (var body in _unreachableBodies) {
+				Annotations.SetAction (body.Method, MethodAction.ConvertToThrow);
+			}
+		}
+
 		void InitializeType (TypeDefinition type)
 		{
 			if (type.HasNestedTypes) {
@@ -188,6 +198,7 @@ bool ProcessPrimaryQueue ()
 				ProcessQueue ();
 				ProcessVirtualMethods ();
 				ProcessMarkedTypesWithInterfaces ();
+				ProcessPendingBodies ();
 				DoAdditionalProcessing ();
 			}
 
@@ -242,6 +253,17 @@ void ProcessMarkedTypesWithInterfaces ()
 			}
 		}
 
+		void ProcessPendingBodies ()
+		{
+			for (int i = 0; i < _unreachableBodies.Count; i++) {
+				var body = _unreachableBodies [i];
+				if (Annotations.IsInstantiated (body.Method.DeclaringType)) {
+					MarkMethodBody (body);
+					_unreachableBodies.RemoveAt (i--);
+				}
+			}
+		}
+
 		void ProcessVirtualMethod (MethodDefinition method)
 		{
 			var overrides = Annotations.GetOverrides (method);
@@ -1919,23 +1941,28 @@ void MarkNewCodeDependencies (MethodDefinition method)
 				break;
 
 			case MethodAction.ConvertToThrow:
-				if (_context.MarkedKnownMembers.NotSupportedExceptionCtorString != null)
-					break;
+				MarkAndCacheNotSupportedCtorString ();
+				break;
+			}
+		}
 
-				var nse = BCL.FindPredefinedType ("System", "NotSupportedException", _context);
-				if (nse == null)
-					throw new NotSupportedException ("Missing predefined 'System.NotSupportedException' type");
+		void MarkAndCacheNotSupportedCtorString ()
+		{
+			if (_context.MarkedKnownMembers.NotSupportedExceptionCtorString != null)
+				return;
 
-				MarkType (nse);
+			var nse = BCL.FindPredefinedType ("System", "NotSupportedException", _context);
+			if (nse == null)
+				throw new NotSupportedException ("Missing predefined 'System.NotSupportedException' type");
 
-				var nseCtor = MarkMethodIf (nse.Methods, KnownMembers.IsNotSupportedExceptionCtorString);
-				if (nseCtor == null)
-					throw new MarkException ($"Could not find constructor on '{nse.FullName}'");
+			MarkType (nse);
 
-				_context.MarkedKnownMembers.NotSupportedExceptionCtorString = nseCtor;
-				break;
-			}
-		}		
+			var nseCtor = MarkMethodIf (nse.Methods, KnownMembers.IsNotSupportedExceptionCtorString);
+			if (nseCtor == null)
+				throw new MarkException ($"Could not find constructor on '{nse.FullName}'");
+
+			_context.MarkedKnownMembers.NotSupportedExceptionCtorString = nseCtor;
+		}
 
 		void MarkBaseMethods (MethodDefinition method)
 		{
@@ -2068,6 +2095,12 @@ void MarkMethodIfNotNull (MethodReference method)
 
 		protected virtual void MarkMethodBody (MethodBody body)
 		{
+			if (_context.IsOptimizationEnabled (CodeOptimizations.UnreachableBodies) && IsUnreachableBody (body)) {
+				MarkAndCacheNotSupportedCtorString ();
+				_unreachableBodies.Add (body);
+				return;
+			}
+
 			foreach (VariableDefinition var in body.Variables)
 				MarkType (var.VariableType);
 
@@ -2085,6 +2118,14 @@ protected virtual void MarkMethodBody (MethodBody body)
 			PostMarkMethodBody (body);
 		}
 
+		bool IsUnreachableBody (MethodBody body)
+		{
+			return !body.Method.IsStatic
+				&& !Annotations.IsInstantiated (body.Method.DeclaringType)
+				&& MethodBodyScanner.IsWorthConvertingToThrow (body);
+		}
+
+
 		partial void PostMarkMethodBody (MethodBody body);
 
 		void MarkInterfacesNeededByBodyStack (MethodBody body)

src/linker/Linker.Steps/RemoveFeaturesStep.cs

@@ -100,7 +100,8 @@ void ExcludeEventSource (TypeDefinition type)
 					continue;
 				}
 
-				annotations.SetAction (method, MethodAction.ConvertToThrow);
+				if (MethodBodyScanner.IsWorthConvertingToThrow (method.Body))
+					annotations.SetAction (method, MethodAction.ConvertToThrow);
 			}
 		}
 
@@ -149,7 +150,7 @@ void ExcludeEventSourceImplementation (TypeDefinition type)
 					continue;
 				}
 
-				if (!skip)
+				if (!skip && MethodBodyScanner.IsWorthConvertingToThrow (method.Body))
 					annotations.SetAction (method, MethodAction.ConvertToThrow);
 			}
 		}
@@ -166,7 +167,8 @@ void ExcludeMonoCollation (TypeDefinition type)
 					{
 						foreach (var method in type.Methods)
 						{
-							annotations.SetAction(method, MethodAction.ConvertToThrow);
+							if (MethodBodyScanner.IsWorthConvertingToThrow (method.Body))
+								annotations.SetAction(method, MethodAction.ConvertToThrow);
 						}
 					}
 

src/linker/Linker/Driver.cs

@@ -363,6 +363,9 @@ public void Run (ILogger customLogger = null)
 						case "overrideremoval":
 							context.DisabledOptimizations |= CodeOptimizations.OverrideRemoval;
 							break;
+						case "unreachablebodies":
+							context.DisabledOptimizations |= CodeOptimizations.UnreachableBodies;
+							break;
 						}
 					}
 				}
@@ -522,6 +525,7 @@ static void Usage (string msg)
 			Console.WriteLine ("  --disable-opt <name>      Disable one of the default optimizations");
 			Console.WriteLine ("                              beforefieldinit: Unused static fields are removed if there is no static ctor");
 			Console.WriteLine ("                              overrideremoval: Overrides of virtual methods on types that are never instantiated are removed");
+			Console.WriteLine ("                              unreachablebodies: Instance methods that are marked but can never be entered are converted to throws");
 			Console.WriteLine ("  --exclude-feature <name>  Any code which has a feature <name> in linked assemblies will be removed");
 			Console.WriteLine ("                              com: Support for COM Interop");
 			Console.WriteLine ("                              etw: Event Tracing for Windows");

src/linker/Linker/LinkContext.cs

@@ -401,5 +401,10 @@ public enum CodeOptimizations
 		/// that do not get an instance constructor marked.
 		/// </summary>
 		OverrideRemoval = 1 << 1,
+
+		/// <summary>
+		/// Option to disable delaying marking of instance methods until an instance of that type could exist
+		/// </summary>
+		UnreachableBodies = 1 << 2
 	}
 }

src/linker/Linker/MethodBodyScanner.cs

@@ -6,6 +6,49 @@
 
 namespace Mono.Linker {
 	public static class MethodBodyScanner {
+		public static bool IsWorthConvertingToThrow (MethodBody body)
+		{
+			// Some bodies are cheaper size wise to leave alone than to convert to a throw
+			Instruction previousMeaningful = null;
+			int meaningfulCount = 0;
+			foreach (var ins in body.Instructions)  {
+				// Handle ignoring noops because because (1) it's a valid case to ignore
+				// and (2) When running the tests on .net core roslyn tosses in no ops
+				// and that leads to a difference in test results between mcs and .net framework csc.
+				if (ins.OpCode.Code == Code.Nop)
+					continue;
+
+				meaningfulCount++;
+
+				if (meaningfulCount == 1 && ins.OpCode.Code == Code.Ret)
+					return false;
+
+				if (meaningfulCount == 2 && ins.OpCode.Code == Code.Ret && previousMeaningful != null) {
+					if (previousMeaningful.OpCode.StackBehaviourPop == StackBehaviour.Pop0) {
+						switch (previousMeaningful.OpCode.StackBehaviourPush) {
+							case StackBehaviour.Pushi:
+							case StackBehaviour.Pushi8:
+							case StackBehaviour.Pushr4:
+							case StackBehaviour.Pushr8:
+								return false;
+						}
+
+						switch (previousMeaningful.OpCode.Code) {
+							case Code.Ldnull:
+								return false;
+						}
+					}
+				}
+
+				if (meaningfulCount >= 2)
+					return true;
+
+				previousMeaningful = ins;
+			}
+
+			return true;
+		}
+
 		public static IEnumerable<InterfaceImplementation> GetReferencedInterfaces (AnnotationStore annotations, MethodBody body)
 		{
 			var possibleStackTypes = AllPossibleStackTypes (body.Method);

test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/BaseRemovedEventSource.cs

@@ -49,6 +49,11 @@ public CustomCtorEventSource (int value)
 			"throw",
 		})]
 		protected override void OnEventCommand (EventCommandEventArgs command)
+		{
+			Removed2 ();
+		}
+
+		static void Removed2 ()
 		{
 		}
 

test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/BaseRemovedEventSourceEmptyBody.cs

@@ -0,0 +1,75 @@
+using System.Diagnostics.Tracing;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.BCLFeatures.ETW  {
+	[SetupLinkerArgument ("--exclude-feature", "etw")]
+	public class BaseRemovedEventSourceEmptyBody {
+		public static void Main ()
+		{
+			var b = CustomCtorEventSourceEmptyBody.Log.IsEnabled ();
+			if (b)
+				CustomCtorEventSourceEmptyBody.Log.SomeMethod ();
+		}
+	}
+
+	[Kept]
+	[KeptBaseType (typeof (EventSource))]
+	[KeptMember (".ctor()")]
+	[KeptMember (".cctor()")]
+	[EventSource (Name = "MyCompany")]
+	class CustomCtorEventSourceEmptyBody : EventSource {
+		public class Keywords {
+			public const EventKeywords Page = (EventKeywords)1;
+
+			public int Unused;
+		}
+
+		[Kept]
+		public static CustomCtorEventSourceEmptyBody Log = new MyEventSourceBasedOnCustomCtorEventSourceEmptyBody (1);
+
+		[Kept]
+		[ExpectedInstructionSequence (new []
+		{
+			"ldarg.0",
+			"call",
+			"ret",
+		})]
+		public CustomCtorEventSourceEmptyBody (int value)
+		{
+			Removed ();
+		}
+
+		[Kept]
+		protected override void OnEventCommand (EventCommandEventArgs command)
+		{
+			// Not converted to throw because the body is empty
+		}
+
+		[Kept]
+		[ExpectedInstructionSequence (new []
+		{
+			"ldstr",
+			"newobj",
+			"throw",
+		})]
+		[Event (8)]
+		public void SomeMethod ()
+		{
+			Removed ();
+		}
+
+		public void Removed ()
+		{
+		}
+	}
+
+	[Kept]
+	[KeptBaseType (typeof (CustomCtorEventSourceEmptyBody))]
+	class MyEventSourceBasedOnCustomCtorEventSourceEmptyBody : CustomCtorEventSourceEmptyBody {
+		[Kept]
+		public MyEventSourceBasedOnCustomCtorEventSourceEmptyBody (int value) : base (value)
+		{
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/BCLFeatures/ETW/Excluded.cs

@@ -40,6 +40,11 @@ public class Keywords {
 			"throw",
 		})]
 		protected override void OnEventCommand (EventCommandEventArgs command)
+		{
+			Removed2 ();
+		}
+
+		static void Removed2 ()
 		{
 		}