-
Notifications
You must be signed in to change notification settings - Fork 64
[generator] Add support for peerConstructorPartialMethod #1087
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.
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
Conversation
|
The compilation overhead of |
Context: dotnet/java-interop#1087 Does It Build™?
148a553 to
054dd3a
Compare
Context: dotnet/java-interop#1087 Does It Build™?
|
dotnet/android#7886 looks "green enough" for this to be merge-worthy. This change doesn't appear to break anything. |
|
|
||
| public virtual void Write (CodeWriter writer) | ||
| { | ||
| WritePartialMethodDeclaration (writer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way of implementation breaks the "spirit" of the SourceWriter model. That is, it is using a MethodWriter to write 2 methods.
This should instead be implemented as a new ConstructorPartialMethod : MethodWriter class in generator.csproj that gets added to the ClassWriter model when the JavaLangObjectConstructor is created.
Should be something like:
// Add required constructor for all JLO inheriting classes
if (klass.FullName != "Java.Lang.Object" && klass.InheritsObject) {
Methods.Add (new ConstructorPartialMethod (klass, opt));
Constructors.Add (new JavaLangObjectConstructor (klass, opt));
}The only change we should need for MethodWriter is if it doesn't support partial methods in general.
Context: dotnet#1085 Context: dotnet/runtime#82121 Some Java objects are *big*, e.g. [`Bitmap`][0] instances, but as far as MonoVM is concerned, the `Bitmap` instances are *tiny*: a few pointers, and that's it. MonoVM doesn't know that it could be referencing several MB of data in the Java VM. MonoVM is gaining support for [`GC.AddMemoryPressure()`][1] and [`GC.RemoveMemoryPressure()`][2], which potentially allows for the parent of all cross-VM GC integrations: using the `GC` methods to inform MonoVM of how much non-managed memory has been allocated. This could allow MonoVM to collect more frequently when total process memory is low. How should we call `GC.AddMemoryPressure()` and `GC.RemoveMemoryPressure()`? `GC.RemoveMemoryPressure()` is straightforward: a class can override `Java.Lang.Object.Dispose(bool)`. `GC.AddMemoryPressure()` is the problem: where and how can it be called from a class binding? This is trickier, as there was no way to have custom code called by the bound type. Instead, various forms of "hacky workarounds" are often employed, e.g. copying `generator`-emitted content into a `partial` class, then using `metadata` to *prevent* `generator` from binding those members. It's all around fugly. Fortunately C# has a solution: [`partial` methods][3]! Add support for a `peerConstructorPartialMethod` metadata entry, applicable to `<class/>` elements, which contains the name of a `partial` method to both declare and invoke from the "peer constructor": <attr path="//class[@name='Bitmap']" name="peerConstructorPartialMethod" >_OnBitmapCreated</attr> This will alter our existing "peer constructor" generation code, a'la: partial class Bitmap : Java.Lang.Object { internal Bitmap (IntPtr h, JniHandleOwnership t) : base (h, t) { } } to instead become: partial class Bitmap : Java.Lang.Object { partial void _OnBitmapCreated (); internal Bitmap (IntPtr h, JniHandleOwnership t) : base (h, t) { _OnBitmapCreated (); } } This allows a hand-written `partial class Bitmap` to do: // Hand-written code partial class Bitmap { int _memoryPressure; partial void _OnBitmapCreated () { _memoryPressure = ByteCount; GC.AddMemoryPressure (_memoryPressure); } protected override void Dispose (bool disposing) { if (_memoryPressure != 0) { GC.RemoveMemoryPressure (_memoryPressure); _memoryPressure = 0; } } } TODO: "extend" this for `<method/>`s as well? [0]: https://developer.android.com/reference/android/graphics/Bitmap [1]: https://learn.microsoft.com/dotnet/api/system.gc.addmemorypressure?view=net-7.0 [2]: https://learn.microsoft.com/dotnet/api/system.gc.removememorypressure?view=net-7.0 [3]: https://learn.microsoft.com/dotnet/csharp/language-reference/keywords/partial-method
054dd3a to
6138fa5
Compare
jpobst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks for the changes!
Fixes: dotnet/java-interop#1090 Changes: dotnet/java-interop@53bfb4a...0355acf * dotnet/java-interop@0355acf8: [Xamarin.Android.Tools.Bytecode] Ignore synthetic ctor params (dotnet/java-interop#1091) * dotnet/java-interop@909239d4: [generator] Add support for peerConstructorPartialMethod (dotnet/java-interop#1087) * dotnet/java-interop@8f3fe625: [generator] Fix an NRE when cloning a method with generic arguments (dotnet/java-interop#1089) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Context: #1085
Context: dotnet/runtime#82121
Some Java objects are big, e.g.
Bitmapinstances, but asfar as MonoVM is concerned, the
Bitmapinstances are tiny: afew pointers, and that's it. MonoVM doesn't know that it could be
referencing several MB of data in the Java VM.
MonoVM is gaining support for
GC.AddMemoryPressure()andGC.RemoveMemoryPressure(), which potentially allows for theparent of all cross-VM GC integrations: using the
GCmethods toinform MonoVM of how much non-managed memory has been allocated.
This could allow MonoVM to collect more frequently when total process
memory is low.
How should we call
GC.AddMemoryPressure()andGC.RemoveMemoryPressure()?GC.RemoveMemoryPressure()is straightforward: a class can overrideJava.Lang.Object.Dispose(bool).GC.AddMemoryPressure()is the problem: where and how can it becalled from a class binding? This is trickier, as there was no way
to have custom code called by the bound type. Instead, various
forms of "hacky workarounds" are often employed, e.g. copying
generator-emitted content into apartialclass, then usingmetadatato preventgeneratorfrom binding those members.It's all around fugly.
Fortunately C# has a solution:
partialmethods!Add support for a
peerConstructorPartialMethodmetadata entry,applicable to
<class/>elements, which contains the name of apartialmethod to both declare and invoke from the "peer constructor":This will alter our existing "peer constructor" generation code, a'la:
to instead become:
This allows a hand-written
partial class Bitmapto do:TODO: "extend" this for
<method/>s as well?