Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] lazily populate Resource lookup (#7686)
Browse files Browse the repository at this point in the history
Fixes: #7684

Comparing .NET 7 to main, I noticed:

    .NET 7
    Task LinkAssembliesNoShrink 4ms
    main/.NET 8
    Task LinkAssembliesNoShrink 101ms

Under `dotnet trace` a lot of the time was spent in:

    94.74ms MonoDroid.Tuner.FixLegacyResourceDesignerStep.LoadDesigner()

Reviewing the code, I think we can "lazily" call the `LoadDesigner()`
method. Now it delays until the `ProcessAssemblyDesigner()` method. I
had to reorder logic slightly to do this.

I also updated one log message to only log duplicates, as it was
logging hundreds of lines:

    if (output.ContainsKey (key)) {
        LogMessage ($"          Found duplicate {key}");
    } else {
        output.Add (key, property.GetMethod);
    }

Which also showed up in `dotnet trace`:

    25.58ms Microsoft.Android.Build.Tasks.MSBuildExtensions.LogDebugMessage()

With these changes, I instead get:

    Task LinkAssembliesNoShrink 5ms

Which is probably ~the same performance as before or plenty good enough!

Lastly, I also removed some other code & members in
`FixLegacyResourceDesignerStep` that Visual Studio showed me was
unused.
  • Loading branch information
jonathanpeppers authored Jan 17, 2023
1 parent 8c24b8f commit 899d781
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,8 @@ public class FixLegacyResourceDesignerStep : LinkDesignerBase
{
internal const string DesignerAssemblyName = "_Microsoft.Android.Resource.Designer";
internal const string DesignerAssemblyNamespace = "Microsoft.Android.Resource.Designer";
#if ILLINK
protected override void Process ()
{
cache = Context;
}
#else // ILLINK
public FixLegacyResourceDesignerStep (IMetadataResolver cache)
{
this.cache = cache;
}

readonly
#endif // ILLINK
IMetadataResolver cache;
bool designerLoaded = false;
AssemblyDefinition designerAssembly = null;
TypeDefinition designerType = null;
Dictionary<string, MethodDefinition> lookup;
Expand All @@ -49,35 +37,34 @@ protected override void EndProcess ()

protected override void LoadDesigner ()
{
if (designerAssembly != null)
if (designerLoaded)
return;
var designerNameAssembly = AssemblyNameReference.Parse ($"{DesignerAssemblyName}, Version=1.0.0.0");
try {
designerAssembly = Resolve (designerNameAssembly);
} catch (Mono.Cecil.AssemblyResolutionException) {
LogMessage ($" Could not resolve assembly {DesignerAssemblyName}.");
} catch (System.IO.FileNotFoundException) {
LogMessage ($" Assembly {DesignerAssemblyName} did not exist.");
}
if (designerAssembly == null) {
return;
}
designerType = designerAssembly.MainModule.GetTypes ().FirstOrDefault (x => x.FullName == $"{DesignerAssemblyNamespace}.Resource");
if (designerType == null) {
LogMessage ($" Did not find {DesignerAssemblyNamespace}.Resource type. It was probably linked out.");
return;
var designerNameAssembly = AssemblyNameReference.Parse ($"{DesignerAssemblyName}, Version=1.0.0.0");
try {
designerAssembly = Resolve (designerNameAssembly);
LogMessage ($" Loaded {designerNameAssembly}");
} catch (Mono.Cecil.AssemblyResolutionException) {
LogMessage ($" Could not resolve assembly {DesignerAssemblyName}.");
} catch (System.IO.FileNotFoundException) {
LogMessage ($" Assembly {DesignerAssemblyName} did not exist.");
}
if (designerAssembly == null) {
return;
}
designerType = designerAssembly.MainModule.GetTypes ().FirstOrDefault (x => x.FullName == $"{DesignerAssemblyNamespace}.Resource");
if (designerType == null) {
LogMessage ($" Did not find {DesignerAssemblyNamespace}.Resource type. It was probably linked out.");
return;
}
lookup = BuildResourceDesignerPropertyLookup (designerType);
} finally {
designerLoaded = true;
}
lookup = BuildResourceDesignerPropertyLookup (designerType);
return;
}

internal override bool ProcessAssemblyDesigner (AssemblyDefinition assembly)
{
if (designerAssembly == null || designerType == null) {
LogMessage ($" Not using {DesignerAssemblyName}");
return false;
}

if (!FindResourceDesigner (assembly, mainApplication: false, out TypeDefinition designer, out CustomAttribute designerAttribute)) {
LogMessage ($" {assembly.Name.Name} has no designer. ");
return false;
Expand All @@ -90,6 +77,15 @@ internal override bool ProcessAssemblyDesigner (AssemblyDefinition assembly)
return false;
}

// This is expected for the first call, in <LinkAssembliesNoShrink/>
if (!designerLoaded)
LoadDesigner ();

if (designerAssembly == null || designerType == null) {
LogMessage ($" Not using {DesignerAssemblyName}");
return false;
}

LogMessage ($" Adding reference {designerAssembly.Name.Name}.");
assembly.MainModule.AssemblyReferences.Add (designerAssembly.Name);
var importedDesignerType = assembly.MainModule.ImportReference (designerType.Resolve ());
Expand All @@ -114,9 +110,10 @@ Dictionary<string, MethodDefinition> BuildResourceDesignerPropertyLookup (TypeDe
foreach (PropertyDefinition property in definition.Properties)
{
string key = $"{definition.Name}::{property.Name}";
if (!output.ContainsKey (key)) {
LogMessage ($" Adding {key}");
output.Add(key, property.GetMethod);
if (output.ContainsKey (key)) {
LogMessage ($" Found duplicate {key}");
} else {
output.Add (key, property.GetMethod);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static Pipeline CreatePipeline (LinkerOptions options)
// end monodroid specific

if (options.UseDesignerAssembly)
pipeline.AppendStep (new FixLegacyResourceDesignerStep (cache));
pipeline.AppendStep (new FixLegacyResourceDesignerStep ());
pipeline.AppendStep (new FixAbstractMethodsStep (cache));
pipeline.AppendStep (new MonoDroidMarkStep (cache));
pipeline.AppendStep (new SweepStep ());
Expand Down
11 changes: 2 additions & 9 deletions src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ public override bool RunTask ()
var cache = new TypeDefinitionCache ();
var fixAbstractMethodsStep = new FixAbstractMethodsStep (resolver, cache, Log);
var addKeepAliveStep = new AddKeepAlivesStep (resolver, cache, Log, UsingAndroidNETSdk);
var fixLegacyResourceDesignerStep = new FixLegacyResourceDesignerStep (resolver, cache, Log);
if (UseDesignerAssembly)
fixLegacyResourceDesignerStep.Load ();
var fixLegacyResourceDesignerStep = new FixLegacyResourceDesignerStep (resolver, Log);
for (int i = 0; i < SourceFiles.Length; i++) {
var source = SourceFiles [i];
var destination = DestinationFiles [i];
Expand Down Expand Up @@ -133,17 +131,12 @@ class FixLegacyResourceDesignerStep : MonoDroid.Tuner.FixLegacyResourceDesignerS
readonly DirectoryAssemblyResolver resolver;
readonly TaskLoggingHelper logger;

public FixLegacyResourceDesignerStep (DirectoryAssemblyResolver resolver, TypeDefinitionCache cache, TaskLoggingHelper logger)
: base (cache)
public FixLegacyResourceDesignerStep (DirectoryAssemblyResolver resolver, TaskLoggingHelper logger)
{
this.resolver = resolver;
this.logger = logger;
}

public void Load () {
LoadDesigner ();
}

public override void LogMessage (string message)
{
logger.LogDebugMessage ("{0}", message);
Expand Down

0 comments on commit 899d781

Please sign in to comment.