From c49b934a94d7240ad99f75800da7b239356b8fb8 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 12 Feb 2020 22:57:30 -0600 Subject: [PATCH] [Xamarin.Android.Build.Tasks] use Java.Interop's TypeDefinitionCache I was profiling builds with the Mono profiler and noticed: Method call summary Total(ms) Self(ms) Calls Method name 70862 97 89713 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) Almost 90K calls? What is that coming from??? 61422 calls from: Java.Interop.Tools.Cecil.TypeDefinitionRocks/d__1:MoveNext () Java.Interop.Tools.Cecil.TypeDefinitionRocks:GetBaseType (Mono.Cecil.TypeDefinition) Mono.Cecil.TypeReference:Resolve () Mono.Cecil.ModuleDefinition:Resolve (Mono.Cecil.TypeReference) Mono.Cecil.MetadataResolver:Resolve (Mono.Cecil.TypeReference) Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference) Ok, this jogged my memory. Stephane Delcroix had mentioned one of the big wins for XamlC in Xamarin.Forms was to cache any time `TypeReference.Resolve()` was called: https://github.com/xamarin/Xamarin.Forms/blob/1b9c22b4b9b1c1354a3a5c35ad445a2738c6f6c3/Xamarin.Forms.Build.Tasks/TypeReferenceExtensions.cs#L437-L443 XamlC was able to use `static` here, because it's using a feature of MSBuild to run in a separate `AppDomain`: https://github.com/xamarin/Xamarin.Forms/blob/1b9c22b4b9b1c1354a3a5c35ad445a2738c6f6c3/Xamarin.Forms.Build.Tasks/XamlTask.cs#L20-L21 However, I think we can simply add a new non-static `TypeDefinitionCache` class that would allow callers to control the caching strategy. Callers will need to control the scope of the `TypeDefinitionCache` so it matches any `DirectoryAssemblyResolver` being used. Right now most Xamarin.Android builds will open assemblies with Mono.Cecil twice: once for `` and once for the linker. So for example, we can add caching in an API-compatible way: [Obsolete ("Use the TypeDefinitionCache overload for better performance.")] public static TypeDefinition GetBaseType (this TypeDefinition type) => GetBaseType (type, cache: null); public static TypeDefinition GetBaseType (this TypeDefinition type, TypeDefinitionCache cache) { if (bt == null) return null; if (cache != null) return cache.Resolve (bt); return bt.Resolve (); } We just need to ensure `cache: null` is valid. I took this approach for any `public` APIs and made any `private` or `internal` APIs *require* a `TypeDefinitionCache`. I fixed every instance where `[Obsolete]` would cause a warning here in Java.Interop. We'll probably see a small improvement in `generator` and `jnimarshalmethod-gen`. Here in xamarin-android, I fixed all the `[Obsolete]` warnings that were called in ``. I can make more changes for the linker in a future PR. ~~ Results ~~ The reduced calls to `DirectoryAssemblyResolver.Resolve`: Method call summary Total(ms) Self(ms) Calls Method name Before; 70862 97 89713 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) After: 68830 35 26315 Java.Interop.Tools.Cecil.DirectoryAssemblyResolver:Resolve (Mono.Cecil.AssemblyNameReference,Mono.Cecil.ReaderParameters) ~63,398 less calls. In a build of the Xamarin.Forms integration project on macOS / Mono: Before: 1365 ms GenerateJavaStubs 1 calls After: 862 ms GenerateJavaStubs 1 calls It is almost a 40% improvement, around ~500ms better. --- .../Tasks/GenerateJavaStubs.cs | 29 ++++++++++--------- .../Utilities/ManifestDocument.cs | 28 +++++++++--------- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 0fcdea2300d..11d27543278 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -163,7 +163,8 @@ void Run (DirectoryAssemblyResolver res) } // Step 1 - Find all the JLO types - var scanner = new JavaTypeScanner (this.CreateTaskLogger ()) { + var cache = new TypeDefinitionCache (); + var scanner = new JavaTypeScanner (this.CreateTaskLogger (), cache) { ErrorOnCustomJavaObject = ErrorOnCustomJavaObject, }; @@ -175,7 +176,7 @@ void Run (DirectoryAssemblyResolver res) var javaTypes = new List (); foreach (TypeDefinition td in allJavaTypes) { - if (!userAssemblies.ContainsKey (td.Module.Assembly.Name.Name) || JavaTypeScanner.ShouldSkipJavaCallableWrapperGeneration (td)) { + if (!userAssemblies.ContainsKey (td.Module.Assembly.Name.Name) || JavaTypeScanner.ShouldSkipJavaCallableWrapperGeneration (td, cache)) { continue; } @@ -183,7 +184,7 @@ void Run (DirectoryAssemblyResolver res) } // Step 3 - Generate Java stub code - var success = CreateJavaSources (javaTypes); + var success = CreateJavaSources (javaTypes, cache); if (!success) return; @@ -199,7 +200,7 @@ void Run (DirectoryAssemblyResolver res) string managedKey = type.FullName.Replace ('/', '.'); string javaKey = JavaNativeTypeManager.ToJniName (type).Replace ('/', '.'); - acw_map.Write (type.GetPartialAssemblyQualifiedName ()); + acw_map.Write (type.GetPartialAssemblyQualifiedName (cache)); acw_map.Write (';'); acw_map.Write (javaKey); acw_map.WriteLine (); @@ -208,14 +209,14 @@ void Run (DirectoryAssemblyResolver res) bool hasConflict = false; if (managed.TryGetValue (managedKey, out conflict)) { if (!managedConflicts.TryGetValue (managedKey, out var list)) - managedConflicts.Add (managedKey, list = new List { conflict.GetPartialAssemblyName () }); - list.Add (type.GetPartialAssemblyName ()); + managedConflicts.Add (managedKey, list = new List { conflict.GetPartialAssemblyName (cache) }); + list.Add (type.GetPartialAssemblyName (cache)); hasConflict = true; } if (java.TryGetValue (javaKey, out conflict)) { if (!javaConflicts.TryGetValue (javaKey, out var list)) - javaConflicts.Add (javaKey, list = new List { conflict.GetAssemblyQualifiedName () }); - list.Add (type.GetAssemblyQualifiedName ()); + javaConflicts.Add (javaKey, list = new List { conflict.GetAssemblyQualifiedName (cache) }); + list.Add (type.GetAssemblyQualifiedName (cache)); success = false; hasConflict = true; } @@ -228,7 +229,7 @@ void Run (DirectoryAssemblyResolver res) acw_map.Write (javaKey); acw_map.WriteLine (); - acw_map.Write (JavaNativeTypeManager.ToCompatJniName (type).Replace ('/', '.')); + acw_map.Write (JavaNativeTypeManager.ToCompatJniName (type, cache).Replace ('/', '.')); acw_map.Write (';'); acw_map.Write (javaKey); acw_map.WriteLine (); @@ -265,7 +266,7 @@ void Run (DirectoryAssemblyResolver res) manifest.NeedsInternet = NeedsInternet; manifest.InstantRunEnabled = InstantRunEnabled; - var additionalProviders = manifest.Merge (Log, allJavaTypes, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments); + var additionalProviders = manifest.Merge (Log, cache, allJavaTypes, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments); // Only write the new manifest if it actually changed if (manifest.SaveIfChanged (Log, MergedAndroidManifestOutput)) { @@ -286,10 +287,10 @@ void Run (DirectoryAssemblyResolver res) StringWriter regCallsWriter = new StringWriter (); regCallsWriter.WriteLine ("\t\t// Application and Instrumentation ACWs must be registered first."); foreach (var type in javaTypes) { - if (JavaNativeTypeManager.IsApplication (type) || JavaNativeTypeManager.IsInstrumentation (type)) { + if (JavaNativeTypeManager.IsApplication (type, cache) || JavaNativeTypeManager.IsInstrumentation (type, cache)) { string javaKey = JavaNativeTypeManager.ToJniName (type).Replace ('/', '.'); regCallsWriter.WriteLine ("\t\tmono.android.Runtime.register (\"{0}\", {1}.class, {1}.__md_methods);", - type.GetAssemblyQualifiedName (), javaKey); + type.GetAssemblyQualifiedName (cache), javaKey); } } regCallsWriter.Close (); @@ -300,7 +301,7 @@ void Run (DirectoryAssemblyResolver res) template => template.Replace ("// REGISTER_APPLICATION_AND_INSTRUMENTATION_CLASSES_HERE", regCallsWriter.ToString ())); } - bool CreateJavaSources (IEnumerable javaTypes) + bool CreateJavaSources (IEnumerable javaTypes, TypeDefinitionCache cache) { string outputPath = Path.Combine (OutputDirectory, "src"); string monoInit = GetMonoInitSource (AndroidSdkPlatform, UseSharedRuntime); @@ -311,7 +312,7 @@ bool CreateJavaSources (IEnumerable javaTypes) foreach (var t in javaTypes) { using (var writer = MemoryStreamPool.Shared.CreateStreamWriter ()) { try { - var jti = new JavaCallableWrapperGenerator (t, Log.LogWarning) { + var jti = new JavaCallableWrapperGenerator (t, Log.LogWarning, cache) { GenerateOnCreateOverrides = generateOnCreateOverrides, ApplicationJavaClass = ApplicationJavaClass, MonoRuntimeInitialization = monoInit, diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs index 825b7a7736f..54c83b6d9dc 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs @@ -221,7 +221,7 @@ void ReorderActivityAliases (TaskLoggingHelper log, XElement app) } } - public IList Merge (TaskLoggingHelper log, List subclasses, string applicationClass, bool embed, string bundledWearApplicationName, IEnumerable mergedManifestDocuments) + public IList Merge (TaskLoggingHelper log, TypeDefinitionCache cache, List subclasses, string applicationClass, bool embed, string bundledWearApplicationName, IEnumerable mergedManifestDocuments) { string applicationName = ApplicationName; @@ -241,7 +241,7 @@ public IList Merge (TaskLoggingHelper log, List subclass if (manifest.Attribute (androidNs + "versionName") == null) manifest.SetAttributeValue (androidNs + "versionName", "1.0"); - app = CreateApplicationElement (manifest, applicationClass, subclasses); + app = CreateApplicationElement (manifest, applicationClass, subclasses, cache); if (app.Attribute (androidNs + "label") == null && applicationName != null) app.SetAttributeValue (androidNs + "label", applicationName); @@ -296,12 +296,12 @@ public IList Merge (TaskLoggingHelper log, List subclass PackageName = t.Namespace; var name = JavaNativeTypeManager.ToJniName (t).Replace ('/', '.'); - var compatName = JavaNativeTypeManager.ToCompatJniName (t).Replace ('/', '.'); + var compatName = JavaNativeTypeManager.ToCompatJniName (t, cache).Replace ('/', '.'); if (((string) app.Attribute (attName)) == compatName) { app.SetAttributeValue (attName, name); } - Func generator = GetGenerator (t); + Func generator = GetGenerator (t, cache); if (generator == null) continue; @@ -395,7 +395,7 @@ public IList Merge (TaskLoggingHelper log, List subclass } } - AddInstrumentations (manifest, subclasses, targetSdkVersionValue); + AddInstrumentations (manifest, subclasses, targetSdkVersionValue, cache); AddPermissions (app); AddPermissionGroups (app); AddPermissionTrees (app); @@ -492,20 +492,20 @@ IEnumerable FixupNameElements(string packageName, IEnumerable node return nodes; } - Func GetGenerator (TypeDefinition type) + Func GetGenerator (TypeDefinition type, TypeDefinitionCache cache) { - if (type.IsSubclassOf ("Android.App.Activity")) + if (type.IsSubclassOf ("Android.App.Activity", cache)) return ActivityFromTypeDefinition; - if (type.IsSubclassOf ("Android.App.Service")) + if (type.IsSubclassOf ("Android.App.Service", cache)) return (t, name, v) => ToElement (t, name, ServiceAttribute.FromTypeDefinition, x => x.ToElement (PackageName)); - if (type.IsSubclassOf ("Android.Content.BroadcastReceiver")) + if (type.IsSubclassOf ("Android.Content.BroadcastReceiver", cache)) return (t, name, v) => ToElement (t, name, BroadcastReceiverAttribute.FromTypeDefinition, x => x.ToElement (PackageName)); - if (type.IsSubclassOf ("Android.Content.ContentProvider")) + if (type.IsSubclassOf ("Android.Content.ContentProvider", cache)) return (t, name, v) => ToProviderElement (t, name); return null; } - XElement CreateApplicationElement (XElement manifest, string applicationClass, List subclasses) + XElement CreateApplicationElement (XElement manifest, string applicationClass, List subclasses, TypeDefinitionCache cache) { var application = manifest.Descendants ("application").FirstOrDefault (); @@ -534,7 +534,7 @@ XElement CreateApplicationElement (XElement manifest, string applicationClass, L if (aa == null) continue; - if (!t.IsSubclassOf ("Android.App.Application")) + if (!t.IsSubclassOf ("Android.App.Application", cache)) throw new InvalidOperationException (string.Format ("Found [Application] on type {0}. [Application] can only be used on subclasses of Application.", t.FullName)); typeAttr.Add (aa); @@ -860,7 +860,7 @@ void AddSupportsGLTextures (XElement application) } } - void AddInstrumentations (XElement manifest, IList subclasses, int targetSdkVersion) + void AddInstrumentations (XElement manifest, IList subclasses, int targetSdkVersion, TypeDefinitionCache cache) { var assemblyAttrs = Assemblies.SelectMany (path => InstrumentationAttribute.FromCustomAttributeProvider (Resolver.GetAssembly (path))); @@ -874,7 +874,7 @@ void AddInstrumentations (XElement manifest, IList subclasses, i } foreach (var type in subclasses) - if (type.IsSubclassOf ("Android.App.Instrumentation")) { + if (type.IsSubclassOf ("Android.App.Instrumentation", cache)) { var xe = InstrumentationFromTypeDefinition (type, JavaNativeTypeManager.ToJniName (type).Replace ('/', '.'), targetSdkVersion); if (xe != null) manifest.Add (xe);