Skip to content

[jnimarshalmethod-gen] Avoid creating duplicate marshal methods #385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 23, 2018
Merged

[jnimarshalmethod-gen] Avoid creating duplicate marshal methods #385

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68a67c9
[jnimarshalmethod-gen] Avoid creating duplicate marshal methods
radekdoulik Oct 16, 2018
38794bf
[jnimarshalmethod-gen] Use HasOverrides when sorting methods
radekdoulik Oct 22, 2018
7a7341f
Use TryGetValue instead of ContainsKey
radekdoulik Oct 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions tools/jnimarshalmethod-gen/App.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ void ProcessAssemblies (List<string> assemblies)
ad = AssemblyDefinition.ReadAssembly (assembly, readWriteParametersNoSymbols);
resolver.AddToCache (ad);
}

Extensions.MethodMap.Clear ();
}

foreach (var assembly in assemblies) {
Expand Down Expand Up @@ -221,6 +223,42 @@ static TypeBuilder GetTypeBuilder (ModuleBuilder mb, Type type)
return tb;
}

class MethodsComparer : IComparer<MethodInfo>
{
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)
{
if (a.DeclaringType != type)
return 1;

var atd = td.GetMethodDefinition (a);
if (atd == null)
return 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);
}
}

static HashSet<string> addedMethods = new HashSet<string> ();

void CreateMarshalMethodAssembly (string path)
{
var assembly = Assembly.LoadFile (Path.GetFullPath (path));
Expand Down Expand Up @@ -299,7 +337,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 (type, td));

addedMethods.Clear ();

foreach (var method in methods) {
// TODO: Constructors
var export = method.GetCustomAttribute<JavaCallableAttribute> ();
string signature = null;
Expand Down Expand Up @@ -328,6 +372,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 );
Expand All @@ -349,6 +396,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);
Expand Down Expand Up @@ -463,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 ('+', '/');
Copy link
Contributor

@dellis1972 dellis1972 Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is this can now return null? If so do we know where this method is called? Do those calling points need to be updated to handle a null?

Copy link
Member Author

@radekdoulik radekdoulik Oct 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated path with null goes only thru CompareTypes where it is used only for comparison.

}

static bool CompareTypes (Type reflectionType, TypeReference cecilType)
Expand Down Expand Up @@ -503,11 +552,19 @@ static bool MethodsAreEqual (MethodInfo methodInfo, MethodDefinition methodDefin
return true;
}

internal static Dictionary<MethodInfo, MethodDefinition> MethodMap = new Dictionary<MethodInfo, MethodDefinition> ();

public static MethodDefinition GetMethodDefinition (this TypeDefinition td, MethodInfo method)
{
if (MethodMap.TryGetValue (method, out var md))
return md;

foreach (var m in td.Methods)
if (MethodsAreEqual (method, m))
if (MethodsAreEqual (method, m)) {
MethodMap [method] = m;

return m;
}

return null;
}
Expand Down