Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] <GenerateJavaStubs/> improvements (#2956)
Browse files Browse the repository at this point in the history
I went through the `_GenerateJavaStubs` MSBuild target and the
`<GenerateJavaStubs/>` MSBuild task. There was an "easy" win to
improve incremental build performance. I also made a few other general
improvements to `<GenerateJavaStubs/>`.

~~ Inputs & Outputs ~~

`_GenerateJavaStubs` has `Inputs` of:

    Inputs="$(MSBuildAllProjects);@(_ResolvedAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(_AndroidResourceDest)"

Lets say a NetStandard assembly changes, such as a XAML change in a
Xamarin.Forms app. In this case `_GenerateJavaStubs` does not actually
need to run again.

We can use `@(_ResolvedUserMonoAndroidAssemblies)` as an input instead
of `@(_ResolvedAssemblies)`.

This is the biggest win--to skip this target completely.

~~ selectedWhitelistAssemblies ~~

`ManifestDocument` had the concept of a "whitelist" assembly, which
currently consisted of a single assembly:

    internal static readonly string [] FrameworkAttributeLookupTargets = {"Mono.Android.GoogleMaps.dll"};

`Mono.Android.GoogleMaps.dll` no longer exists, so I was able to
remove a bit of code that still used that dead codepath.

~~ Typemaps ~~

Just a couple minor improvements here:

* Fixed the usage of the `[Obsolete]` constructor for
  `TypeNameMapGenerator`. Using a `(TaskLevel level, string value)`
  callback instead.
* I reused the `MemoryStream` and got rid of the `UpdateWhenChanged`
  method.

~~ Tests ~~

I updated the `IncrementalBuildTest.ProduceReferenceAssembly` test
quite a bit:

* The library project using `$(ProduceReferenceAssembly)` is now a
  NetStandard library.
* I check all the targets that should be skipped from now on.

I made a few changes in `Xamarin.ProjectTools` to clean things up:

* The `DotNetStandard` class should just define
  `Language=XamarinAndroidProjectLanguage.CSharp` by default.
* Added a `ShouldRestorePackageReferences` property for deciding if we
  pass `/restore` or not.
* The `DotNetStandard` class always needs to use `/restore`, even if
  there are no `PackageReferences`.
* I moved a couple methods from `DotNetXamarinProject` to
  `XamarinProject` so the `DotNetStandard` has them.

~~ Results ~~

Testing the Xamarin.Forms project in this repo. A fresh build I wasn't
able to see a noticeable improvement.

*However*, an incremental build with XAML-change:

    Before:
      861 ms  _GenerateJavaStubs                         1 calls
    After:
      n/a

The target is skipped now:

    _GenerateJavaStubs:
    Skipping target "_GenerateJavaStubs" because all output files are up-to-date with respect to the input files.

Overall this seems to be ~800ms better for incremental builds with
XAML changes.
  • Loading branch information
jonathanpeppers authored and dellis1972 committed Apr 16, 2019
1 parent 5a9d1a6 commit 9b99cce
Show file tree
Hide file tree
Showing 11 changed files with 66 additions and 86 deletions.
34 changes: 13 additions & 21 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Xml.Linq;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using MonoDroid.Utils;
using Mono.Cecil;


Expand Down Expand Up @@ -104,14 +103,9 @@ void Run (DirectoryAssemblyResolver res)
res.SearchDirectories.Add (dir.ItemSpec);
}

var selectedWhitelistAssemblies = new List<string> ();

// Put every assembly we'll need in the resolver
foreach (var assembly in ResolvedAssemblies) {
var assemblyFullPath = Path.GetFullPath (assembly.ItemSpec);
res.Load (assemblyFullPath);
if (MonoAndroidHelper.FrameworkAttributeLookupTargets.Any (a => Path.GetFileName (assembly.ItemSpec) == a))
selectedWhitelistAssemblies.Add (assemblyFullPath);
res.Load (assembly.ItemSpec);
}

// However we only want to look for JLO types in user code
Expand Down Expand Up @@ -234,7 +228,7 @@ void Run (DirectoryAssemblyResolver res)
manifest.NeedsInternet = NeedsInternet;
manifest.InstantRunEnabled = InstantRunEnabled;

var additionalProviders = manifest.Merge (all_java_types, selectedWhitelistAssemblies, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments);
var additionalProviders = manifest.Merge (all_java_types, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments);

using (var stream = new MemoryStream ()) {
manifest.Save (stream);
Expand Down Expand Up @@ -287,19 +281,17 @@ void SaveResource (string resource, string filename, string destDir, Func<string

void WriteTypeMappings (List<TypeDefinition> types)
{
using (var gen = UseSharedRuntime
? new TypeNameMapGenerator (types, Log.LogDebugMessage)
: new TypeNameMapGenerator (ResolvedAssemblies.Select (p => p.ItemSpec), Log.LogDebugMessage)) {
UpdateWhenChanged (Path.Combine (OutputDirectory, "typemap.jm"), gen.WriteJavaToManaged);
UpdateWhenChanged (Path.Combine (OutputDirectory, "typemap.mj"), gen.WriteManagedToJava);
}
}

void UpdateWhenChanged (string path, Action<Stream> generator)
{
void logger (TraceLevel level, string value) => Log.LogDebugMessage (value);
TypeNameMapGenerator createTypeMapGenerator () => UseSharedRuntime ?
new TypeNameMapGenerator (types, logger) :
new TypeNameMapGenerator (ResolvedAssemblies.Select (p => p.ItemSpec), logger);
using (var gen = createTypeMapGenerator ())
using (var stream = new MemoryStream ()) {
generator (stream);
MonoAndroidHelper.CopyIfStreamChanged (stream, path);
gen.WriteJavaToManaged (stream);
MonoAndroidHelper.CopyIfStreamChanged (stream, Path.Combine (OutputDirectory, "typemap.jm"));
stream.SetLength (0); //Reuse the stream
gen.WriteManagedToJava (stream);
MonoAndroidHelper.CopyIfStreamChanged (stream, Path.Combine (OutputDirectory, "typemap.mj"));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,10 @@ public void ProduceReferenceAssembly ()
app.SetProperty ("AndroidUseSharedRuntime", false.ToString ());

int count = 0;
var lib = new XamarinAndroidLibraryProject {
var lib = new DotNetStandard {
ProjectName = "MyLibrary",
Sdk = "Microsoft.NET.Sdk",
TargetFramework = "netstandard2.0",
Sources = {
new BuildItem.Source ("Bar.cs") {
TextContent = () => "public class Bar { public Bar () { System.Console.WriteLine (" + count++ + "); } }"
Expand All @@ -503,8 +505,10 @@ public void ProduceReferenceAssembly ()
Assert.IsTrue (appBuilder.Build (app, doNotCleanupOnUpdate: true, saveProject: false), "second app build should have succeeded.");

var targetsShouldSkip = new [] {
//TODO: perhaps more targets will skip here eventually?
"CoreCompile",
"_BuildLibraryImportsCache",
"_ResolveLibraryProjectImports",
"_GenerateJavaStubs",
};
foreach (var target in targetsShouldSkip) {
Assert.IsTrue (appBuilder.Output.IsTargetSkipped (target), $"`{target}` should be skipped!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ public void CheckAppBundle ([Values (true, false)] bool isRelease)
public void NetStandardReferenceTest ()
{
var netStandardProject = new DotNetStandard () {
Language = XamarinAndroidProjectLanguage.CSharp,
ProjectName = "XamFormsSample",
ProjectGuid = Guid.NewGuid ().ToString (),
Sdk = "Microsoft.NET.Sdk",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace Xamarin.ProjectTools
{
public class DotNetStandard : XamarinProject
{

public override string ProjectTypeGuid {
get {
return string.Empty;
Expand All @@ -20,7 +19,12 @@ public DotNetStandard ()
OtherBuildItems = new List<BuildItem> ();
SetProperty (CommonProperties, "DebugType", "full");
ItemGroupList.Add (Sources);
Language = XamarinAndroidProjectLanguage.CSharp;
}

// NetStandard projects always need to restore
public override bool ShouldRestorePackageReferences => true;

public string PackageTargetFallback {
get { return GetProperty ("PackageTargetFallback"); }
set { SetProperty ("PackageTargetFallback", value); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@ public string IntermediateOutputPath {
set { SetProperty (ActiveConfigurationProperties, KnownProperties.IntermediateOutputPath, value); }
}

public BuildItem GetItem (string include)
{
return ItemGroupList.SelectMany (g => g).First (i => i.Include ().Equals (include, StringComparison.OrdinalIgnoreCase));
}

public void AddReferences (params string [] references)
{
foreach (var s in references)
Expand All @@ -89,12 +84,6 @@ public void AddSources (params string [] sources)
Sources.Add (new BuildItem.Source (s));
}

public void Touch (params string [] itemPaths)
{
foreach (var item in itemPaths)
GetItem (item).Timestamp = DateTimeOffset.UtcNow;
}

public virtual ProjectRootElement Construct ()
{
var root = ProjectRootElement.Create ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public bool Build (XamarinProject project, bool doNotCleanupOnUpdate = false, st
project.NuGetRestore (Path.Combine (XABuildPaths.TestOutputDirectory, ProjectDirectory), PackagesDirectory);
}

bool result = BuildInternal (Path.Combine (ProjectDirectory, project.ProjectFilePath), Target, parameters, environmentVariables, restore: project.PackageReferences?.Count > 0);
bool result = BuildInternal (Path.Combine (ProjectDirectory, project.ProjectFilePath), Target, parameters, environmentVariables, restore: project.ShouldRestorePackageReferences);
built_before = true;

if (CleanupAfterSuccessfulBuild)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void Save ()

public bool BuildProject(XamarinProject project, string target = "Build")
{
BuildSucceeded = BuildInternal(Path.Combine (SolutionPath, project.ProjectName, project.ProjectFilePath), target, restore: project.PackageReferences?.Count > 0);
BuildSucceeded = BuildInternal(Path.Combine (SolutionPath, project.ProjectName, project.ProjectFilePath), target, restore: project.ShouldRestorePackageReferences);
return BuildSucceeded;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public abstract class XamarinProject
public IList<Package> Packages { get; private set; }
public IList<BuildItem> References { get; private set; }
public IList<Package> PackageReferences { get; private set; }
public virtual bool ShouldRestorePackageReferences => PackageReferences?.Count > 0;
public IList<Import> Imports { get; private set; }
PropertyGroup common, debug, release;

Expand Down Expand Up @@ -120,6 +121,17 @@ public void SetProperty (IList<Property> group, string name, Func<string> value,
}
}

public BuildItem GetItem (string include)
{
return ItemGroupList.SelectMany (g => g).First (i => i.Include ().Equals (include, StringComparison.OrdinalIgnoreCase));
}

public void Touch (params string [] itemPaths)
{
foreach (var item in itemPaths)
GetItem (item).Timestamp = DateTimeOffset.UtcNow;
}

string project_file_path;
public string ProjectFilePath {
get { return project_file_path ?? ProjectName + Language.DefaultProjectExtension; }
Expand Down
72 changes: 27 additions & 45 deletions src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ void ReorderActivityAliases (XElement app)
}
}

public IList<string> Merge (List<TypeDefinition> subclasses, List<string> selectedWhitelistAssemblies, string applicationClass, bool embed, string bundledWearApplicationName, IEnumerable<string> mergedManifestDocuments)
public IList<string> Merge (List<TypeDefinition> subclasses, string applicationClass, bool embed, string bundledWearApplicationName, IEnumerable<string> mergedManifestDocuments)
{
string applicationName = ApplicationName;

Expand All @@ -244,7 +244,7 @@ public IList<string> Merge (List<TypeDefinition> subclasses, List<string> select
if (manifest.Attribute (androidNs + "versionName") == null)
manifest.SetAttributeValue (androidNs + "versionName", "1.0");

app = CreateApplicationElement (manifest, applicationClass, subclasses, selectedWhitelistAssemblies);
app = CreateApplicationElement (manifest, applicationClass, subclasses);

if (app.Attribute (androidNs + "label") == null && applicationName != null)
app.SetAttributeValue (androidNs + "label", applicationName);
Expand Down Expand Up @@ -399,12 +399,12 @@ public IList<string> Merge (List<TypeDefinition> subclasses, List<string> select
}

AddInstrumentations (manifest, subclasses, targetSdkVersionValue);
AddPermissions (app, selectedWhitelistAssemblies);
AddPermissionGroups (app, selectedWhitelistAssemblies);
AddPermissionTrees (app, selectedWhitelistAssemblies);
AddUsesPermissions (app, selectedWhitelistAssemblies);
AddUsesFeatures (app, selectedWhitelistAssemblies);
AddSupportsGLTextures (app, selectedWhitelistAssemblies);
AddPermissions (app);
AddPermissionGroups (app);
AddPermissionTrees (app);
AddUsesPermissions (app);
AddUsesFeatures (app);
AddSupportsGLTextures (app);

ReorderActivityAliases (app);
ReorderElements (app);
Expand Down Expand Up @@ -508,7 +508,7 @@ Func<TypeDefinition, string, int, XElement> GetGenerator (TypeDefinition type)
return null;
}

XElement CreateApplicationElement (XElement manifest, string applicationClass, List<TypeDefinition> subclasses, List<string> selectedWhitelistAssemblies)
XElement CreateApplicationElement (XElement manifest, string applicationClass, List<TypeDefinition> subclasses)
{
var application = manifest.Descendants ("application").FirstOrDefault ();

Expand All @@ -521,10 +521,10 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L
.Where (attr => attr != null)
.ToList ();
var usesLibraryAttr =
Assemblies.Concat (selectedWhitelistAssemblies).SelectMany (path => UsesLibraryAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)))
Assemblies.SelectMany (path => UsesLibraryAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)))
.Where (attr => attr != null);
var usesConfigurationAttr =
Assemblies.Concat (selectedWhitelistAssemblies).SelectMany (path => UsesConfigurationAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)))
Assemblies.SelectMany (path => UsesConfigurationAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)))
.Where (attr => attr != null);
if (assemblyAttr.Count > 1)
throw new InvalidOperationException ("There can be only one [assembly:Application] attribute defined.");
Expand Down Expand Up @@ -772,54 +772,42 @@ public void AddFastDeployPermissions ()
app.AddBeforeSelf (new XElement ("uses-permission", new XAttribute (attName, permReadExternalStorage)));
}

void AddPermissions (XElement application, List<string> selectedWhitelistAssemblies)
void AddPermissions (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => PermissionAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
Assemblies.SelectMany (path => PermissionAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
// Add unique permissions to the manifest
foreach (var pa in assemblyAttrs.Distinct (new PermissionAttribute.PermissionAttributeComparer ()))
if (!application.Parent.Descendants ("permission").Any (x => (string)x.Attribute (attName) == pa.Name))
application.AddBeforeSelf (pa.ToElement (PackageName));
}

void AddPermissionGroups (XElement application, List<string> selectedWhitelistAssemblies)
void AddPermissionGroups (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => PermissionGroupAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
Assemblies.SelectMany (path => PermissionGroupAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));

// Add unique permissionGroups to the manifest
foreach (var pga in assemblyAttrs.Distinct (new PermissionGroupAttribute.PermissionGroupAttributeComparer ()))
if (!application.Parent.Descendants ("permissionGroup").Any (x => (string)x.Attribute (attName) == pga.Name))
application.AddBeforeSelf (pga.ToElement (PackageName));
}

void AddPermissionTrees (XElement application, List<string> selectedWhitelistAssemblies)
void AddPermissionTrees (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => PermissionTreeAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
var assemblyAttrs =
Assemblies.SelectMany (path => PermissionTreeAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));

// Add unique permissionGroups to the manifest
foreach (var pta in assemblyAttrs.Distinct (new PermissionTreeAttribute.PermissionTreeAttributeComparer ()))
if (!application.Parent.Descendants ("permissionTree").Any (x => (string)x.Attribute (attName) == pta.Name))
application.AddBeforeSelf (pta.ToElement (PackageName));
}

void AddUsesPermissions (XElement application, List<string> selectedWhitelistAssemblies)
void AddUsesPermissions (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => UsesPermissionAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
var assemblyAttrs =
Assemblies.SelectMany (path => UsesPermissionAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));

// Add unique permissions to the manifest
foreach (var upa in assemblyAttrs.Distinct (new UsesPermissionAttribute.UsesPermissionComparer ()))
Expand All @@ -841,13 +829,10 @@ void AddUsesLibraries (XElement application, IEnumerable<UsesLibraryAttribute> l
application.Add (ula.ToElement (PackageName));
}

void AddUsesFeatures (XElement application, List<string> selectedWhitelistAssemblies)
void AddUsesFeatures (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => UsesFeatureAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
var assemblyAttrs =
Assemblies.SelectMany (path => UsesFeatureAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));

// Add unique features by Name or glESVersion to the manifest
foreach (var feature in assemblyAttrs) {
Expand All @@ -865,13 +850,10 @@ void AddUsesFeatures (XElement application, List<string> selectedWhitelistAssemb
}
}

void AddSupportsGLTextures (XElement application, List<string> selectedWhitelistAssemblies)
void AddSupportsGLTextures (XElement application)
{
// Look in user assemblies + whitelist (like Maps)
var check_assemblies = Assemblies.Union (selectedWhitelistAssemblies);

var assemblyAttrs =
check_assemblies.SelectMany (path => SupportsGLTextureAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));
var assemblyAttrs =
Assemblies.SelectMany (path => SupportsGLTextureAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path)));

// Add unique items by Name to the manifest
foreach (var feature in assemblyAttrs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,6 @@ internal static IEnumerable<string> GetFrameworkAssembliesToTreatAsUserAssemblie
}
#endif

internal static readonly string [] FrameworkAttributeLookupTargets = {"Mono.Android.GoogleMaps.dll"};
internal static readonly string [] FrameworkEmbeddedJarLookupTargets = {
"Mono.Android.Support.v13.dll",
"Mono.Android.Support.v4.dll",
Expand All @@ -507,7 +506,6 @@ internal static IEnumerable<string> GetFrameworkAssembliesToTreatAsUserAssemblie
};
// MUST BE SORTED CASE-INSENSITIVE
internal static readonly string[] FrameworkAssembliesToTreatAsUserAssemblies = {
"Mono.Android.GoogleMaps.dll",
"Mono.Android.Support.v13.dll",
"Mono.Android.Support.v4.dll",
"Xamarin.Android.NUnitLite.dll",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2245,7 +2245,7 @@ because xbuild doesn't support framework reference assemblies.

<Target Name="_GenerateJavaStubs"
DependsOnTargets="_SetLatestTargetFrameworkVersion;_PrepareAssemblies;$(_AfterPrepareAssemblies)"
Inputs="$(MSBuildAllProjects);@(_ResolvedAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(_AndroidResourceDest)"
Inputs="$(MSBuildAllProjects);@(_ResolvedUserMonoAndroidAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(_AndroidResourceDest)"
Outputs="$(_AndroidStampDirectory)_GenerateJavaStubs.stamp">
<GenerateJavaStubs
ResolvedAssemblies="@(_ResolvedAssemblies)"
Expand Down

0 comments on commit 9b99cce

Please sign in to comment.