[Xamarin.Android.Build.Tasks] should Dispose AssemblyDefinition (#2148)

jonathanpeppers · jonpryor · commit fc6b7e94557a · 2018-09-06T11:01:27.000-04:00
Reviewing our codebase, we found a place where the `<BuildApk/>` task was calling `MonoAndroidHelper.IsReferenceAssembly(string)`, and `IsReferenceAssembly(string)` had two notable problems when it was first introduced in 8f2ae24: 1. It was using `ReaderParameters.InMemory`=true 2. We were not calling `AssemblyDefinition.Dispose()`! (1) is not ideal, as we load *every* assembly into memory, including our ~20-30MB `Mono.Android.dll`! foreach (ITaskItem assembly in ResolvedUserAssemblies) { if (MonoAndroidHelper.IsReferenceAssembly (assembly.ItemSpec)) { //... foreach (ITaskItem assembly in ResolvedFrameworkAssemblies) { if (MonoAndroidHelper.IsReferenceAssembly (assembly.ItemSpec)) { Update `MonoAndroidHelper.IsReferenceAssembly(string)` so that instead of loading the assembly into memory, we instead wrap the `AssemblyDefinition` use within a `using` block. This prevents the assembly file from being held open for an unduly long time, and allows us to stop using `ReaderParameters.InMemory=true`, reducing memory use. This makes the `<BuildApk/>` task *slower*, but *improves* the overall build time: Before: 5890 ms BuildApk 1 calls Time Elapsed 00:01:02.59 After: 6377 ms BuildApk 1 calls Time Elapsed 00:00:59.37 I think the `AssemblyDefinition` instances must have been getting cleaned up later in the build by the GC (if we were lucky), and also used *a lot* more memory. This memory use and related GC would contribute to overall build time.
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
@@ -307,8 +307,9 @@ public static bool IsFrameworkAssembly (string assembly, bool checkSdkPath)
 
 		public static bool IsReferenceAssembly (string assembly)
 		{
-			var a = AssemblyDefinition.ReadAssembly (assembly, new ReaderParameters () { InMemory = true, ReadSymbols = false, });
-			return IsReferenceAssembly (a);
+			var rp = new ReaderParameters { ReadSymbols = false };
+			using (var a = AssemblyDefinition.ReadAssembly (assembly, rp))
+				return IsReferenceAssembly (a);
 		}
 
 		public static bool IsReferenceAssembly (AssemblyDefinition assembly)

Original file line number	Diff line number	Diff line change
`@@ -307,8 +307,9 @@ public static bool IsFrameworkAssembly (string assembly, bool checkSdkPath)`
`307`	`307`
`308`	`308`	`public static bool IsReferenceAssembly (string assembly)`
`309`	`309`	`{`
`310`		`- var a = AssemblyDefinition.ReadAssembly (assembly, new ReaderParameters () { InMemory = true, ReadSymbols = false, });`
`311`		`- return IsReferenceAssembly (a);`
	`310`	`+ var rp = new ReaderParameters { ReadSymbols = false };`
	`311`	`+ using (var a = AssemblyDefinition.ReadAssembly (assembly, rp))`
	`312`	`+ return IsReferenceAssembly (a);`
`312`	`313`	`}`
`313`	`314`
`314`	`315`	`public static bool IsReferenceAssembly (AssemblyDefinition assembly)`