From 679cec9b09b3be0664eca87d1bd1bc22683cfbdb Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 7 Sep 2018 14:10:01 -0500 Subject: [PATCH] [linker] move StripEmbeddedLibraries into the linker Fixes: https://github.com/xamarin/xamarin-android/issues/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. --- .../Linker/MonoDroid.Tuner/Linker.cs | 2 +- .../RemoveLibraryResourceZip.cs | 26 ------ .../MonoDroid.Tuner/StripEmbeddedLibraries.cs | 58 ++++++++++++ .../Tasks/LinkAssemblies.cs | 18 ---- .../Tasks/StripEmbeddedLibraries.cs | 93 ------------------- .../Xamarin.Android.Build.Tests/BuildTest.cs | 62 ++++++++++--- .../Xamarin.Android.Build.Tests.csproj | 4 + .../Xamarin.Android.Build.Tasks.csproj | 3 +- .../Xamarin.Android.Common.targets | 22 ++--- 9 files changed, 119 insertions(+), 169 deletions(-) delete mode 100644 src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveLibraryResourceZip.cs create mode 100644 src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibraries.cs delete mode 100644 src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs index 8bef4f076a7..3cdebc6e57a 100644 --- 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 deleted file mode 100644 index 03cb6a2a303..00000000000 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/RemoveLibraryResourceZip.cs +++ /dev/null @@ -1,26 +0,0 @@ -using System; -using System.Collections; -using System.Collections.Generic; -using System.Linq; - -using Mono.Linker; -using Mono.Linker.Steps; - -using Mono.Tuner; -using Mobile.Tuner; - -using Mono.Cecil; - -namespace MonoDroid.Tuner { - - public class RemoveLibraryResourceZip : BaseStep { - protected override void ProcessAssembly (AssemblyDefinition assembly) - { - foreach (var mod in assembly.Modules) { - var lres = mod.Resources.FirstOrDefault (r => r.Name == "__AndroidLibraryProjects__.zip"); - if (lres != null) - mod.Resources.Remove (lres); - } - } - } -} diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibraries.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibraries.cs new file mode 100644 index 00000000000..3169077eb8a --- /dev/null +++ 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 index f1955538c6a..cd42b134435 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs @@ -69,24 +69,6 @@ IEnumerable 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 deleted file mode 100644 index b1e412b04ae..00000000000 --- a/src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs +++ /dev/null @@ -1,93 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.IO; -using System.Linq; -using System.Xml; -using System.Xml.Linq; -using Mono.Cecil; -using Microsoft.Build.Utilities; -using Microsoft.Build.Framework; -using System.Text.RegularExpressions; - -using Java.Interop.Tools.Cecil; - -namespace Xamarin.Android.Tasks -{ - public class StripEmbeddedLibraries : Task - { - [Required] - public ITaskItem[] Assemblies { get; set; } - - public StripEmbeddedLibraries () - { - } - - public override bool Execute () - { - Log.LogDebugMessage ("StripEmbeddedLibraries Task"); - Log.LogDebugTaskItems (" Assemblies: ", Assemblies); - - using (var res = new DirectoryAssemblyResolver (this.CreateTaskLogger (), true, new ReaderParameters { ReadWrite = true } )) { - return Execute (res); - } - } - - bool Execute (DirectoryAssemblyResolver res) - { - foreach (var assembly in Assemblies) - res.Load (Path.GetFullPath (assembly.ItemSpec)); - - foreach (var assemblyName in Assemblies) { - var suffix = assemblyName.ItemSpec.EndsWith (".dll") ? String.Empty : ".dll"; - string hintPath = assemblyName.GetMetadata ("HintPath").Replace (Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); - string fileName = assemblyName.ItemSpec + suffix; - if (!String.IsNullOrEmpty (hintPath) && !File.Exists (hintPath)) // ignore invalid HintPath - hintPath = null; - string assemblyPath = String.IsNullOrEmpty (hintPath) ? fileName : hintPath; - if (MonoAndroidHelper.IsFrameworkAssembly (fileName) && !MonoAndroidHelper.FrameworkEmbeddedJarLookupTargets.Contains (Path.GetFileName (fileName))) - continue; - - var assembly = res.GetAssembly (assemblyPath); - bool assembly_modified = false; - foreach (var mod in assembly.Modules) { - // embedded jars - var resjars = mod.Resources.Where (r => r.Name.EndsWith (".jar", StringComparison.InvariantCultureIgnoreCase)).Select (r => (EmbeddedResource) r); - foreach (var resjar in resjars.ToArray ()) { - Log.LogDebugMessage (" Stripped {0}", resjar.Name); - mod.Resources.Remove (resjar); - assembly_modified = true; - } - // embedded AndroidNativeLibrary archive - var nativezip = mod.Resources.FirstOrDefault (r => r.Name == "__AndroidNativeLibraries__.zip") as EmbeddedResource; - if (nativezip != null) { - Log.LogDebugMessage (" Stripped {0}", nativezip.Name); - mod.Resources.Remove (nativezip); - assembly_modified = true; - } - // embedded AndroidResourceLibrary archive - var reszip = mod.Resources.FirstOrDefault (r => r.Name == "__AndroidLibraryProjects__.zip") as EmbeddedResource; - if (reszip != null) { - Log.LogDebugMessage (" Stripped {0}", reszip.Name); - mod.Resources.Remove (reszip); - assembly_modified = true; - } - } - if (assembly_modified) { - Log.LogDebugMessage (" The stripped library is saved as {0}", assemblyPath); - - // Output assembly needs to regenerate symbol file even if no IL/metadata was touched - // because Cecil still rewrites all assembly types in Cecil order (type A, nested types of A, type B, etc) - // and not in the original order causing symbols if original order doesn't match Cecil order - var wp = new WriterParameters () { - WriteSymbols = assembly.MainModule.HasSymbols - }; - - assembly.Write (wp); - } - } - return true; - } - } -} - 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 index aaa387da0fc..af10a7ebd98 100644 --- 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; @@ -839,9 +840,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 ( @@ -875,9 +873,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"); @@ -1571,19 +1566,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."); @@ -1595,6 +1619,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 index 9a80fb4318f..bd518dee85f 100644 --- 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 @@ {3F1F2F50-AF1A-4A5A-BEDB-193372F068D7} Xamarin.Android.Build.Tasks + + {15945D4B-FF56-4BCC-B598-2718D199DD08} + Xamarin.Android.Cecil + {e34bcfa0-caa4-412c-aa1c-75db8d67d157} Xamarin.Android.Tools.AndroidSdk diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj index b679aa6529e..db16612819a 100644 --- 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 @@ + @@ -194,7 +195,6 @@ - @@ -205,7 +205,6 @@ - diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index a5de5858bf7..7a6be18cff9 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -89,7 +89,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. - @@ -486,17 +485,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. - - - - - -