Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] remove TFI == MonoAndroid checks
Browse files Browse the repository at this point in the history 
In preparation for .NET 5, we think the `$(TargetFrameworkMoniker)`
*might* be:

    .NETCoreApp,Version=5.0,Profile=Xamarin.Android

It might also be `Profile=Android`, we don't know for sure yet.

The TFM currently is:

    MonoAndroid,Version=v10.0

I think we can (and should) remove any code during the build that
looks at `$(TargetFrameworkIdentifier)`.

The places we are currently doing this are for performance:

* The `_ResolveLibraryProjectImports` or `_GenerateJavaStubs` targets
  do not need to run against .NET standard assemblies.

We can rely on a reference to `Mono.Android.dll` instead. We are
already using System.Reflection.Metadata to see if the assembly
actually has this reference.

So an example of the condition:

    Condition="'%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'"

Can just be:

    Condition="'%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'"

I also took the opportunity for two small perf improvements:

* `<FilterAssemblies/>` only needs to look for obsolete attributes in
  assemblies that reference `Mono.Android.dll`.

* `<ResolveAssemblies/>` was collection a dictionary of API levels and
  then iterating over it later to emit warnings. This now just emits
  warnings as assemblies are processed.

~~ Results ~~

Testing a build with no changes for the SmartHotel360 app:

    Before:
     60 ms  FilterAssemblies                           2 calls
    318 ms  ResolveAssemblies                          1 calls
    After:
     49 ms  FilterAssemblies                           2 calls
    317 ms  ResolveAssemblies                          1 calls

I would think there is a small ~12ms performance improvement to all
builds of this app.
  • Loading branch information
@jonathanpeppers
jonathanpeppers committed Feb 5, 2020
1 parent 476921a commit 2f06e64
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 66 deletions.
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Copyright (C) 2016 Xamarin. All rights reserved.
/>
<_ResolvedUserMonoAndroidAssembliesForDesigner
Include="@(ResolvedUserAssemblies)"
Condition="'%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'"
Condition="'%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'"
/>
</ItemGroup>
</Target>
Expand Down
34 changes: 4 additions & 30 deletions src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ public class FilterAssemblies : AndroidTask
{
public override string TaskPrefix => "FLT";

const string TargetFrameworkIdentifier = "MonoAndroid";
const string MonoAndroidReference = "Mono.Android";

public bool DesignTimeBuild { get; set; }
Expand All @@ -42,19 +41,9 @@ public override bool RunTask ()
}
using (var pe = new PEReader (File.OpenRead (assemblyItem.ItemSpec))) {
var reader = pe.GetMetadataReader ();
var assemblyDefinition = reader.GetAssemblyDefinition ();
var targetFrameworkIdentifier = GetTargetFrameworkIdentifier (assemblyDefinition, reader);
if (string.Compare (targetFrameworkIdentifier, TargetFrameworkIdentifier, StringComparison.OrdinalIgnoreCase) == 0) {
output.Add (assemblyItem);
continue;
}

// In the rare case, [assembly: TargetFramework("MonoAndroid,Version=v9.0")] may not match
Log.LogDebugMessage ($"{nameof (TargetFrameworkIdentifier)} did not match: {assemblyItem.ItemSpec}");

// Fallback to looking for a Mono.Android reference
// By default look for a reference to Mono.Android.dll
if (HasReference (reader)) {
Log.LogDebugMessage ($"{MonoAndroidReference} reference found: {assemblyItem.ItemSpec}");
CheckAssemblyAttributes (reader);
output.Add (assemblyItem);
continue;
}
Expand All @@ -71,27 +60,13 @@ public override bool RunTask ()
return !Log.HasLoggedErrors;
}

string GetTargetFrameworkIdentifier (AssemblyDefinition assembly, MetadataReader reader)
void CheckAssemblyAttributes (MetadataReader reader)
{
string targetFrameworkIdentifier = null;
var assembly = reader.GetAssemblyDefinition ();
foreach (var handle in assembly.GetCustomAttributes ()) {
var attribute = reader.GetCustomAttribute (handle);
var name = reader.GetCustomAttributeFullName (attribute);
switch (name) {
case "System.Runtime.Versioning.TargetFrameworkAttribute":
var arguments = attribute.GetCustomAttributeArguments ();
foreach (var p in arguments.FixedArguments) {
// Of the form "MonoAndroid,Version=v8.1"
var value = p.Value?.ToString ();
if (!string.IsNullOrEmpty (value)) {
int commaIndex = value.IndexOf (",", StringComparison.Ordinal);
if (commaIndex != -1) {
targetFrameworkIdentifier = value.Substring (0, commaIndex);
break;
}
}
}
break;
case "Android.IncludeAndroidResourcesFromAttribute":
case "Android.NativeLibraryReferenceAttribute":
case "Java.Interop.JavaLibraryReferenceAttribute":
Expand All @@ -102,7 +77,6 @@ string GetTargetFrameworkIdentifier (AssemblyDefinition assembly, MetadataReader
break;
}
}
return targetFrameworkIdentifier;
}

bool HasReference (MetadataReader reader)
Expand Down
45 changes: 15 additions & 30 deletions src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ void Execute (MetadataResolver resolver)
lockFile = LockFileUtilities.GetLockFile (ProjectAssetFile, logger);
}

mainApiLevel = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);

try {
foreach (var assembly in Assemblies) {
// Add each user assembly and all referenced assemblies (recursive)
Expand Down Expand Up @@ -123,14 +125,6 @@ void Execute (MetadataResolver resolver)
// Add I18N assemblies if needed
AddI18nAssemblies (resolver, assemblies);

var mainapiLevel = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);
foreach (var item in api_levels.Where (x => mainapiLevel < x.Value)) {
var itemOSVersion = MonoAndroidHelper.SupportedVersions.GetFrameworkVersionFromApiLevel (item.Value);
LogCodedWarning ("XA0105", ProjectFile, 0,
"The $(TargetFrameworkVersion) for {0} ({1}) is greater than the $(TargetFrameworkVersion) for your project ({2}). " +
"You need to increase the $(TargetFrameworkVersion) for your project.", Path.GetFileName (item.Key), itemOSVersion, TargetFrameworkVersion);
}

var resolvedAssemblies = new List<ITaskItem> (assemblies.Count);
var resolvedSymbols = new List<ITaskItem> (assemblies.Count);
var resolvedFrameworkAssemblies = new List<ITaskItem> (assemblies.Count);
Expand All @@ -157,7 +151,7 @@ void Execute (MetadataResolver resolver)
}

readonly List<string> do_not_package_atts = new List<string> ();
readonly Dictionary<string, int> api_levels = new Dictionary<string, int> ();
int? mainApiLevel;
int indent = 2;

string ResolveRuntimeAssemblyForReferenceAssembly (LockFile lockFile, string assemblyPath)
Expand Down Expand Up @@ -214,23 +208,16 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary<string, ITaskI
if (resolutionPath == null)
resolutionPath = new List<string>();

CheckAssemblyAttributes (assembly, reader, out string targetFrameworkIdentifier);
CheckAssemblyAttributes (assembly, reader);

LogMessage ("{0}Adding assembly reference for {1}, recursively...", new string (' ', indent), assemblyName);
resolutionPath.Add (assemblyName);
indent += 2;

// Add this assembly
ITaskItem assemblyItem = null;
if (topLevel) {
if (assemblies.TryGetValue (assemblyName, out assemblyItem)) {
if (!string.IsNullOrEmpty (targetFrameworkIdentifier) && string.IsNullOrEmpty (assemblyItem.GetMetadata ("TargetFrameworkIdentifier"))) {
assemblyItem.SetMetadata ("TargetFrameworkIdentifier", targetFrameworkIdentifier);
}
}
} else {
if (!assemblies.TryGetValue (assemblyName, out ITaskItem assemblyItem) && !topLevel) {
assemblies [assemblyName] =
assemblyItem = CreateAssemblyTaskItem (assemblyPath, targetFrameworkIdentifier);
assemblyItem = CreateAssemblyTaskItem (assemblyPath);
}

// Recurse into each referenced assembly
Expand Down Expand Up @@ -272,10 +259,8 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary<string, ITaskI
resolutionPath.RemoveAt (resolutionPath.Count - 1);
}

void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader, out string targetFrameworkIdentifier)
void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader)
{
targetFrameworkIdentifier = null;

foreach (var handle in assembly.GetCustomAttributes ()) {
var attribute = reader.GetCustomAttribute (handle);
switch (reader.GetCustomAttributeFullName (attribute)) {
Expand All @@ -293,22 +278,24 @@ void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader
var arguments = attribute.GetCustomAttributeArguments ();
foreach (var p in arguments.FixedArguments) {
// Of the form "MonoAndroid,Version=v8.1"
// If this is a .NET 5 TFM, we won't emit these warnings
var value = p.Value?.ToString ();
if (!string.IsNullOrEmpty (value)) {
int commaIndex = value.IndexOf (",", StringComparison.Ordinal);
if (commaIndex != -1) {
targetFrameworkIdentifier = value.Substring (0, commaIndex);
string targetFrameworkIdentifier = value.Substring (0, commaIndex);
if (targetFrameworkIdentifier == "MonoAndroid") {
const string match = "Version=";
var versionIndex = value.IndexOf (match, commaIndex, StringComparison.Ordinal);
if (versionIndex != -1) {
versionIndex += match.Length;
string version = value.Substring (versionIndex, value.Length - versionIndex);
var apiLevel = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (version);
if (apiLevel != null) {
if (apiLevel > mainApiLevel) {
var assemblyName = reader.GetString (assembly.Name);
LogDebugMessage ("{0}={1}", assemblyName, apiLevel);
api_levels [assemblyName] = apiLevel.Value;
LogCodedWarning ("XA0105", ProjectFile, 0,
"The $(TargetFrameworkVersion) for {0} ({1}) is greater than the $(TargetFrameworkVersion) for your project ({2}). " +
"You need to increase the $(TargetFrameworkVersion) for your project.", assemblyName, version, TargetFrameworkVersion);
}
}
}
Expand Down Expand Up @@ -368,14 +355,12 @@ void ResolveI18nAssembly (MetadataResolver resolver, string name, Dictionary<str
assemblies [name] = CreateAssemblyTaskItem (assembly);
}

static ITaskItem CreateAssemblyTaskItem (string assembly, string targetFrameworkIdentifier = null)
static ITaskItem CreateAssemblyTaskItem (string assembly)
{
var assemblyFullPath = Path.GetFullPath (assembly);
var dictionary = new Dictionary<string, string> (2) {
var dictionary = new Dictionary<string, string> (1) {
{ "ReferenceAssembly", assemblyFullPath },
};
if (!string.IsNullOrEmpty (targetFrameworkIdentifier))
dictionary.Add ("TargetFrameworkIdentifier", targetFrameworkIdentifier);
return new TaskItem (assemblyFullPath, dictionary);
}
}
Expand Down
4 changes: 0 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,6 @@ public static string GetNativeLibraryAbi (ITaskItem lib)

public static bool IsMonoAndroidAssembly (ITaskItem assembly)
{
var tfi = assembly.GetMetadata ("TargetFrameworkIdentifier");
if (string.Compare (tfi, "MonoAndroid", StringComparison.OrdinalIgnoreCase) == 0)
return true;

var hasReference = assembly.GetMetadata ("HasMonoAndroidReference");
return bool.TryParse (hasReference, out bool value) && value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ because xbuild doesn't support framework reference assemblies.
<ItemGroup>
<_ResolvedUserMonoAndroidAssemblies
Include="@(_ResolvedUserAssemblies)"
Condition=" '%(_ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' "
Condition=" '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' "
/>
</ItemGroup>
</Target>
Expand Down

0 comments on commit 2f06e64

Please sign in to comment.