Skip to content

Commit

Permalink
[linker] move StripEmbeddedLibraries into the linker
Browse files Browse the repository at this point in the history
Fixes: dotnet#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.
  • Loading branch information
jonathanpeppers committed Sep 18, 2018
1 parent 95ca102 commit 679cec9
Show file tree
Hide file tree
Showing 9 changed files with 119 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 ());
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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;
}
}
}
18 changes: 0 additions & 18 deletions src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
93 changes: 0 additions & 93 deletions src/Xamarin.Android.Build.Tasks/Tasks/StripEmbeddedLibraries.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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.");
Expand All @@ -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!");
}
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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" />
Expand All @@ -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" />
Expand Down
Loading

0 comments on commit 679cec9

Please sign in to comment.