Skip to content

Commit

Permalink
[generator] improve *Implementor constructors
Browse files Browse the repository at this point in the history 
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:

    (2.2%) mono.android!Android.Views.View.IOnFocusChangeListenerImplementor..ctor()

https://github.com/dotnet/maui/blob/2041476e78452891029f4d2bd806c45be42f4878/src/Core/src/Handlers/View/ViewHandler.Android.cs#L14

MAUI subscribes to `Android.Views.View.FocusChange` for every view
placed on the screen -- which happens while scrolling in this sample.

Reviewing, the generated code for this constructor still uses the
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 instead see:

    (0.81%) mono.android!Android.Views.View.IOnFocusChangeListenerImplementor..ctor()

This should improve the performance of all C# events that wrap Java
listeners -- and all .NET MAUI views on Android.

These changes also stop emitting the `[Register]` attribute for
`*Implementor` classes:

    [global::Android.Runtime.Register ("mono/android/view/View_OnFocusChangeListenerImplementor")]

Which is not needed, because `*Implementor` classes are generated,
internal implementation details.
  • Loading branch information
@jonathanpeppers
jonathanpeppers committed May 4, 2023
1 parent 3c2a066 commit 41d79eb
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 18 deletions.
10 changes: 7 additions & 3 deletions ...Tests/Unit-Tests/CodeGeneratorExpectedResults/Common/WriteDuplicateInterfaceEventArgs.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,18 @@ public partial class AnimationEndEventArgs : global::System.EventArgs {

}

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

object sender;

public AnimatorListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/java/code/AnimatorListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
public unsafe AnimatorListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
{
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
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);
this.sender = sender;
}

Expand Down
10 changes: 7 additions & 3 deletions ...Unit-Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteDuplicateInterfaceEventArgs.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,18 @@ public partial class AnimationEndEventArgs : global::System.EventArgs {

}

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

object sender;

public AnimatorListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/java/code/AnimatorListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
public unsafe AnimatorListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
{
global::Android.Runtime.JNIEnv.FinishCreateInstance (this.PeerReference, "()V");
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);
this.sender = sender;
}

Expand Down
10 changes: 7 additions & 3 deletions tests/generator-Tests/expected.xaji/Core_Jar2Xml/Android.Views.View.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,15 @@ public unsafe void OnClick (global::Android.Views.View v)

}

[global::Android.Runtime.Register ("mono/android/view/View_OnClickListenerImplementor")]
internal sealed partial class IOnClickListenerImplementor : global::Java.Lang.Object, IOnClickListener {
public IOnClickListenerImplementor () : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/android/view/View_OnClickListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
public unsafe IOnClickListenerImplementor () : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
{
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
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);
}

#pragma warning disable 0649
Expand Down
10 changes: 7 additions & 3 deletions ...tor-Tests/expected.xaji/GenericArguments/Com.Google.Android.Exoplayer.Drm.IExoMediaDrm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,18 @@ public byte[] P4 {

}

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

object sender;

public IExoMediaDrmOnEventListenerImplementor (object sender) : base (global::Android.Runtime.JNIEnv.StartCreateInstance ("mono/com/google/android/exoplayer/drm/ExoMediaDrm_OnEventListenerImplementor", "()V"), JniHandleOwnership.TransferLocalRef)
public unsafe IExoMediaDrmOnEventListenerImplementor (object sender) : base (IntPtr.Zero, JniHandleOwnership.DoNotTransfer)
{
global::Android.Runtime.JNIEnv.FinishCreateInstance (((global::Java.Lang.Object) this).Handle, "()V");
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);
this.sender = sender;
}

Expand Down
16 changes: 10 additions & 6 deletions tools/generator/SourceWriters/InterfaceEventHandlerImplClass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,14 @@ public InterfaceEventHandlerImplClass (InterfaceGen iface, CodeGenerationOptions
IsSealed = true;
IsPartial = true;

Attributes.Add (new RegisterAttr (jni_class, additionalProperties: iface.AdditionalAttributeString ()) { UseGlobal = true });

if (iface.NeedsSender)
Fields.Add (new FieldWriter { Name = "sender", Type = TypeReferenceWriter.Object });

AddConstructor (iface, jni_class, opt);
AddConstructor (iface);
AddMethods (iface, opt);
}

void AddConstructor (InterfaceGen iface, string jniClass, CodeGenerationOptions opt)
void AddConstructor (InterfaceGen iface)
{
var ctor = new ConstructorWriter {
Name = iface.Name + "Implementor",
Expand All @@ -41,9 +39,15 @@ void AddConstructor (InterfaceGen iface, string jniClass, CodeGenerationOptions
if (iface.NeedsSender)
ctor.Parameters.Add (new MethodParameterWriter ("sender", TypeReferenceWriter.Object));

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

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

if (iface.NeedsSender)
ctor.Body.Add ("this.sender = sender;");
Expand Down

0 comments on commit 41d79eb

Please sign in to comment.