Skip to content

Commit 38bc7d0

Browse files
jonathanpeppersjonpryor
jonathanpeppers
authored and
jonpryor
committed
[Xamarin.Android.Build.Tasks] simplify LINQ in ResolveAssemblies (#2168)
Looking at the `<ResolveAssemblies/>` task, I noticed a lot of LINQ usage that could be simplified. We were using `HashSet<string>` that contains a list of all assemblies, and the following code: if (!topLevel && assemblies.All (a => new AssemblyNameDefinition (a, null).Name != assembly.Name.Name)) assemblies.Add (fullPath); This was called from a *recursive* function and needlessly calls `new AssemblyNameDefinition().Name` for every other assembly... Roughly an O(N^2) operation. So I changed the `HashSet<string>` to be a `Dictionary<string, string>`, where the "key" is the `assembly.Name.Name` and the value the full path to the assembly. I also fixed up usage of `assembly.MainModule.FullyQualifiedName`, which was deprecated; `FileName` should be used instead. Lastly, I fixed up several LINQ expressions on the `[Output]`s of `<ResolveAssemblies/>`. I rewrote them to use a single `foreach` loop instead of the equivalent LINQ that would loop over the list multiple times. ~~ Results ~~ The build time improvements here were "not zero", but also not huge. Testing the `tests/Xamarin.Forms-Performance-Integration` project, a build with no changes... Before: 188 ms ResolveAssemblies 1 calls After: 181 ms ResolveAssemblies 1 calls I think this change is still worthwhile, since: - The code is a bit cleaner, less LINQ for others to profile/look at, and I plan to refactor this task more soon. - The time savings is part of the dev-loop, and even builds with no changes. - There should be overall less allocations. Other changes: - Removed extraneous log messages for inputs/outputs.
1 parent 9d77ff1 commit 38bc7d0

File tree

1 file changed

+40
-39
lines changed

1 file changed

+40
-39
lines changed

‎src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs‎

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,10 @@ public override bool Execute ()
6868

6969
void Execute (DirectoryAssemblyResolver resolver)
7070
{
71-
LogDebugMessage ("ResolveAssemblies Task");
72-
LogDebugMessage (" ReferenceAssembliesDirectory: {0}", ReferenceAssembliesDirectory);
73-
LogDebugMessage (" I18nAssemblies: {0}", I18nAssemblies);
74-
LogDebugMessage (" LinkMode: {0}", LinkMode);
75-
LogDebugTaskItems (" Assemblies:", Assemblies);
76-
LogDebugMessage (" ProjectAssetFile: {0}", ProjectAssetFile);
77-
LogDebugMessage (" TargetMoniker: {0}", TargetMoniker);
78-
7971
foreach (var dir in ReferenceAssembliesDirectory.Split (new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries))
8072
resolver.SearchDirectories.Add (dir);
8173

82-
var assemblies = new HashSet<string> ();
74+
var assemblies = new Dictionary<string, string> ();
8375

8476
var topAssemblyReferences = new List<AssemblyDefinition> ();
8577
var logger = new NuGetLogger((s) => {
@@ -113,7 +105,7 @@ void Execute (DirectoryAssemblyResolver resolver)
113105
}
114106
}
115107
topAssemblyReferences.Add (assemblyDef);
116-
assemblies.Add (Path.GetFullPath (assemblyDef.MainModule.FullyQualifiedName));
108+
assemblies [assemblyDef.Name.Name] = Path.GetFullPath (assemblyDef.MainModule.FileName);
117109
}
118110
} catch (Exception ex) {
119111
LogError ("Exception while loading assemblies: {0}", ex);
@@ -130,21 +122,30 @@ void Execute (DirectoryAssemblyResolver resolver)
130122
// Add I18N assemblies if needed
131123
AddI18nAssemblies (resolver, assemblies);
132124

133-
ResolvedAssemblies = assemblies.Select (a => new TaskItem (a)).ToArray ();
134-
ResolvedSymbols = assemblies.Select (a => a + ".mdb").Where (a => File.Exists (a)).Select (a => new TaskItem (a)).ToArray ();
135-
ResolvedSymbols = ResolvedSymbols.Concat (
136-
assemblies.Select (a => Path.ChangeExtension (a, "pdb"))
137-
.Where (a => File.Exists (a) && Files.IsPortablePdb (a))
138-
.Select (a => new TaskItem (a)))
139-
.ToArray ();
140-
ResolvedFrameworkAssemblies = ResolvedAssemblies.Where (p => MonoAndroidHelper.IsFrameworkAssembly (p.ItemSpec, true)).ToArray ();
141-
ResolvedUserAssemblies = ResolvedAssemblies.Where (p => !MonoAndroidHelper.IsFrameworkAssembly (p.ItemSpec, true)).ToArray ();
125+
var resolvedAssemblies = new List<ITaskItem> (assemblies.Count);
126+
var resolvedSymbols = new List<ITaskItem> (assemblies.Count);
127+
var resolvedFrameworkAssemblies = new List<ITaskItem> (assemblies.Count);
128+
var resolvedUserAssemblies = new List<ITaskItem> (assemblies.Count);
129+
foreach (var assembly in assemblies.Values) {
130+
var mdb = assembly + ".mdb";
131+
var pdb = Path.ChangeExtension (assembly, "pdb");
132+
if (File.Exists (mdb))
133+
resolvedSymbols.Add (new TaskItem (mdb));
134+
if (File.Exists (pdb) && Files.IsPortablePdb (pdb))
135+
resolvedSymbols.Add (new TaskItem (pdb));
136+
var assemblyItem = new TaskItem (assembly);
137+
resolvedAssemblies.Add (assemblyItem);
138+
if (MonoAndroidHelper.IsFrameworkAssembly (assembly, checkSdkPath: true)) {
139+
resolvedFrameworkAssemblies.Add (assemblyItem);
140+
} else {
141+
resolvedUserAssemblies.Add (assemblyItem);
142+
}
143+
}
144+
ResolvedAssemblies = resolvedAssemblies.ToArray ();
145+
ResolvedSymbols = resolvedSymbols.ToArray ();
146+
ResolvedFrameworkAssemblies = resolvedFrameworkAssemblies.ToArray ();
147+
ResolvedUserAssemblies = resolvedUserAssemblies.ToArray ();
142148
ResolvedDoNotPackageAttributes = do_not_package_atts.ToArray ();
143-
144-
LogDebugTaskItems (" [Output] ResolvedAssemblies:", ResolvedAssemblies);
145-
LogDebugTaskItems (" [Output] ResolvedUserAssemblies:", ResolvedUserAssemblies);
146-
LogDebugTaskItems (" [Output] ResolvedFrameworkAssemblies:", ResolvedFrameworkAssemblies);
147-
LogDebugTaskItems (" [Output] ResolvedDoNotPackageAttributes:", ResolvedDoNotPackageAttributes);
148149
}
149150

150151
readonly List<string> do_not_package_atts = new List<string> ();
@@ -185,14 +186,14 @@ AssemblyDefinition ResolveRuntimeAssemblyForReferenceAssembly (LockFile lockFile
185186
return null;
186187
}
187188

188-
void AddAssemblyReferences (DirectoryAssemblyResolver resolver, ICollection<string> assemblies, AssemblyDefinition assembly, List<string> resolutionPath)
189+
void AddAssemblyReferences (DirectoryAssemblyResolver resolver, Dictionary<string, string> assemblies, AssemblyDefinition assembly, List<string> resolutionPath)
189190
{
190-
var fqname = assembly.MainModule.FullyQualifiedName;
191-
var fullPath = Path.GetFullPath (fqname);
191+
var assemblyName = assembly.Name.Name;
192+
var fullPath = Path.GetFullPath (assembly.MainModule.FileName);
192193

193194
// Don't repeat assemblies we've already done
194195
bool topLevel = resolutionPath == null;
195-
if (!topLevel && assemblies.Contains (fullPath))
196+
if (!topLevel && assemblies.ContainsKey (assemblyName))
196197
return;
197198

198199
if (resolutionPath == null)
@@ -210,8 +211,8 @@ void AddAssemblyReferences (DirectoryAssemblyResolver resolver, ICollection<stri
210211
indent += 2;
211212

212213
// Add this assembly
213-
if (!topLevel && assemblies.All (a => new AssemblyNameDefinition (a, null).Name != assembly.Name.Name))
214-
assemblies.Add (fullPath);
214+
if (!topLevel)
215+
assemblies [assemblyName] = fullPath;
215216

216217
// Recurse into each referenced assembly
217218
foreach (AssemblyNameReference reference in assembly.MainModule.AssemblyReferences) {
@@ -256,7 +257,7 @@ static LinkModes ParseLinkMode (string linkmode)
256257
return mode;
257258
}
258259

259-
void AddI18nAssemblies (DirectoryAssemblyResolver resolver, ICollection<string> assemblies)
260+
void AddI18nAssemblies (DirectoryAssemblyResolver resolver, Dictionary<string, string> assemblies)
260261
{
261262
var i18n = Linker.ParseI18nAssemblies (I18nAssemblies);
262263
var link = ParseLinkMode (LinkMode);
@@ -265,28 +266,28 @@ void AddI18nAssemblies (DirectoryAssemblyResolver resolver, ICollection<string>
265266
if (i18n == Mono.Linker.I18nAssemblies.None)
266267
return;
267268

268-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N"));
269+
ResolveI18nAssembly (resolver, "I18N", assemblies);
269270

270271
if (i18n.HasFlag (Mono.Linker.I18nAssemblies.CJK))
271-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N.CJK"));
272+
ResolveI18nAssembly (resolver, "I18N.CJK", assemblies);
272273

273274
if (i18n.HasFlag (Mono.Linker.I18nAssemblies.MidEast))
274-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N.MidEast"));
275+
ResolveI18nAssembly (resolver, "I18N.MidEast", assemblies);
275276

276277
if (i18n.HasFlag (Mono.Linker.I18nAssemblies.Other))
277-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N.Other"));
278+
ResolveI18nAssembly (resolver, "I18N.Other", assemblies);
278279

279280
if (i18n.HasFlag (Mono.Linker.I18nAssemblies.Rare))
280-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N.Rare"));
281+
ResolveI18nAssembly (resolver, "I18N.Rare", assemblies);
281282

282283
if (i18n.HasFlag (Mono.Linker.I18nAssemblies.West))
283-
assemblies.Add (ResolveI18nAssembly (resolver, "I18N.West"));
284+
ResolveI18nAssembly (resolver, "I18N.West", assemblies);
284285
}
285286

286-
string ResolveI18nAssembly (DirectoryAssemblyResolver resolver, string name)
287+
void ResolveI18nAssembly (DirectoryAssemblyResolver resolver, string name, Dictionary<string, string> assemblies)
287288
{
288289
var assembly = resolver.Resolve (AssemblyNameReference.Parse (name));
289-
return Path.GetFullPath (assembly.MainModule.FullyQualifiedName);
290+
assemblies [name] = Path.GetFullPath (assembly.MainModule.FileName);
290291
}
291292
}
292293
}

0 commit comments

Comments
 (0)