Bad direction, too complex...

Trying to perform type scanning only once and then "mirror" assemblies from all other supported ABIs based on the first scan appears to be too fragile and overly complicated. It appears the best course of action is to scan for Java types over each assemblies for each architecture and then compare the sets (based on type names). JCWs can still be generated only once, of course.
dotnet · Nov 29, 2023 · c3ae673 · c3ae673
1 parent a6eb63f
commit c3ae673
Show file tree

Hide file tree

Showing 4 changed files with 248 additions and 60 deletions.
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
@@ -208,26 +208,35 @@ void Run (bool useMarshalMethods)
 
 			// Now that "never" never happened, we can proceed knowing that at least the assembly sets are the same for each architecture
 			MarshalMethodsClassifier? classifier = null;
-			AndroidTargetArch firstArch = AndroidTargetArch.None;
-			bool archAgnosticDone = false;
+			MarshalMethodsMirrorHelperState? mirrorHelperState = null;
+			bool abiAgnosticCode = false;
 
 			foreach (var kvp in allAssembliesPerArch) {
 				AndroidTargetArch arch = kvp.Key;
 				Dictionary<string, ITaskItem> archAssemblies = kvp.Value;
 
 				XAAssemblyResolverNew res = MakeResolver (useMarshalMethods, arch, archAssemblies);
-				if (!archAgnosticDone) {
+				if (!abiAgnosticCode) {
 					var cache = new TypeDefinitionCache ();
 					(List<JavaType> allJavaTypes, List<JavaType> javaTypesForJCW) = ScanForJavaTypes (res, cache, archAssemblies, userAssembliesPerArch[arch], useMarshalMethods);
 
 					if (!GenerateJavaSourcesAndMaybeClassifyMarshalMethods (res, javaTypesForJCW, cache, useMarshalMethods, out classifier)) {
 						return;
 					}
-					firstArch = arch;
-					archAgnosticDone = true;
+					abiAgnosticCode = true;
+
+					if (useMarshalMethods) {
+						mirrorHelperState = new MarshalMethodsMirrorHelperState (arch, archAssemblies, classifier);
+					}
+				}
+
+				if (mirrorHelperState != null) {
+					mirrorHelperState.CurrentArch = arch;
+					mirrorHelperState.CurrentArchResolver = res;
+					mirrorHelperState.CurrentArchAssemblies = archAssemblies;
 				}
 
-				RewriteMarshalMethods (classifier, firstArch, allAssembliesPerArch);
+				RewriteMarshalMethods (mirrorHelperState);
 			}
 		}
 
@@ -252,14 +261,14 @@ void Run (bool useMarshalMethods)
 			return (allJavaTypes, javaTypesForJCW);
 		}
 
-		void RewriteMarshalMethods (MarshalMethodsClassifier? classifier, AndroidTargetArch classifiedArch, Dictionary<AndroidTargetArch, Dictionary<string, ITaskItem>> allAssembliesPerArch)
+		void RewriteMarshalMethods (MarshalMethodsMirrorHelperState? mirrorHelperState)
 		{
-			if (classifier == null) {
+			if (mirrorHelperState == null) {
 				return;
 			}
 
-			var mirrorHelper = new MarshalMethodsMirrorHelper (classifier, classifiedArch, allAssembliesPerArch, Log);
-			IDictionary<AndroidTargetArch, ArchitectureMarshalMethods> perArchMarshalMethods = mirrorHelper.Reflect ();
+			var mirrorHelper = new MarshalMethodsMirrorHelper (mirrorHelperState, Log);
+			ArchitectureMarshalMethods perArchMarshalMethods = mirrorHelper.Reflect ();
 
 			// We need to parse the environment files supplied by the user to see if they want to use broken exception transitions. This information is needed
 			// in order to properly generate wrapper methods in the marshal methods assembly rewriter.
@@ -888,7 +897,7 @@ void FindMatchingMethodInType (MarshalMethodEntry methodEntry, TypeDefinition ty
 				}
 
 				Log.LogDebugMessage ($"Found match for '{typeNativeCallbackMethod.FullName}' in {type.Module.FileName}");
-				string methodKey = classifier.GetStoreMethodKey (methodEntry);
+				string methodKey = methodEntry.GetStoreMethodKey (classifier.TypeDefinitionCache);
 				classifier.MarshalMethods[methodKey].Add (new MarshalMethodEntry (methodEntry, typeNativeCallbackMethod));
 			}
 		}

diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs
@@ -80,6 +80,13 @@ string EnsureNonEmpty (string s, string argName)
 
 			return s;
 		}
+
+		public string GetStoreMethodKey (TypeDefinitionCache tdCache)
+		{
+			MethodDefinition registeredMethod = RegisteredMethod;
+			string typeName = registeredMethod.DeclaringType.FullName.Replace ('/', '+');
+			return $"{typeName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}\t{registeredMethod.Name}";
+		}
 	}
 
 	class MarshalMethodsClassifier : JavaCallableMethodClassifier
@@ -236,6 +243,7 @@ public bool Matches (MethodDefinition method)
 		public ICollection<AssemblyDefinition> Assemblies => assemblies;
 		public ulong RejectedMethodCount => rejectedMethodCount;
 		public ulong WrappedMethodCount => wrappedMethodCount;
+		public TypeDefinitionCache TypeDefinitionCache => tdCache;
 
 		public MarshalMethodsClassifier (TypeDefinitionCache tdCache, IAssemblyResolver res, TaskLoggingHelper log)
 		{
@@ -688,16 +696,9 @@ FieldDefinition FindField (TypeDefinition type, string fieldName, bool lookForIn
 			return FindField (tdCache.Resolve (type.BaseType), fieldName, lookForInherited);
 		}
 
-		public string GetStoreMethodKey (MarshalMethodEntry methodEntry)
-		{
-			MethodDefinition registeredMethod = methodEntry.RegisteredMethod;
-			string typeName = registeredMethod.DeclaringType.FullName.Replace ('/', '+');
-			return $"{typeName}, {registeredMethod.DeclaringType.GetPartialAssemblyName (tdCache)}\t{registeredMethod.Name}";
-		}
-
 		void StoreMethod (MarshalMethodEntry entry)
 		{
-			string key = GetStoreMethodKey (entry);
+			string key = entry.GetStoreMethodKey (tdCache);
 
 			// Several classes can override the same method, we need to generate the marshal method only once, at the same time
 			// keeping track of overloads