Skip to content
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

Jump to bottom

[generator] emit faster/better overloads for String #1101

Merged
merged 6 commits into from
Apr 25, 2023
Merged

[generator] emit faster/better overloads for String #1101

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b02f76f
[generator] emit faster/better overloads for String
jonathanpeppers Apr 20, 2023
15671db
Diff of generator output
jonathanpeppers Apr 20, 2023
81b23d8
Write "new" style unit tests
jonathanpeppers Apr 20, 2023
c5b92b3
Special case virtual and non-virtual
jonathanpeppers Apr 24, 2023
c1ff177
First parameter to `Java.Lang.String.ctor()` has to be IntPtr
jonathanpeppers Apr 24, 2023
4403f87
Put the `virtual` case back the way it was
jonathanpeppers Apr 25, 2023
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
105 changes: 105 additions & 0 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,59 @@ protected override CodeGenerationOptions CreateOptions ()
};
return options;
}

[Test]
public void StringPropertyOverride ([Values("true", "false")] string final)
{
var xml = @$"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object'
final='true' name='String' static='false' visibility='public'>
</class>
</package>
<package name='android.widget' jni-name='android/widget'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='TextView' static='false' visibility='public'>
<method abstract='false' deprecated='not deprecated' final='{final}' name='getText' bridge='false' native='false' return='java.lang.CharSequence' static='false' synchronized='false' synthetic='false' visibility='public'>
</method>
<method abstract='false' deprecated='not deprecated' final='{final}' name='setText' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public'>
<parameter name='text' type='java.lang.CharSequence'>
</parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var klass = gens.Single (g => g.Name == "TextView");

generator.Context.ContextTypes.Push (klass);
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

if (final == "true") {
Assert.True (writer.ToString ().Contains (
@"set {
const string __id = ""setText.(Ljava/lang/CharSequence;)V"";
global::Java.Interop.JniObjectReference text = global::Java.Interop.JniEnvironment.Strings.NewString (value);
try {
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
__args [0] = new JniArgumentValue (text);
_members.InstanceMethods.InvokeNonvirtualVoidMethod (__id, this, __args);
} finally {
global::Java.Interop.JniObjectReference.Dispose (ref text);
}
}
}"), $"was: `{writer}`");
} else {
Assert.True (writer.ToString ().Contains (
@"set {
var jls = value == null ? null : new global::Java.Lang.String (JNIEnv.NewString (value), JniHandleOwnership.TransferLocalRef | JniHandleOwnership.DoNotRegister);
TextFormatted = jls;
if (jls != null) jls.Dispose ();
}"), $"was: `{writer}`");
}
}
}

[TestFixture]
Expand Down Expand Up @@ -1236,6 +1289,58 @@ public void SupportedOSPlatformConstFields ()

StringAssert.Contains ("[global::System.Runtime.Versioning.SupportedOSPlatformAttribute (\"android30.0\")]", builder.ToString (), "Should contain SupportedOSPlatform!");
}

[Test]
public void StringPropertyOverride ([Values ("true", "false")] string final)
{
var xml = @$"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object'
final='true' name='String' static='false' visibility='public'>
</class>
</package>
<package name='android.widget' jni-name='android/widget'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='TextView' static='false' visibility='public'>
<method abstract='false' deprecated='not deprecated' final='{final}' name='getText' bridge='false' native='false' return='java.lang.CharSequence' static='false' synchronized='false' synthetic='false' visibility='public'>
</method>
<method abstract='false' deprecated='not deprecated' final='{final}' name='setText' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public'>
<parameter name='text' type='java.lang.CharSequence'>
</parameter>
</method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var klass = gens.Single (g => g.Name == "TextView");

generator.Context.ContextTypes.Push (klass);
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
generator.Context.ContextTypes.Pop ();

if (final == "true") {
Assert.True (writer.ToString ().Contains (
@"set {
const string __id = ""setText.(Ljava/lang/CharSequence;)V"";
global::Java.Interop.JniObjectReference native_text = global::Java.Interop.JniEnvironment.Strings.NewString (value);
try {
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
__args [0] = new JniArgumentValue (native_text);
_members.InstanceMethods.InvokeNonvirtualVoidMethod (__id, this, __args);
} finally {
global::Java.Interop.JniObjectReference.Dispose (ref native_text);
}
}"), $"was: `{writer}`");
} else {
Assert.True (writer.ToString ().Contains (
@"set {
var jls = value == null ? null : new global::Java.Lang.String (JNIEnv.NewString (value), JniHandleOwnership.TransferLocalRef | JniHandleOwnership.DoNotRegister);
TextFormatted = jls;
if (jls != null) jls.Dispose ();
}"), $"was: `{writer}`");
}
}
}

[TestFixture]
Expand Down
7 changes: 4 additions & 3 deletions tools/generator/SourceWriters/BoundClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ void AddProperty (ClassGen klass, Property property, CodeGenerationOptions opt)
Properties.Add (bound_property);

if (property.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
Properties.Add (new BoundPropertyStringVariant (property, opt));
Properties.Add (new BoundPropertyStringVariant (property, opt, bound_property));
}

void AddAbstractPropertyDeclaration (ClassGen klass, Property property, CodeGenerationOptions opt)
Expand All @@ -361,10 +361,11 @@ void AddAbstractPropertyDeclaration (ClassGen klass, Property property, CodeGene
}
}

Properties.Add (new BoundAbstractProperty (klass, property, opt));
var bound_property = new BoundAbstractProperty (klass, property, opt);
Properties.Add (bound_property);

if (property.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal))
Properties.Add (new BoundPropertyStringVariant (property, opt));
Properties.Add (new BoundPropertyStringVariant (property, opt, bound_property));
}

void AddNestedTypes (ClassGen klass, CodeGenerationOptions opt, CodeGeneratorContext context, GenerationInfo genInfo)
Expand Down
4 changes: 2 additions & 2 deletions tools/generator/SourceWriters/BoundInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,13 @@ void AddProperties (InterfaceGen iface, CodeGenerationOptions opt)
Properties.Add (bound_property);

if (prop.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
Properties.Add (new BoundPropertyStringVariant (prop, opt));
Properties.Add (new BoundPropertyStringVariant (prop, opt, bound_property));
} else {
var bound_property = new BoundProperty (iface, prop, opt, true, false);
Properties.Add (bound_property);

if (prop.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
Properties.Add (new BoundPropertyStringVariant (prop, opt));
Properties.Add (new BoundPropertyStringVariant (prop, opt, bound_property));
}
}
}
Expand Down
34 changes: 30 additions & 4 deletions tools/generator/SourceWriters/BoundPropertyStringVariant.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,15 @@ namespace generator.SourceWriters
// an overload with type 'string' as a convenience for the user.
public class BoundPropertyStringVariant : PropertyWriter
{
public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt)
public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, BoundAbstractProperty original)
: this (property, opt, original.IsVirtual)
{ }

public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, BoundProperty original)
: this(property, opt, original.IsVirtual)
{ }

private BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, bool isOriginalVirtual)
{
var is_array = property.Getter.RetVal.IsArray;

Expand Down Expand Up @@ -46,9 +54,27 @@ public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt)
SetBody.Add ($"{property.AdjustedName} = jlsa;");
SetBody.Add ($"foreach (var jls in jlsa) if (jls != null) jls.Dispose ();");
} else {
SetBody.Add ($"var jls = value == null ? null : new global::Java.Lang.String (value);");
SetBody.Add ($"{property.AdjustedName} = jls;");
SetBody.Add ($"if (jls != null) jls.Dispose ();");
if (isOriginalVirtual) {
SetBody.Add ($"var jls = value == null ? null : new global::Java.Lang.String (JNIEnv.NewString (value), JniHandleOwnership.TransferLocalRef | JniHandleOwnership.DoNotRegister);");
SetBody.Add ($"{property.AdjustedName} = jls;");
SetBody.Add ($"if (jls != null) jls.Dispose ();");
} else {
// Emit a "fast" path if the property is non-virtual
IsUnsafe = true;
var method = property.Setter;
var parameter = method.Parameters [0];

SetBody.Add ($"const string __id = \"{method.JavaName}.{method.JniSignature}\";");
SetBody.Add ($"global::Java.Interop.JniObjectReference {parameter.ToNative (opt)} = global::Java.Interop.JniEnvironment.Strings.NewString (value);");

SetBody.Add ("try {");

SourceWriterExtensions.AddMethodBodyTryBlock (SetBody, method, opt);

SetBody.Add ("} finally {");
SetBody.Add ($"\tglobal::Java.Interop.JniObjectReference.Dispose (ref {parameter.ToNative (opt)});");
SetBody.Add ("}");
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tools/generator/SourceWriters/ClassInvokerClass.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ void AddPropertyInvokers (ClassGen klass, IEnumerable<Property> properties, Hash
Properties.Add (bound_property);

if (prop.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
Properties.Add (new BoundPropertyStringVariant (prop, opt));
Properties.Add (new BoundPropertyStringVariant (prop, opt, bound_property));
}
}

Expand Down
37 changes: 21 additions & 16 deletions tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,27 @@ public static void AddMethodBody (List<string> body, Method method, CodeGenerati

body.Add ("try {");

AddMethodBodyTryBlock (body, method, opt);

if (method.IsCompatVirtualMethod) {
body.Add ("}");
body.Add ("catch (Java.Lang.NoSuchMethodError) {");
body.Add (" throw new Java.Lang.AbstractMethodError (__id);");
}

body.Add ("} finally {");

foreach (string cleanup in method.Parameters.GetCallCleanup (opt))
body.Add ("\t" + cleanup);

foreach (var p in method.Parameters.Where (para => para.ShouldGenerateKeepAlive ()))
body.Add ($"\tglobal::System.GC.KeepAlive ({opt.GetSafeIdentifier (p.Name)});");

body.Add ("}");
}

public static void AddMethodBodyTryBlock (List<string> body, Method method, CodeGenerationOptions opt)
{
AddParameterListCallArgs (body, method.Parameters, opt, false);

var invokeType = JavaInteropCodeGenerator.GetInvokeType (method.RetVal.CallMethodPrefix);
Expand Down Expand Up @@ -265,22 +286,6 @@ public static void AddMethodBody (List<string> body, Method method, CodeGenerati
}
body.Add ($"\treturn {method.RetVal.ReturnCast}{method.RetVal.FromNative (opt, r, true) + opt.GetNullForgiveness (method.RetVal)};");
}

if (method.IsCompatVirtualMethod) {
body.Add ("}");
body.Add ("catch (Java.Lang.NoSuchMethodError) {");
body.Add (" throw new Java.Lang.AbstractMethodError (__id);");
}

body.Add ("} finally {");

foreach (string cleanup in method.Parameters.GetCallCleanup (opt))
body.Add ("\t" + cleanup);

foreach (var p in method.Parameters.Where (para => para.ShouldGenerateKeepAlive ()))
body.Add ($"\tglobal::System.GC.KeepAlive ({opt.GetSafeIdentifier (p.Name)});");

body.Add ("}");
}

public static void AddParameterListCallArgs (List<string> body, ParameterList parameters, CodeGenerationOptions opt, bool invoker)
Expand Down