Skip to content

Commit a91ae7f

Browse files
jonathanpeppersjonathanpeppers
authored
[generator] improve *Implementor constructors (#1105)
Context: dotnet/maui#12130 Context: https://github.com/angelru/CvSlowJittering Profiling a .NET MAUI customer sample while scrolling on a Pixel 5, I see ~2.2% of the time spent in the `IOnFocusChangeListenerImplementor` constructor, due to [subscription to the `View.FocusChange` event][0]: (2.2%) mono.android!Android.Views.View.IOnFocusChangeListenerImplementor..ctor() MAUI subscribes to [`Android.Views.View.FocusChange` event][1] for every view placed on the screen, which happens while scrolling in this sample. Reviewing, the generated code for the `IOnFocusChangeListenerImplementor` constructor, we see it still uses outdated `JNIEnv` APIs: public IOnFocusChangeListenerImplementor () : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/android/view/View_OnFocusChangeListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef) { global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V"); } Which we can change to use the newer/faster Java.Interop APIs: public unsafe IOnFocusChangeListenerImplementor () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer) { const string __id = "()V"; if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero) return; var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null); SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef); JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null); } These are better because the equivalent call to `JNIEnv.FindClass()` is cached, among other things. After these changes, I now see: (0.81%) mono.android!Android.Views.View.IOnFocusChangeListenerImplementor..ctor() This should improve the performance of all C# events that wrap Java listeners, including and all .NET MAUI views on Android. Additionally, stop emitting the `[Register]` attribute for `*Implementor` classes: [global::Android.Runtime.Register ("mono/android/view/View_OnFocusChangeListenerImplementor")] partial class IOnFocusChangeListenerImplementor {/* … */} The `[Register]` attribute is not needed, because `*Implementor` classes are generated internal implementation details. [0]: https://github.com/dotnet/maui/blob/2041476e78452891029f4d2bd806c45be42f4878/src/Core/src/Handlers/View/ViewHandler.Android.cs#L14 [1]: https://learn.microsoft.com/en-us/dotnet/api/android.views.view.focuschange?view=xamarin-android-sdk-13
1 parent 7d42864 commit a91ae7f

File tree

5 files changed

+38
-18
lines changed

5 files changed

+38
-18
lines changed

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/Common/WriteDuplicateInterfaceEventArgs.txt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,18 @@ public partial class AnimationEndEventArgs : global::System.EventArgs {
157157

158158
}
159159

160-
[global::Android.Runtime.Register ("mono/java/code/AnimatorListenerImplementor")]
161160
internal sealed partial class AnimatorListenerImplementor : global::Java.Lang.Object, AnimatorListener {
162161

163162
object sender;
164163

165-
public AnimatorListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/java/code/AnimatorListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
164+
public unsafe AnimatorListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
166165
{
167-
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
166+
const string __id = "()V";
167+
if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
168+
return;
169+
var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
170+
SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef);
171+
JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null);
168172
this.sender = sender;
169173
}
170174

‎tests/generator-Tests/Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteDuplicateInterfaceEventArgs.txt‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,18 @@ public partial class AnimationEndEventArgs : global::System.EventArgs {
4747

4848
}
4949

50-
[global::Android.Runtime.Register ("mono/java/code/AnimatorListenerImplementor")]
5150
internal sealed partial class AnimatorListenerImplementor : global::Java.Lang.Object, AnimatorListener {
5251

5352
object sender;
5453

55-
public AnimatorListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/java/code/AnimatorListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
54+
public unsafe AnimatorListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
5655
{
57-
global::Android.Runtime.JNIEnv.FinishCreateInstance (this.PeerReference, "()V");
56+
const string __id = "()V";
57+
if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
58+
return;
59+
var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
60+
SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef);
61+
JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null);
5862
this.sender = sender;
5963
}
6064

‎tests/generator-Tests/expected.xaji/Core_Jar2Xml/Android.Views.View.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,15 @@ public unsafe void OnClick (global::Android.Views.View v)
111111

112112
}
113113

114-
[global::Android.Runtime.Register ("mono/android/view/View_OnClickListenerImplementor")]
115114
internal sealed partial class IOnClickListenerImplementor : global::Java.Lang.Object, IOnClickListener {
116-
public IOnClickListenerImplementor () : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/android/view/View_OnClickListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
115+
public unsafe IOnClickListenerImplementor () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
117116
{
118-
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
117+
const string __id = "()V";
118+
if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
119+
return;
120+
var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
121+
SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef);
122+
JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null);
119123
}
120124

121125
#pragma warning disable 0649

‎tests/generator-Tests/expected.xaji/GenericArguments/Com.Google.Android.Exoplayer.Drm.IExoMediaDrm.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,14 +162,18 @@ public byte[] P4 {
162162

163163
}
164164

165-
[global::Android.Runtime.Register ("mono/com/google/android/exoplayer/drm/ExoMediaDrm_OnEventListenerImplementor")]
166165
internal sealed partial class IExoMediaDrmOnEventListenerImplementor : global::Java.Lang.Object, IExoMediaDrmOnEventListener {
167166

168167
object sender;
169168

170-
public IExoMediaDrmOnEventListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/com/google/android/exoplayer/drm/ExoMediaDrm_OnEventListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
169+
public unsafe IExoMediaDrmOnEventListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
171170
{
172-
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
171+
const string __id = "()V";
172+
if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)
173+
return;
174+
var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);
175+
SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef);
176+
JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null);
173177
this.sender = sender;
174178
}
175179

‎tools/generator/SourceWriters/InterfaceEventHandlerImplClass.cs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,14 @@ public InterfaceEventHandlerImplClass (InterfaceGen iface, CodeGenerationOptions
2222
IsSealed = true;
2323
IsPartial = true;
2424

25-
Attributes.Add (new RegisterAttr (jni_class, additionalProperties: iface.AdditionalAttributeString ()) { UseGlobal = true });
26-
2725
if (iface.NeedsSender)
2826
Fields.Add (new FieldWriter { Name = "sender", Type = TypeReferenceWriter.Object });
2927

30-
AddConstructor (iface, jni_class, opt);
28+
AddConstructor (iface);
3129
AddMethods (iface, opt);
3230
}
3331

34-
void AddConstructor (InterfaceGen iface, string jniClass, CodeGenerationOptions opt)
32+
void AddConstructor (InterfaceGen iface)
3533
{
3634
var ctor = new ConstructorWriter {
3735
Name = iface.Name + "Implementor",
@@ -41,9 +39,15 @@ void AddConstructor (InterfaceGen iface, string jniClass, CodeGenerationOptions
4139
if (iface.NeedsSender)
4240
ctor.Parameters.Add (new MethodParameterWriter ("sender", TypeReferenceWriter.Object));
4341

44-
ctor.BaseCall = $"base (global::Android.Runtime.JNIEnv.StartCreateInstance (\"{jniClass}\", \"()V\"), JniHandleOwnership.TransferLocalRef)";
42+
ctor.IsUnsafe = true;
43+
ctor.BaseCall = "base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)";
4544

46-
ctor.Body.Add ($"global::Android.Runtime.JNIEnv.FinishCreateInstance ({iface.GetObjectHandleProperty (opt, "this")}, \"()V\");");
45+
ctor.Body.Add ("const string __id = \"()V\";");
46+
ctor.Body.Add ("if (((global::Java.Lang.Object) this).Handle != IntPtr.Zero)");
47+
ctor.Body.Add ("\treturn;");
48+
ctor.Body.Add ("var h = JniPeerMembers.InstanceMethods.StartCreateInstance (__id, ((object) this).GetType (), null);");
49+
ctor.Body.Add ("SetHandle (h.Handle, JniHandleOwnership.TransferLocalRef);");
50+
ctor.Body.Add ("JniPeerMembers.InstanceMethods.FinishCreateInstance (__id, this, null);");
4751

4852
if (iface.NeedsSender)
4953
ctor.Body.Add ("this.sender = sender;");

0 commit comments

Comments
 (0)