[linker] move StripEmbeddedLibraries into the linker (#2162)

jonathanpeppers · jonpryor · commit 95aeb611cd61 · 2018-09-21T13:02:26.000-04:00
Fixes: #1092 The `<StripEmbeddedLibraries/>` MSBuild task can take 1-2 seconds, and it mainly removes `__AndroidLibraryProjects__.zip` from assemblies. If we moved this to happen during linking, it has various benefits: - The linker already has every assembly opened and loaded. - We know if the linker is going to `Skip`/`Delete` an assembly, so we can likewise skip it. - The linker writes all the assemblies out at the end, so we don't have a second "write" step. Changes to make this happen: - Removed the `<StripEmbeddedLibraries/>` MSBuild task and related targets - Removed `$(_AndroidStripFlag)` from our targets, since it is no longer used - Created a new `StripEmbeddedLibraries` linker step that runs late during linking - Removed a `RemoveLibraryResourceZip` linker step, as it seemed to be duplicative. I timed before and after with the Xamarin.Forms test project: .\bin\Debug\bin\xabuild .\tests\Xamarin.Forms-Performance-Integration\Droid\Xamarin.Forms.Performance.Integration.Droid.csproj /p:Configuration=Release /t:Clean .\bin\Debug\bin\xabuild .\tests\Xamarin.Forms-Performance-Integration\Droid\Xamarin.Forms.Performance.Integration.Droid.csproj /p:Configuration=Release /t:Build /bl Before: 1233 ms StripEmbeddedLibraries 1 calls 14925 ms LinkAssemblies 1 calls After: 15437 ms LinkAssemblies 1 calls As expected, `<LinkAssemblies/>` will be slightly slower, but since `<StripEmbeddedLibraries/>` is not called at all, we have a net gain of around 700ms. Once this has been merged and working for `Release` builds, I plan to do some further research to find out if running the new `StripEmbeddedLibraries` linker step will help for `Debug` builds. It could be a net performance improvement, if the time taken to remove these files improves deployment and app startup times. Greatly expanded upon an existing test: - Made it a Xamarin.Forms project - Moved the `@(AndroidEnvironment)` item to a referenced library project - Added an `$(AndroidLinkSkip)` option for a support library assembly - Made sure the `$(AndroidLinkSkip)` assembly is saved and stripped - Check and make sure `@(EmbeddedResource)` items are stripped at the end Other changes: - Removed some assertions in tests looking for `_StripEmbeddedLibraries`, since it is removed now.
diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs
@@ -103,7 +103,6 @@ static Pipeline CreatePipeline (LinkerOptions options)
 			pipeline.AppendStep (new PreserveCrypto ());
 			pipeline.AppendStep (new PreserveCode ());
 
-			pipeline.AppendStep (new RemoveLibraryResourceZip ());
 			pipeline.AppendStep (new RemoveResources (options.I18nAssemblies)); // remove collation tables
 			// end monodroid specific
 
@@ -114,6 +113,7 @@ static Pipeline CreatePipeline (LinkerOptions options)
 			// monodroid tuner steps
 			if (!string.IsNullOrWhiteSpace (options.ProguardConfiguration))
 				pipeline.AppendStep (new GenerateProguardConfiguration (options.ProguardConfiguration));
+			pipeline.AppendStep (new StripEmbeddedLibraries ());
 			// end monodroid specific
 			pipeline.AppendStep (new RegenerateGuidStep ());
 			pipeline.AppendStep (new OutputStep ());
diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveLibraryResourceZip.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveLibraryResourceZip.cs
diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibraries.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibraries.cs
@@ -0,0 +1,58 @@
+﻿using Mono.Cecil;
+using Mono.Linker;
+using Mono.Linker.Steps;
+using System;
+using System.Linq;
+using Xamarin.Android.Tasks;
+
+namespace MonoDroid.Tuner
+{
+	public class StripEmbeddedLibraries : BaseStep
+	{
+		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		{
+			if (!Annotations.HasAction (assembly))
+				return;
+			var action = Annotations.GetAction (assembly);
+			if (action == AssemblyAction.Skip || action == AssemblyAction.Delete)
+				return;
+
+			var fileName = assembly.Name.Name + ".dll";
+			if (MonoAndroidHelper.IsFrameworkAssembly (fileName) && !MonoAndroidHelper.FrameworkEmbeddedJarLookupTargets.Contains (fileName))
+				return;
+
+			bool assembly_modified = false;
+			foreach (var mod in assembly.Modules) {
+				foreach (var r in mod.Resources.ToArray ()) {
+					if (ShouldStripResource (r)) {
+						Context.LogMessage ("    Stripped {0} from {1}", r.Name, fileName);
+						mod.Resources.Remove (r);
+						assembly_modified = true;
+					}
+				}
+			}
+			if (assembly_modified && action == AssemblyAction.Copy) {
+				Annotations.SetAction (assembly, AssemblyAction.Save);
+			}
+		}
+
+		bool ShouldStripResource (Resource r)
+		{
+			if (!(r is EmbeddedResource))
+				return false;
+			// embedded jars
+			if (r.Name.EndsWith (".jar", StringComparison.InvariantCultureIgnoreCase))
+				return true;
+			// embedded AndroidNativeLibrary archive
+			if (r.Name == "__AndroidNativeLibraries__.zip")
+				return true;
+			// embedded AndroidResourceLibrary archive
+			if (r.Name == "__AndroidLibraryProjects__.zip")
+				return true;
+			// embedded AndroidEnvironment item
+			if (r.Name.StartsWith ("__AndroidEnvironment__", StringComparison.Ordinal))
+				return true;
+			return false;
+		}
+	}
+}
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs
@@ -69,24 +69,6 @@ IEnumerable<AssemblyDefinition> GetRetainAssemblies (DirectoryAssemblyResolver r
 
 		public override bool Execute ()
 		{
-			Log.LogDebugMessage ("LinkAssemblies Task");
-			Log.LogDebugMessage ("  UseSharedRuntime: {0}", UseSharedRuntime);
-			Log.LogDebugMessage ("  MainAssembly: {0}", MainAssembly);
-			Log.LogDebugMessage ("  OutputDirectory: {0}", OutputDirectory);
-			Log.LogDebugMessage ("  OptionalDestinationDirectory: {0}", OptionalDestinationDirectory);
-			Log.LogDebugMessage ("  I18nAssemblies: {0}", I18nAssemblies);
-			Log.LogDebugMessage ("  LinkMode: {0}", LinkMode);
-			Log.LogDebugMessage ("  LinkSkip: {0}", LinkSkip);
-			Log.LogDebugTaskItems ("  LinkDescriptions:", LinkDescriptions);
-			Log.LogDebugTaskItems ("  ResolvedAssemblies:", ResolvedAssemblies);
-			Log.LogDebugMessage ("  EnableProguard: {0}", EnableProguard);
-			Log.LogDebugMessage ("  ProguardConfiguration: {0}", ProguardConfiguration);
-			Log.LogDebugMessage ("  DumpDependencies: {0}", DumpDependencies);
-			Log.LogDebugMessage ("  LinkOnlyNewerThan: {0}", LinkOnlyNewerThan);
-			Log.LogDebugMessage ("  HttpClientHandlerType: {0}", HttpClientHandlerType);
-			Log.LogDebugMessage ("  TlsProvider: {0}", TlsProvider);
-			Log.LogDebugMessage ("  PreserveJniMarshalMethods: {0}", PreserveJniMarshalMethods);
-
 			var rp = new ReaderParameters {
 				InMemory    = true,
 			};
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs b/src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs
@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using System.Xml.Linq;
 using Microsoft.Build.Framework;
+using Mono.Cecil;
 using NUnit.Framework;
 using Xamarin.ProjectTools;
 
@@ -849,9 +850,6 @@ public void BasicApplicationRepetitiveBuild ()
 				Assert.IsTrue (
 					b.Output.IsTargetSkipped ("_Sign"),
 					"the _Sign target should not run");
-				Assert.IsTrue (
-					b.Output.IsTargetSkipped ("_StripEmbeddedLibraries"),
-					"the _StripEmbeddedLibraries target should not run");
 				proj.AndroidResources.Last ().Timestamp = null;
 				Assert.IsTrue (b.Build (proj), "third build failed");
 				Assert.IsFalse (
@@ -885,9 +883,6 @@ public class Foo {
 				Assert.IsTrue (
 					b.Output.IsTargetSkipped ("_Sign"),
 					"the _Sign target should not run");
-				Assert.IsTrue (
-					b.Output.IsTargetSkipped ("_StripEmbeddedLibraries"),
-					"the _StripEmbeddedLibraries target should not run");
 				Assert.IsTrue (
 					b.Output.IsTargetSkipped ("_LinkAssembliesShrink"),
 					"the _LinkAssembliesShrink target should not run");
@@ -1581,19 +1576,48 @@ public void CheckSequencePointGeneration (bool isRelease, bool monoSymbolArchive
 		[Test]
 		public void BuildApplicationWithMonoEnvironment ([Values ("", "Normal", "Offline")] string sequencePointsMode)
 		{
-			var proj = new XamarinAndroidApplicationProject () {
+			var lib = new XamarinAndroidLibraryProject {
+				ProjectName = "Library1",
 				IsRelease = true,
 				OtherBuildItems = { new AndroidItem.AndroidEnvironment ("Mono.env") {
 						TextContent = () => "MONO_DEBUG=soft-breakpoints"
 					},
 				},
 			};
-			proj.SetProperty ("_AndroidSequencePointsMode", sequencePointsMode);
-			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
-				b.Verbosity = LoggerVerbosity.Diagnostic;
-				Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
-				var apk = Path.Combine (Root, b.ProjectDirectory,
-					proj.IntermediateOutputPath, "android", "bin", "UnnamedProject.UnnamedProject.apk");
+			var app = new XamarinAndroidApplicationProject () {
+				IsRelease = true,
+				AndroidLinkModeRelease = AndroidLinkMode.Full,
+				References = {
+					new BuildItem ("ProjectReference","..\\Library1\\Library1.csproj"),
+				},
+			};
+			app.MainActivity = app.DefaultMainActivity.Replace ("public class MainActivity : Activity", "public class MainActivity : Xamarin.Forms.Platform.Android.FormsAppCompatActivity");
+			app.Packages.Add (KnownPackages.XamarinForms_3_0_0_561731);
+			app.Packages.Add (KnownPackages.Android_Arch_Core_Common_26_1_0);
+			app.Packages.Add (KnownPackages.Android_Arch_Lifecycle_Common_26_1_0);
+			app.Packages.Add (KnownPackages.Android_Arch_Lifecycle_Runtime_26_1_0);
+			app.Packages.Add (KnownPackages.AndroidSupportV4_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportCompat_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportCoreUI_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportCoreUtils_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportDesign_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportFragment_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportMediaCompat_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportV7AppCompat_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportV7CardView_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportV7MediaRouter_27_0_2_1);
+			app.Packages.Add (KnownPackages.SupportV7RecyclerView_27_0_2_1);
+			//LinkSkip one assembly that contains __AndroidLibraryProjects__.zip
+			string linkSkip = KnownPackages.SupportV7AppCompat_27_0_2_1.Id;
+			app.SetProperty ("AndroidLinkSkip", linkSkip);
+			app.SetProperty ("_AndroidSequencePointsMode", sequencePointsMode);
+			using (var libb = CreateDllBuilder (Path.Combine ("temp", TestName, lib.ProjectName)))
+			using (var appb = CreateApkBuilder (Path.Combine ("temp", TestName, app.ProjectName))) {
+				Assert.IsTrue (libb.Build (lib), "Library build should have succeeded.");
+				Assert.IsTrue (appb.Build (app), "App should have succeeded.");
+				Assert.IsTrue (StringAssertEx.ContainsText (appb.LastBuildOutput, $"Save assembly: {linkSkip}"), $"{linkSkip} should be saved, and not linked!");
+				var apk = Path.Combine (Root, appb.ProjectDirectory,
+					app.IntermediateOutputPath, "android", "bin", "UnnamedProject.UnnamedProject.apk");
 				using (var zipFile = ZipHelper.OpenZip (apk)) {
 					var data = ZipHelper.ReadFileFromZip (zipFile, "environment");
 					Assert.IsNotNull (data, "environment should exist in the apk.");
@@ -1605,6 +1629,18 @@ public void BuildApplicationWithMonoEnvironment ([Values ("", "Normal", "Offline
 						string.IsNullOrEmpty (sequencePointsMode) ? true : x.Contains ("gen-compact-seq-points")),
 						"The values from Mono.env should have been merged into environment");
 				}
+				var assemblyDir = Path.Combine (Root, appb.ProjectDirectory, app.IntermediateOutputPath, "android", "assets");
+				var rp = new ReaderParameters { ReadSymbols = false };
+				foreach (var assemblyFile in Directory.EnumerateFiles (assemblyDir, "*.dll")) {
+					using (var assembly = AssemblyDefinition.ReadAssembly (assemblyFile)) {
+						foreach (var module in assembly.Modules) {
+							var resources = module.Resources.Select (r => r.Name).ToArray ();
+							Assert.IsFalse (StringAssertEx.ContainsText (resources, "__AndroidEnvironment__"), "AndroidEnvironment EmbeddedResource should be stripped!");
+							Assert.IsFalse (StringAssertEx.ContainsText (resources, "__AndroidLibraryProjects__.zip"), "__AndroidLibraryProjects__.zip should be stripped!");
+							Assert.IsFalse (StringAssertEx.ContainsText (resources, "__AndroidNativeLibraries__.zip"), "__AndroidNativeLibraries__.zip should be stripped!");
+						}
+					}
+				}
 			}
 		}
 
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj
@@ -61,6 +61,10 @@
       <Project>{3F1F2F50-AF1A-4A5A-BEDB-193372F068D7}</Project>
       <Name>Xamarin.Android.Build.Tasks</Name>
     </ProjectReference>
+    <ProjectReference Include="..\..\..\..\external\Java.Interop\src\Xamarin.Android.Cecil\Xamarin.Android.Cecil.csproj">
+      <Project>{15945D4B-FF56-4BCC-B598-2718D199DD08}</Project>
+      <Name>Xamarin.Android.Cecil</Name>
+    </ProjectReference>
     <ProjectReference Include="..\..\..\..\external\xamarin-android-tools\src\Xamarin.Android.Tools.AndroidSdk\Xamarin.Android.Tools.AndroidSdk.csproj">
       <Project>{e34bcfa0-caa4-412c-aa1c-75db8d67d157}</Project>
       <Name>Xamarin.Android.Tools.AndroidSdk</Name>
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj
@@ -107,6 +107,7 @@
     <Compile Include="Linker\MonoDroid.Tuner\LinkerOptions.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\MonoDroidMarkStep.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\PreserveHttpAndroidClientHandler.cs" />
+    <Compile Include="Linker\MonoDroid.Tuner\StripEmbeddedLibraries.cs" />
     <Compile Include="Tasks\Aapt.cs" />
     <Compile Include="Tasks\Aapt2.cs" />
     <Compile Include="Tasks\Aapt2Compile.cs" />
@@ -194,7 +195,6 @@
     <Compile Include="Tasks\CreateLibraryResourceArchive.cs" />
     <Compile Include="Tasks\ResolveLibraryProjectImports.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\PreserveExportedTypes.cs" />
-    <Compile Include="Linker\MonoDroid.Tuner\RemoveLibraryResourceZip.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\PreserveDynamicTypes.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\PreserveLinqExpressions.cs" />
     <Compile Include="Tasks\NdkUtils.cs" />
@@ -205,7 +205,6 @@
     <Compile Include="Utilities\ResourceDesignerImportGenerator.cs" />
     <Compile Include="Tasks\CreateManagedLibraryResourceArchive.cs" />
     <Compile Include="Tasks\CheckForRemovedItems.cs" />
-    <Compile Include="Tasks\StripEmbeddedLibraries.cs" />
     <Compile Include="Tasks\CheckProjectItems.cs" />
     <Compile Include="Tasks\CheckDuplicateJavaLibraries.cs" />
     <Compile Include="Mono.Android\PermissionAttribute.Partial.cs" />
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
