[Xamarin.Android.Tools.ApiXmlAdjuster] Use Dictionaries for perf (#756)

The `Xamarin.Android.Tools.ApiXmlAdjuster` process builds an in-memory
model of every Java type we know about, and then queries this model
many times in order to ensure we can resolve every needed Java type to
build a binding.  The data structure of this model is:

	public class JavaApi
	{
	    public List<JavaPackage> Packages { get; }
	}

	public class JavaPackage
	{
	    public List<JavaType> Types { get; }
	}

The model is then queried using LINQ::

	// Bad
	var type = api.Packages.SelectMany (p => p.Types)
	    .FirstOrDefault (t => t.Name == type_name);

	// Less bad
	var type = api.Packages.FirstOrDefault (p => p.Name == pkg_name)
	    ?.Types.FirstOrDefault (t => t.Name == type_name);


In the random GooglePlayServices package I used for testing, `JavaApi`
contained 310 packages and a total of 5941 types.  Repeatedly looping
through them looking for the correct type takes a considerable amount
of time.

Instead, we can use a `Dictionary` to store packages and types keyed by
name to significantly speed up type resolution:

	public class JavaApi
	{
	    public Dictionary<string, JavaPackage> Packages { get; }
	}

	public class JavaPackage
	{
	    public Dictionary<string, List<JavaType>> Types { get; }
	}


For the GPS project, this reduced time taken considerably:

| Version | Time |
| ------- | ---- |
|  master | 9.3s |
| This PR | 1.4s |

The only "interesting" detail is that we can have multiple types with
the same *Java* name, such as `MyInterface` and `MyInterfaceConsts`.
Thus we need to use `Dictionary<string, List<JavaType>>` to ensure we
collect them all.

src/Java.Interop.Tools.JavaSource/Java.Interop.Tools.JavaSource/JavaStubParser.cs

@@ -315,7 +315,12 @@ public JavaStubGrammar ()
 			identifier.AstConfig.NodeCreator = (ctx, node) => node.AstNode = node.Token.ValueString;
 			compile_unit.AstConfig.NodeCreator = (ctx, node) => {
 				ProcessChildren (ctx, node);
-				node.AstNode = new JavaPackage (null) { Name = (string)node.ChildNodes [0].AstNode, Types = ((IEnumerable<JavaType>)node.ChildNodes [2].AstNode).ToList () };
+				var pkg = new JavaPackage (null) { Name = (string) node.ChildNodes [0].AstNode };
+
+				foreach (var t in (IEnumerable<JavaType>) node.ChildNodes [2].AstNode)
+					pkg.AddType (t);
+
+				node.AstNode = pkg;
 			};
 			opt_package_decl.AstConfig.NodeCreator = SelectSingleChild;
 			package_decl.AstConfig.NodeCreator = SelectChildValueAt (1);
@@ -637,9 +642,13 @@ public JavaStubParser ()
 		void FlattenNestedTypes (JavaPackage package)
 		{
 			var results = new List<JavaType> ();
-			foreach (var t in package.Types)
+			foreach (var t in package.AllTypes)
 				Flatten (results, t);
-			package.Types = results.ToList ();
+
+			package.ClearTypes ();
+
+			foreach (var t in results)
+				package.AddType (t);
 
 			void Flatten (List<JavaType> list, JavaType t)
 			{

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs

@@ -10,29 +10,77 @@ public partial class JavaApi
 	{
 		public JavaApi ()
 		{
-			Packages = new List<JavaPackage> ();
+			Packages = new Dictionary<string, JavaPackage> ();
 		}
 
 		public  string?                 ExtendedApiSource   { get; set; }
 		public  string?                 Platform            { get; set; }
-		public  IList<JavaPackage>      Packages            { get; set; }
+		public  IDictionary<string, JavaPackage>      Packages            { get; }
+
+		public ICollection<JavaPackage> AllPackages => Packages.Values;
 	}
 
 	public partial class JavaPackage
 	{
+		private Dictionary<string, List<JavaType>> types = new Dictionary<string, List<JavaType>> ();
+
 		public JavaPackage (JavaApi? parent)
 		{
 			Parent = parent;
-
-			Types = new List<JavaType> ();
 		}
 
 		public  JavaApi?                Parent              { get; private set; }
 
 		public  string?                 Name                { get; set; }
 		public  string?                 JniName             { get; set; }
-		public  IList<JavaType>         Types               { get; set; }
-
+
+		// Yes, there can be multiple types with the same *Java* name.
+		// For example:
+		// - MyInterface
+		// - MyInterfaceConsts
+		// It's debatable whether we handle this "properly", as most callers just
+		// do `First ()`, but it's been working for years so I'm not changing it.
+		// Exposes an IReadOnlyDictionary so caller cannot bypass our AddType/RemoveType code.
+		public  IReadOnlyDictionary<string, List<JavaType>>   Types => types;
+
+		// Use this for a flat list of *all* types
+		public IEnumerable<JavaType> AllTypes => Types.Values.SelectMany (v => v);
+
+		public void AddType (JavaType type)
+		{
+			// If this is a duplicate key, add it to existing list
+			if (Types.TryGetValue (type.Name!, out var list)) {
+				list.Add (type);
+				return;
+			}
+
+			// Add to a new list
+			var new_list = new List<JavaType> ();
+			new_list.Add (type);
+
+			types.Add (type.Name!, new_list);
+		}
+
+		public void RemoveType (JavaType type)
+		{
+			if (!Types.TryGetValue (type.Name!, out var list))
+				return;
+
+			// Remove 1 type from list if it contains multiple types
+			if (list.Count > 1) {
+				list.Remove (type);
+				return;
+			}
+
+			// Remove the whole dictionary entry
+			types.Remove (type.Name!);
+		}
+
+		public void ClearTypes ()
+		{
+			types.Clear ();
+		}
+
 		// Content of this value is not stable.
 		public override string ToString ()
 		{
@@ -123,14 +171,14 @@ static ManagedType ()
 			dummy_system_package = new JavaPackage (null) { Name = "System" };
 			system_object = new ManagedType (dummy_system_package) { Name = "Object" };
 			system_exception = new ManagedType (dummy_system_package) { Name = "Exception" };
-			dummy_system_package.Types.Add (system_object);
-			dummy_system_package.Types.Add (system_exception);
+			dummy_system_package.AddType (system_object);
+			dummy_system_package.AddType (system_exception);
 			dummy_system_io_package = new JavaPackage (null) { Name = "System.IO" };
 			system_io_stream = new ManagedType (dummy_system_io_package) { Name = "Stream" };
-			dummy_system_io_package.Types.Add (system_io_stream);
+			dummy_system_io_package.AddType (system_io_stream);
 			dummy_system_xml_package = new JavaPackage (null) { Name = "System.Xml" };
 			system_xml_xmlreader = new ManagedType (dummy_system_xml_package) { Name = "XmlReader" };
-			dummy_system_io_package.Types.Add (system_xml_xmlreader);
+			dummy_system_io_package.AddType (system_xml_xmlreader);
 		}
 
 		public static IEnumerable<JavaPackage> DummyManagedPackages {

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiDefectFinderExtensions.cs

@@ -7,7 +7,7 @@ public static class JavaApiDefectFinderExtensions
 	{
 		public static void FindDefects (this JavaApi api)
 		{
-			foreach (var type in api.Packages.SelectMany (p => p.Types).Where (t => !t.IsReferenceOnly))
+			foreach (var type in api.AllPackages.SelectMany (p => p.AllTypes).Where (t => !t.IsReferenceOnly))
 				type.FindDefects ();
 		}
 

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiGenericInheritanceMapperExtensions.cs

@@ -8,7 +8,7 @@ public static class JavaApiGenericInheritanceMapperExtensions
 	{
 		public static void CreateGenericInheritanceMapping (this JavaApi api)
 		{
-			foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
+			foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
 				kls.PrepareGenericInheritanceMapping ();
 		}
 

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiNonBindableStripper.cs

@@ -9,7 +9,7 @@ public static class JavaApiNonBindableStripper
 		public static void StripNonBindables (this JavaApi api)
 		{
 			var invalids = new List<JavaMember> ();
-			foreach (var member in api.Packages.SelectMany (p => p.Types)
+			foreach (var member in api.AllPackages.SelectMany (p => p.AllTypes)
 			         .SelectMany (t => t.Members).Where (m => m.Name != null && m.Name.Contains ('$')))
 				invalids.Add (member);
 			foreach (var invalid in invalids)

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiOverrideMarkerExtensions.cs

@@ -14,7 +14,7 @@ public static void MarkOverrides (this JavaApi api)
 
 		public static void MarkOverrides (this JavaApi api, HashSet<JavaClass> doneList)
 		{
-			foreach (var kls in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ())
+			foreach (var kls in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ())
 				kls.MarkOverrides (doneList);
 		}
 

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiTypeResolverExtensions.cs

@@ -39,13 +39,31 @@ static JavaTypeReference JavaTypeNameToReference (this JavaApi api, JavaTypeName
 
 		public static JavaType FindNonGenericType (this JavaApi api, string? name)
 		{
-			var ret = FindPackages (api, name ?? "")
-				.SelectMany (p => p.Types)
-				.FirstOrDefault (t => name == (t.Parent?.Name != null ? t.Parent.Name + "." : "") + t.Name);
-			if (ret == null)
-				ret = ManagedType.DummyManagedPackages
-				                 .SelectMany (p => p.Types)
-				                 .FirstOrDefault (t => t.FullName == name);
+			// Given a type name like 'android.graphics.BitmapFactory.Options'
+			// We're going to search for:
+			// - Pkg: android.graphics.BitmapFactory  Type: Options
+			// - Pkg: android.graphics                Type: BitmapFactory.Options
+			// - Pkg: android                         Type: graphics.BitmapFactory.Options
+			// etc.  We will short-circuit as soon as we find a match
+			var index = name?.LastIndexOf ('.') ?? -1;
+
+			while (index > 0) {
+				var ns = name!.Substring (0, index);
+				var type_name = name.Substring (index + 1);
+
+				if (api.Packages.TryGetValue (ns, out var pkg)) {
+					if (pkg.Types.TryGetValue (type_name, out var type))
+						return type.First ();
+				}
+
+				index = name.LastIndexOf ('.', index - 1);
+			}
+
+			// See if it's purely a C# type
+			var ret = ManagedType.DummyManagedPackages
+				.SelectMany (p => p.AllTypes)
+				.FirstOrDefault (t => t.FullName == name);
+
 			if (ret == null) {
 				// We moved this type to "mono.android.app.IntentService" which makes this
 				// type resolution fail if a user tries to reference it in Java.
@@ -58,43 +76,26 @@ public static JavaType FindNonGenericType (this JavaApi api, string? name)
 			return ret;
 		}
 
-		static IEnumerable<JavaPackage> FindPackages (JavaApi api, string name)
-		{
-			// Given a type name like "java.lang.Object", return packages that could
-			// possibly contain the type so we don't search all packages, ie:
-			// - java.lang
-			// - java
-			var package_names = new List<string> ();
-			int index;
-
-			while ((index = name.LastIndexOf ('.')) >= 0) {
-				name = name.Substring (0, index);
-				package_names.Add (name);
-			}
-
-			return api.Packages.Where (p => package_names.Contains (p.Name, StringComparer.Ordinal)).ToList ();
-		}
-
 		public static void Resolve (this JavaApi api)
 		{
 			while (true) {
 				bool errors = false;
-				foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaClass> ().ToArray ())
+				foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaClass> ().ToArray ())
 					try {
 						t.Resolve ();
 					}
 					catch (JavaTypeResolutionException ex) {
 						Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
 						errors = true;
-						t.Parent?.Types.Remove (t);
+						t.Parent?.RemoveType (t);
 					}
-				foreach (var t in api.Packages.SelectMany (p => p.Types).OfType<JavaInterface> ().ToArray ())
+				foreach (var t in api.AllPackages.SelectMany (p => p.AllTypes).OfType<JavaInterface> ().ToArray ())
 					try {
 						t.Resolve ();
 					} catch (JavaTypeResolutionException ex) {
 						Log.LogError ("Error while processing type '{0}': {1}", t, ex.Message);
 						errors = true;
-						t.Parent?.Types.Remove (t);
+						t.Parent?.RemoveType (t);
 					}
 				if (!errors)
 					break;

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlGeneratorExtensions.cs

@@ -24,15 +24,15 @@ public static void Save (this JavaApi api, XmlWriter writer)
 			if (api.Platform != null)
 				writer.WriteAttributeString ("platform", api.Platform);
 
-			foreach (var pkg in api.Packages) {
-				if (!pkg.Types.Any (t => !t.IsReferenceOnly))
+			foreach (var pkg in api.AllPackages) {
+				if (!pkg.AllTypes.Any (t => !t.IsReferenceOnly))
 					continue;
 				writer.WriteStartElement ("package");
 				writer.WriteAttributeString ("name", pkg.Name);
 				if (!string.IsNullOrEmpty (pkg.JniName)) {
 					writer.WriteAttributeString ("jni-name", pkg.JniName);
 				}
-				foreach (var type in pkg.Types) {
+				foreach (var type in pkg.AllTypes) {
 					if (type.IsReferenceOnly)
 						continue; // skip reference only types
 					if (type is JavaClass)

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiXmlLoaderExtensions.cs

@@ -32,11 +32,14 @@ public static void Load (this JavaApi api, XmlReader reader, bool isReferenceOnl
 						break; // </api>
 					if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "package")
 						throw XmlUtil.UnexpectedElementOrContent ("api", reader, "package");
-					var pkg = api.Packages.FirstOrDefault (p => p.Name == reader.GetAttribute ("name"));
-					if (pkg == null) {
+
+					var name = reader.GetAttribute ("name");
+
+					if (!api.Packages.TryGetValue (name, out var pkg)) {
 						pkg = new JavaPackage (api);
-						api.Packages.Add (pkg);
+						api.Packages.Add (name, pkg);
 					}
+
 					pkg.Load (reader, isReferenceOnly);
 				} while (true);
 
@@ -67,11 +70,11 @@ public static void Load (this JavaPackage package, XmlReader reader, bool isRefe
 					if (reader.LocalName == "class") {
 						var kls = new JavaClass (package) { IsReferenceOnly = isReferenceOnly };
 						kls.Load (reader);
-						package.Types.Add (kls);
+						package.AddType (kls);
 					} else if (reader.LocalName == "interface") {
 						var iface = new JavaInterface (package) { IsReferenceOnly = isReferenceOnly };
 						iface.Load (reader);
-						package.Types.Add (iface);
+						package.AddType (iface);
 					} else
 						throw XmlUtil.UnexpectedElementOrContent ("package", reader, "class", "interface");
 				} while (true);

tests/Java.Interop.Tools.JavaSource-Tests/JavaStubParserTests.cs

@@ -35,7 +35,7 @@ public static void m (String text) {
 			Assert.AreEqual (null, package.Name);
 			Assert.AreEqual (1, package.Types.Count);
 
-			var Example_Type = package.Types [0];
+			var Example_Type = package.AllTypes.First ();
 			Assert.AreEqual ("Example", Example_Type.FullName);
 
 			Assert.AreEqual (1, Example_Type.Members.Count);

tests/Xamarin.Android.Tools.ApiXmlAdjuster-Tests/JavaApiTest.cs

@@ -19,9 +19,9 @@ public void SetupFixture ()
 		[Test]
 		public void TestToString ()
 		{
-			var pkg = api.Packages.First (p => p.Name == "android.database");
+			var pkg = api.AllPackages.First (p => p.Name == "android.database");
 			Assert.AreEqual ("[Package] android.database", pkg.ToString ());
-			var kls = pkg.Types.First (t => t.FullName == "android.database.ContentObservable");
+			var kls = pkg.AllTypes.First (t => t.FullName == "android.database.ContentObservable");
 			Assert.AreEqual ("[Class] android.database.ContentObservable", kls.ToString ());
 		}
 

tests/Xamarin.Android.Tools.ApiXmlAdjuster-Tests/OverrideMarkerTest.cs

@@ -53,7 +53,7 @@ public void AncestralOverrides ()
 			xapi.Resolve ();
 			xapi.CreateGenericInheritanceMapping ();
 			xapi.MarkOverrides ();
-			var t = xapi.Packages.First (_ => _.Name == "XXX").Types.First (_ => _.Name == "SherlockExpandableListActivity");
+			var t = xapi.AllPackages.First (_ => _.Name == "XXX").AllTypes.First (_ => _.Name == "SherlockExpandableListActivity");
 			var m = t.Members.OfType<JavaMethod> ().First (_ => _.Name == "addContentView");
 			Assert.IsNotNull (m.BaseMethod, "base method not found");
 		}
@@ -88,7 +88,7 @@ public void GenericConstructors ()
 			xapi = new JavaApi ();
 			using (var xr = XmlReader.Create (new StringReader (sw.ToString ())))
 				xapi.Load (xr, true);
-			var t = xapi.Packages.First (_ => _.Name == "XXX").Types.First (_ => _.Name == "GenericConstructors");
+			var t = xapi.AllPackages.First (_ => _.Name == "XXX").AllTypes.First (_ => _.Name == "GenericConstructors");
 			var m = t.Members.OfType<JavaConstructor> ().FirstOrDefault ();
 			Assert.IsNotNull (m.TypeParameters, "constructor not found");
 		}

tests/Xamarin.Android.Tools.ApiXmlAdjuster-Tests/TypeResolverTest.cs

@@ -76,20 +76,20 @@ public void IntentServiceHack ()
 			var api = JavaApiTestHelper.GetLoadedApi ();
 
 			// Create "mono.android.app" package
-			var mono_android_app = new JavaPackage (api) { Name = "mono.android.app", JniName = "mono/android/app", Types = new List<JavaType> () };
-			api.Packages.Add (mono_android_app);
+			var mono_android_app = new JavaPackage (api) { Name = "mono.android.app", JniName = "mono/android/app" };
+			api.Packages.Add (mono_android_app.Name, mono_android_app);
 
 			// Remove "android.app.IntentService" type
-			var android_app = api.Packages.Single (p => p.Name == "android.app");
-			var intent_service = android_app.Types.Single (t => t.Name == "IntentService");
-			android_app.Types.Remove (intent_service);
+			var android_app = api.AllPackages.Single (p => p.Name == "android.app");
+			var intent_service = android_app.AllTypes.Single (t => t.Name == "IntentService");
+			android_app.RemoveType (intent_service);
 
 			// Create new "mono.android.app.IntentService" type
 			var new_intent_service = new JavaClass (mono_android_app) {
 				Name = intent_service.Name,
 			};
 
-			mono_android_app.Types.Add (new_intent_service);
+			mono_android_app.AddType (new_intent_service);
 
 			api.Resolve ();