From 68a67c946ce4f652bbea5e487bcac5aa22f0de55 Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Tue, 16 Oct 2018 16:22:27 +0200 Subject: [PATCH 1/3] [jnimarshalmethod-gen] Avoid creating duplicate marshal methods Fixes https://github.com/xamarin/java.interop/issues/384 Sort the methods before generating marshal methods. Sort them so that methods with name containing the interface name are at the end. During processing check whether we already have a marshal method with the same name. (note that the name contains parameter types) --- tools/jnimarshalmethod-gen/App.cs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tools/jnimarshalmethod-gen/App.cs b/tools/jnimarshalmethod-gen/App.cs index 68b707e70..3f5205950 100644 --- a/tools/jnimarshalmethod-gen/App.cs +++ b/tools/jnimarshalmethod-gen/App.cs @@ -221,6 +221,22 @@ static TypeBuilder GetTypeBuilder (ModuleBuilder mb, Type type) return tb; } + class MethodsComparer : IComparer + { + public int Compare (MethodInfo a, MethodInfo b) + { + bool aContainsDot = a.Name.IndexOf ('.') != -1; + bool bContainsDot = b.Name.IndexOf ('.') != -1; + + if (aContainsDot ^ bContainsDot) + return bContainsDot ? -1 : 1; + + return string.Compare (a.Name, b.Name); + } + } + + static HashSet addedMethods = new HashSet (); + void CreateMarshalMethodAssembly (string path) { var assembly = Assembly.LoadFile (Path.GetFullPath (path)); @@ -299,7 +315,13 @@ void CreateMarshalMethodAssembly (string path) var flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; - foreach (var method in type.GetMethods (flags)) { + + var methods = type.GetMethods (flags); + Array.Sort (methods, new MethodsComparer ()); + + addedMethods.Clear (); + + foreach (var method in methods) { // TODO: Constructors var export = method.GetCustomAttribute (); string signature = null; @@ -328,6 +350,9 @@ void CreateMarshalMethodAssembly (string path) if (dt == null) dt = GetTypeBuilder (dm, type); + if (addedMethods.Contains (methodName)) + continue; + if (Verbose) { Console.Write ("Adding marshal method for "); ColorWriteLine ($"{method}", ConsoleColor.Green ); @@ -349,6 +374,8 @@ void CreateMarshalMethodAssembly (string path) signature = builder.GetJniMethodSignature (method); registrationElements.Add (CreateRegistration (name, signature, lambda, targetType, methodName)); + + addedMethods.Add (methodName); } if (dt != null) AddRegisterNativeMembers (dt, targetType, registrationElements); From 38794bfb18e62bd70afa7c826c5c54c5f724eb1c Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Mon, 22 Oct 2018 21:08:30 +0200 Subject: [PATCH 2/3] [jnimarshalmethod-gen] Use HasOverrides when sorting methods When sorting methods so that interface implementation methods are last, use `MethodDefinition:HasOverrides` to identify interface implementations. Introduce new cache to map reflection methods to cecil method definitions. Example cache hit ratio: ~ 92% (1210/1318) when generating marshal methods for `Mono.Android.dll:Java.Nio.CharBuffer*` types. --- tools/jnimarshalmethod-gen/App.cs | 46 +++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/tools/jnimarshalmethod-gen/App.cs b/tools/jnimarshalmethod-gen/App.cs index 3f5205950..8f2dfcbab 100644 --- a/tools/jnimarshalmethod-gen/App.cs +++ b/tools/jnimarshalmethod-gen/App.cs @@ -172,6 +172,8 @@ void ProcessAssemblies (List assemblies) ad = AssemblyDefinition.ReadAssembly (assembly, readWriteParametersNoSymbols); resolver.AddToCache (ad); } + + Extensions.MethodMap.Clear (); } foreach (var assembly in assemblies) { @@ -223,13 +225,33 @@ static TypeBuilder GetTypeBuilder (ModuleBuilder mb, Type type) class MethodsComparer : IComparer { + readonly Type type; + readonly TypeDefinition td; + + public MethodsComparer (Type type, TypeDefinition td) + { + this.type = type; + this.td = td; + } + public int Compare (MethodInfo a, MethodInfo b) { - bool aContainsDot = a.Name.IndexOf ('.') != -1; - bool bContainsDot = b.Name.IndexOf ('.') != -1; + if (a.DeclaringType != type) + return 1; + + var atd = td.GetMethodDefinition (a); + if (atd == null) + return 1; - if (aContainsDot ^ bContainsDot) - return bContainsDot ? -1 : 1; + if (b.DeclaringType != type) + return -1; + + var btd = td.GetMethodDefinition (b); + if (btd == null) + return -1; + + if (atd.HasOverrides ^ btd.HasOverrides) + return btd.HasOverrides ? -1 : 1; return string.Compare (a.Name, b.Name); } @@ -317,7 +339,7 @@ void CreateMarshalMethodAssembly (string path) BindingFlags.Instance | BindingFlags.Static; var methods = type.GetMethods (flags); - Array.Sort (methods, new MethodsComparer ()); + Array.Sort (methods, new MethodsComparer (type, td)); addedMethods.Clear (); @@ -490,11 +512,11 @@ static TypeDefinition FindType (Type type) } } - static class Extensions + internal static class Extensions { public static string GetCecilName (this Type type) { - return type.FullName.Replace ('+', '/'); + return type.FullName?.Replace ('+', '/'); } static bool CompareTypes (Type reflectionType, TypeReference cecilType) @@ -530,11 +552,19 @@ static bool MethodsAreEqual (MethodInfo methodInfo, MethodDefinition methodDefin return true; } + internal static Dictionary MethodMap = new Dictionary (); + public static MethodDefinition GetMethodDefinition (this TypeDefinition td, MethodInfo method) { + if (MethodMap.ContainsKey (method)) + return MethodMap [method]; + foreach (var m in td.Methods) - if (MethodsAreEqual (method, m)) + if (MethodsAreEqual (method, m)) { + MethodMap [method] = m; + return m; + } return null; } From 7a7341f99646d03e31110c88616d1d681707cbed Mon Sep 17 00:00:00 2001 From: Radek Doulik Date: Tue, 23 Oct 2018 10:14:15 +0200 Subject: [PATCH 3/3] Use TryGetValue instead of ContainsKey --- tools/jnimarshalmethod-gen/App.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/jnimarshalmethod-gen/App.cs b/tools/jnimarshalmethod-gen/App.cs index 8f2dfcbab..bc6a6332a 100644 --- a/tools/jnimarshalmethod-gen/App.cs +++ b/tools/jnimarshalmethod-gen/App.cs @@ -556,8 +556,8 @@ static bool MethodsAreEqual (MethodInfo methodInfo, MethodDefinition methodDefin public static MethodDefinition GetMethodDefinition (this TypeDefinition td, MethodInfo method) { - if (MethodMap.ContainsKey (method)) - return MethodMap [method]; + if (MethodMap.TryGetValue (method, out var md)) + return md; foreach (var m in td.Methods) if (MethodsAreEqual (method, m)) {