-
Notifications
You must be signed in to change notification settings - Fork 58
[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. |
@@ -52,11 +52,16 @@ public void SetVisibility (string visibility) | |||
|
|||
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
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.
Bitmap
instances, but asfar as MonoVM is concerned, the
Bitmap
instances 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
GC
methods 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 apartial
class, then usingmetadata
to preventgenerator
from binding those members.It's all around fugly.
Fortunately C# has a solution:
partial
methods!Add support for a
peerConstructorPartialMethod
metadata entry,applicable to
<class/>
elements, which contains the name of apartial
method 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 Bitmap
to do:TODO: "extend" this for
<method/>
s as well?