Skip to content

Commit 3c2a066

Browse files
jonathanpeppersjonathanpeppers
authored
[generator] emit faster overloads for non-virtual Formatted props (#1101)
Context: dotnet/maui#12130 Context: #994 Context: 79a8e1e Context: xamarin/monodroid@23f4212 When binding members which have parameter types or return types which are `java.lang.CharSequence`, the member is "overloaded" to replace `CharSequence` with `System.String`, and the "original" member has a `Formatted` *suffix*. For example, consider [`android.widget.TextView`][1], which has [`getText()`][1] and [`setText()`][2] methods which have parameter types and return types which are `java.lang.CharSequence`: // Java /* partial */ class TextView extends View { public CharSequence getText(); public final void setText(CharSequence text); } When bound, this results in *two* properties: // C# partial class TextView : View { public Java.Lang.ICharSequence? TextFormatted {get => …; set => …; } public string? Text {get => …; set => …; } } This is also done for methods; see also 79a8e1e. The "non-`Formatted` overload" works by creating `String` temporaries to invoke the `Formatted` overload: partial class TextView { public string? Text { get => TextFormatted?.ToString (); set { var jls = value == null ? null : new Java.Lang.String (value); TextFormatted = jls; jls?.Dispose (); } } } *Why* was this done? Because [C# 4.0][3] didn't allow interfaces to provide conversion operators. ([C# 8.0][4] would add support for interfaces to contain operators.) "Overloading" in this fashion made it easier to use `System.String` literals with `ICharSequence` members; compare: view.Text = "string literal"; // vs. view.TextFormatted = new Java.Lang.String("string literal"); // …and who would know how to do this? A problem with the this approach is performance: creating a new `Java.Lang.String` instance requires: 1. Creating the managed peer (the `Java.Lang.String` instance), 2. Creating the native peer (the `java.lang.String` instance), 3. And *registering the mapping* between (1) and (2) which feels a bit "silly" when we immediately dispose of the value. This is particularly noticeable with .NET MAUI apps. Consider the [angelru/CvSlowJittering][5] app, which uses XAML to set `Text` properties, which eventually hit `TextView.Text`. Profiling shows: 653.69ms (6.3%) mono.android!Android.Widget.TextView.set_Text(string) 198.05ms (1.9%) mono.android!Java.Lang.String..ctor(string) 121.57ms (1.2%) mono.android!Java.Lang.Object.Dispose() *6.3%* of scrolling time is spent in the `TextView.Text` property setter! *Partially optimize* this case: if the `*Formatted` member is (1) a property, and (2) *not* `virtual`, then we can directly call the Java setter method. This avoids the need to create a managed peer and to register a mapping between the peers: // New hotness partial class TextView { public string? Text { get => TextFormatted?.ToString (); // unchanged set { const string __id = "setText.(Ljava/lang/CharSequence;)V"; JniObjectReference native_value = JniEnvironment.Strings.NewString (value); try { JniArgumentValue* __args = stackalloc JniArgumentValue [1]; __args [0] = new JniArgumentValue (native_value); _members.InstanceMethods.InvokeNonvirtualVoidMethod (__id, this, __args); } finally { JniObjectReference.Dispose (ref native_value); } } } } [The result][6]? | Method | Mean | Error | StdDev | Allocated | |-------------------- |---------:|----------:|----------:|----------:| | Before SetFinalText | 6.632 us | 0.0101 us | 0.0079 us | 112 B | | After SetFinalText | 1.361 us | 0.0022 us | 0.0019 us | - | The `TextView.Text` property setter invocation time is reduced to 20% of the previous average invocation time. Note: We *cannot* do this "inlining" if the "`Formatted` overload" is [`virtual`][7], as that will result in broken semantics that make sense to *nobody*. TODO: Consider Optimizing the "`virtual` overload" scenario? This could be done by updating `Java.Lang.String`: partial interface ICharSequence { public static String FromJniObjectReference (ref JniObjectReference reference, JniObjectReferenceOptions options); } Then updating the "non-`Formatted` overload" to use `String.FromJniHandle()`: partial class TextView { public string? Text { get => TextFormatted?.ToString (); // unchanged set { JniObjectReference native_value = JniEnvironment.Strings.NewString (value); var java_value = Java.Lang.ICharSequence.FromJniObjectReference (ref native_value, JniObjectReferenceOptions.CopyAndDoNotRegister); TextFormatted = java_value; java_value?.Dispose (); } } } [0]: https://developer.android.com/reference/android/widget/TextView [1]: https://developer.android.com/reference/android/widget/TextView#getText() [2]: https://developer.android.com/reference/android/widget/TextView#setText(java.lang.CharSequence) [3]: https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-version-history#c-version-40 [4]: https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-version-history#c-version-80 [5]: https://github.com/angelru/CvSlowJittering [6]: https://github.com/jonathanpeppers/BenchmarkDotNet-Android/tree/Android.Widget.TextView [7]: #994 (comment)
1 parent e7d2edb commit 3c2a066

File tree

6 files changed

+163
-26
lines changed

6 files changed

+163
-26
lines changed

‎tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,59 @@ protected override CodeGenerationOptions CreateOptions ()
11521152
};
11531153
return options;
11541154
}
1155+
1156+
[Test]
1157+
public void StringPropertyOverride ([Values("true", "false")] string final)
1158+
{
1159+
var xml = @$"<api>
1160+
<package name='java.lang' jni-name='java/lang'>
1161+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
1162+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object'
1163+
final='true' name='String' static='false' visibility='public'>
1164+
</class>
1165+
</package>
1166+
<package name='android.widget' jni-name='android/widget'>
1167+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='TextView' static='false' visibility='public'>
1168+
<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'>
1169+
</method>
1170+
<method abstract='false' deprecated='not deprecated' final='{final}' name='setText' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public'>
1171+
<parameter name='text' type='java.lang.CharSequence'>
1172+
</parameter>
1173+
</method>
1174+
</class>
1175+
</package>
1176+
</api>";
1177+
1178+
var gens = ParseApiDefinition (xml);
1179+
var klass = gens.Single (g => g.Name == "TextView");
1180+
1181+
generator.Context.ContextTypes.Push (klass);
1182+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
1183+
generator.Context.ContextTypes.Pop ();
1184+
1185+
if (final == "true") {
1186+
Assert.True (writer.ToString ().Contains (
1187+
@"set {
1188+
const string __id = ""setText.(Ljava/lang/CharSequence;)V"";
1189+
global::Java.Interop.JniObjectReference text = global::Java.Interop.JniEnvironment.Strings.NewString (value);
1190+
try {
1191+
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
1192+
__args [0] = new JniArgumentValue (text);
1193+
_members.InstanceMethods.InvokeNonvirtualVoidMethod (__id, this, __args);
1194+
} finally {
1195+
global::Java.Interop.JniObjectReference.Dispose (ref text);
1196+
}
1197+
}
1198+
}"), $"was: `{writer}`");
1199+
} else {
1200+
Assert.True (writer.ToString ().Contains (
1201+
@"set {
1202+
var jls = value == null ? null : new global::Java.Lang.String (value);
1203+
TextFormatted = jls;
1204+
if (jls != null) jls.Dispose ();
1205+
}"), $"was: `{writer}`");
1206+
}
1207+
}
11551208
}
11561209

11571210
[TestFixture]
@@ -1236,6 +1289,58 @@ public void SupportedOSPlatformConstFields ()
12361289

12371290
StringAssert.Contains ("[global::System.Runtime.Versioning.SupportedOSPlatformAttribute (\"android30.0\")]", builder.ToString (), "Should contain SupportedOSPlatform!");
12381291
}
1292+
1293+
[Test]
1294+
public void StringPropertyOverride ([Values ("true", "false")] string final)
1295+
{
1296+
var xml = @$"<api>
1297+
<package name='java.lang' jni-name='java/lang'>
1298+
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
1299+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object'
1300+
final='true' name='String' static='false' visibility='public'>
1301+
</class>
1302+
</package>
1303+
<package name='android.widget' jni-name='android/widget'>
1304+
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='TextView' static='false' visibility='public'>
1305+
<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'>
1306+
</method>
1307+
<method abstract='false' deprecated='not deprecated' final='{final}' name='setText' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public'>
1308+
<parameter name='text' type='java.lang.CharSequence'>
1309+
</parameter>
1310+
</method>
1311+
</class>
1312+
</package>
1313+
</api>";
1314+
1315+
var gens = ParseApiDefinition (xml);
1316+
var klass = gens.Single (g => g.Name == "TextView");
1317+
1318+
generator.Context.ContextTypes.Push (klass);
1319+
generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
1320+
generator.Context.ContextTypes.Pop ();
1321+
1322+
if (final == "true") {
1323+
Assert.True (writer.ToString ().Contains (
1324+
@"set {
1325+
const string __id = ""setText.(Ljava/lang/CharSequence;)V"";
1326+
global::Java.Interop.JniObjectReference native_text = global::Java.Interop.JniEnvironment.Strings.NewString (value);
1327+
try {
1328+
JniArgumentValue* __args = stackalloc JniArgumentValue [1];
1329+
__args [0] = new JniArgumentValue (native_text);
1330+
_members.InstanceMethods.InvokeNonvirtualVoidMethod (__id, this, __args);
1331+
} finally {
1332+
global::Java.Interop.JniObjectReference.Dispose (ref native_text);
1333+
}
1334+
}"), $"was: `{writer}`");
1335+
} else {
1336+
Assert.True (writer.ToString ().Contains (
1337+
@"set {
1338+
var jls = value == null ? null : new global::Java.Lang.String (value);
1339+
TextFormatted = jls;
1340+
if (jls != null) jls.Dispose ();
1341+
}"), $"was: `{writer}`");
1342+
}
1343+
}
12391344
}
12401345

12411346
[TestFixture]

‎tools/generator/SourceWriters/BoundClass.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ void AddProperty (ClassGen klass, Property property, CodeGenerationOptions opt)
346346
Properties.Add (bound_property);
347347

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

352352
void AddAbstractPropertyDeclaration (ClassGen klass, Property property, CodeGenerationOptions opt)
@@ -361,10 +361,11 @@ void AddAbstractPropertyDeclaration (ClassGen klass, Property property, CodeGene
361361
}
362362
}
363363

364-
Properties.Add (new BoundAbstractProperty (klass, property, opt));
364+
var bound_property = new BoundAbstractProperty (klass, property, opt);
365+
Properties.Add (bound_property);
365366

366367
if (property.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal))
367-
Properties.Add (new BoundPropertyStringVariant (property, opt));
368+
Properties.Add (new BoundPropertyStringVariant (property, opt, bound_property));
368369
}
369370

370371
void AddNestedTypes (ClassGen klass, CodeGenerationOptions opt, CodeGeneratorContext context, GenerationInfo genInfo)

‎tools/generator/SourceWriters/BoundInterface.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,13 @@ void AddProperties (InterfaceGen iface, CodeGenerationOptions opt)
215215
Properties.Add (bound_property);
216216

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

223223
if (prop.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
224-
Properties.Add (new BoundPropertyStringVariant (prop, opt));
224+
Properties.Add (new BoundPropertyStringVariant (prop, opt, bound_property));
225225
}
226226
}
227227
}

‎tools/generator/SourceWriters/BoundPropertyStringVariant.cs‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,15 @@ namespace generator.SourceWriters
1313
// an overload with type 'string' as a convenience for the user.
1414
public class BoundPropertyStringVariant : PropertyWriter
1515
{
16-
public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt)
16+
public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, BoundAbstractProperty original)
17+
: this (property, opt, original.IsVirtual)
18+
{ }
19+
20+
public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, BoundProperty original)
21+
: this(property, opt, original.IsVirtual)
22+
{ }
23+
24+
private BoundPropertyStringVariant (Property property, CodeGenerationOptions opt, bool isOriginalVirtual)
1725
{
1826
var is_array = property.Getter.RetVal.IsArray;
1927

@@ -46,9 +54,27 @@ public BoundPropertyStringVariant (Property property, CodeGenerationOptions opt)
4654
SetBody.Add ($"{property.AdjustedName} = jlsa;");
4755
SetBody.Add ($"foreach (var jls in jlsa) if (jls != null) jls.Dispose ();");
4856
} else {
49-
SetBody.Add ($"var jls = value == null ? null : new global::Java.Lang.String (value);");
50-
SetBody.Add ($"{property.AdjustedName} = jls;");
51-
SetBody.Add ($"if (jls != null) jls.Dispose ();");
57+
if (isOriginalVirtual) {
58+
SetBody.Add ($"var jls = value == null ? null : new global::Java.Lang.String (value);");
59+
SetBody.Add ($"{property.AdjustedName} = jls;");
60+
SetBody.Add ($"if (jls != null) jls.Dispose ();");
61+
} else {
62+
// Emit a "fast" path if the property is non-virtual
63+
IsUnsafe = true;
64+
var method = property.Setter;
65+
var parameter = method.Parameters [0];
66+
67+
SetBody.Add ($"const string __id = \"{method.JavaName}.{method.JniSignature}\";");
68+
SetBody.Add ($"global::Java.Interop.JniObjectReference {parameter.ToNative (opt)} = global::Java.Interop.JniEnvironment.Strings.NewString (value);");
69+
70+
SetBody.Add ("try {");
71+
72+
SourceWriterExtensions.AddMethodBodyTryBlock (SetBody, method, opt);
73+
74+
SetBody.Add ("} finally {");
75+
SetBody.Add ($"\tglobal::Java.Interop.JniObjectReference.Dispose (ref {parameter.ToNative (opt)});");
76+
SetBody.Add ("}");
77+
}
5278
}
5379
}
5480
}

‎tools/generator/SourceWriters/ClassInvokerClass.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void AddPropertyInvokers (ClassGen klass, IEnumerable<Property> properties, Hash
9696
Properties.Add (bound_property);
9797

9898
if (prop.Type.StartsWith ("Java.Lang.ICharSequence", StringComparison.Ordinal) && !bound_property.IsOverride)
99-
Properties.Add (new BoundPropertyStringVariant (prop, opt));
99+
Properties.Add (new BoundPropertyStringVariant (prop, opt, bound_property));
100100
}
101101
}
102102

‎tools/generator/SourceWriters/Extensions/SourceWriterExtensions.cs‎

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,27 @@ public static void AddMethodBody (List<string> body, Method method, CodeGenerati
238238

239239
body.Add ("try {");
240240

241+
AddMethodBodyTryBlock (body, method, opt);
242+
243+
if (method.IsCompatVirtualMethod) {
244+
body.Add ("}");
245+
body.Add ("catch (Java.Lang.NoSuchMethodError) {");
246+
body.Add (" throw new Java.Lang.AbstractMethodError (__id);");
247+
}
248+
249+
body.Add ("} finally {");
250+
251+
foreach (string cleanup in method.Parameters.GetCallCleanup (opt))
252+
body.Add ("\t" + cleanup);
253+
254+
foreach (var p in method.Parameters.Where (para => para.ShouldGenerateKeepAlive ()))
255+
body.Add ($"\tglobal::System.GC.KeepAlive ({opt.GetSafeIdentifier (p.Name)});");
256+
257+
body.Add ("}");
258+
}
259+
260+
public static void AddMethodBodyTryBlock (List<string> body, Method method, CodeGenerationOptions opt)
261+
{
241262
AddParameterListCallArgs (body, method.Parameters, opt, false);
242263

243264
var invokeType = JavaInteropCodeGenerator.GetInvokeType (method.RetVal.CallMethodPrefix);
@@ -265,22 +286,6 @@ public static void AddMethodBody (List<string> body, Method method, CodeGenerati
265286
}
266287
body.Add ($"\treturn {method.RetVal.ReturnCast}{method.RetVal.FromNative (opt, r, true) + opt.GetNullForgiveness (method.RetVal)};");
267288
}
268-
269-
if (method.IsCompatVirtualMethod) {
270-
body.Add ("}");
271-
body.Add ("catch (Java.Lang.NoSuchMethodError) {");
272-
body.Add (" throw new Java.Lang.AbstractMethodError (__id);");
273-
}
274-
275-
body.Add ("} finally {");
276-
277-
foreach (string cleanup in method.Parameters.GetCallCleanup (opt))
278-
body.Add ("\t" + cleanup);
279-
280-
foreach (var p in method.Parameters.Where (para => para.ShouldGenerateKeepAlive ()))
281-
body.Add ($"\tglobal::System.GC.KeepAlive ({opt.GetSafeIdentifier (p.Name)});");
282-
283-
body.Add ("}");
284289
}
285290

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

0 commit comments

Comments
 (0)