-
Notifications
You must be signed in to change notification settings - Fork 545
[Mono.Android] call new Java "GC Bridge" APIs #10125
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
base: main
Are you sure you want to change the base?
Conversation
bbc1c64
to
58739d3
Compare
} | ||
} | ||
return peers; | ||
} | ||
} | ||
|
||
static GCHandle CreateReferenceTrackingHandle (IJavaPeerable value) => | ||
JavaMarshal.CreateReferenceTrackingHandle (value, value.PeerReference.Handle); |
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 concerns me, because this value will change over time!
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.
To elaborate, with MonoVM -- and thus I assume eventually with dotnet/runtime#114184 -- when a "bridged" instance is no longer reachable from the .NET GC, we:
- "Toggle"
JavaObject.PeerReference
from a JNI Global Reference to a JNI Weak Global Reference. - Trigger a Java-side GC.
- If the instance survives the Java-side GC, we "toggle" the JNI Weak Global Reference to a JNI Global Reference.
This means the value of value.PeerReference
changes, because 1...3 will result in a different JNI Global Reference value.
Meaning using "value.PeerReference.Handle` is immediately "bad".
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.
I also wonder if using value.PeerReference
is necessary? It's for the userContext
parameter; perhaps it could just be null
?
E.g. CreateCrossReferenceHandle()
calls CreateHandleWithExtraInfo()
and providing userContext
as the "extra info", and JavaMarshal.GetContext()
returns GetExtraInfoFromHandle()
, i.e. appears to return "userContext".
Aside: if true, this makes me think that JavaMarshal.GetContext()
is mis-named, and should instead be called JavaMarshal.GetUserContext()
, which would make the relationship more apparent.
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.
…though, to be fair, it's CreateReferenceTrackingHandle(object obj, System.IntPtr context)
, so GetContext()
does make sense…
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.
Where @jonpryor got to in the end is how this should be used. He is correct in that the PeerReference
(assumed to be a JNI handle) should be contained within the allocated memory that is the "context". Renaming to "UserContext" is a reasonable request and we can workshop that in API review.
The PeerReference
property could dereference the context pointer and get the JNI handle if desired. In fact, the context could be a pointer sized allocation or any size, store any addtiional details you want there.
Context: dotnet/runtime#114184 Context: https://github.com/jonathanpeppers/BridgeSandbox Context: dotnet/runtime@main...BrzVlad:runtime:feature-clr-gcbridge So far, I: * Built dotnet/runtime, I built this branch: https://github.com/jonathanpeppers/runtime/tree/gcbridge_impl * Put the relevant arm64 packs in `packages/` folder and configured a `NuGet.config` to use them. * Setup `build-tools\scripts\custom-runtime.targets` to use the 10.0.0-dev dotnet/runtime packs. * In `ManagedValueManager.cs`... * Call a new native method on startup: var mark_cross_references_ftn = RuntimeNativeMethods.clr_initialize_gc_bridge (&BridgeProcessingFinished); * Pass the returned function pointer to: JavaMarshal.Initialize (mark_cross_references_ftn); * In `AddPeer(IJavaPeerable value)` call `JavaMarshal.CreateReferenceTrackingHandle()` * In `RemovePeer(IJavaPeerable value)` call: static unsafe void FreeHandle (GCHandle handle) { IntPtr context = JavaMarshal.GetContext (handle); NativeMemory.Free((void*)context); }
ab51219
to
f4a2148
Compare
What I suspect needs to happen is that partial class JavaObject {
// Remove the following members
[NonSerialized] IntPtr handle;
[NonSerialized] JniObjectReferenceType handle_type;
// Used by JavaInteropGCBridge
[NonSerialized] IntPtr weak_handle;
[NonSerialized] int refs_added;
} and replace with: internal struct JniObjectInfo {
public IntPtr handle;
public JniObjectReferenceType handle_type;
// Used by JavaInteropGCBridge
public IntPtr weak_handle;
public int refs_added;
}
partial class JavaObject {
IntPtr JniObjectInfo = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(JniObjectInfo)));
} …then update everything within This would allow Further note: this in turn means that the appropriate starting point for this PR is in dotnet/java-interop, not dotnet/android. |
The new APIs are all decorated with:
We can use them in java.interop, but would have to wrap them all with java.interop is also not building with The new APIs will only be in .NET 10. So, maybe we have to do a branch of java.interop that just updates |
[UnmanagedCallersOnly] | ||
internal static unsafe void FinishBridgeProcessing (nint sccsLen, StronglyConnectedComponent* sccs, nint ccrsLen, ComponentCrossReference* ccrs) | ||
{ | ||
Java.Lang.JavaSystem.Gc (); |
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 is the C# binding of java.lang.System.gc()
: https://developer.android.com/reference/java/lang/System#gc()
src/Mono.Android/Microsoft.Android.Runtime/ManagedValueManager.cs
Outdated
Show resolved
Hide resolved
Context: dotnet/runtime#114184 Context: dotnet/android#10125 Context: dotnet/android#10125 (comment) Part of adding a GC bridge to CoreCLR are the new APIs: namespace System.Runtime.InteropServices.Java; public struct ComponentCrossReference { public nint SourceGroupIndex, DestinationGroupIndex; } public unsafe struct StronglyConnectedComponent { public nint Count; public IntPtr* Context; } public static partial class JavaMarshal { public static unsafe void Initialize( delegate* unmanaged< System.IntPtr, // sccsLen StronglyConnectedComponent*, // sccs System.IntPtr, // ccrsLen ComponentCrossReference*, // ccrs void> markCrossReferences); public static GCHandle CreateReferenceTrackingHandle(object obj, IntPtr context); public static IntPtr GetContext(GCHandle obj); } Of note is the "data flow" of `context`: * `JavaMarshal.CreateReferenceTrackingHandle()` has a "`context`" parameter. * The `context` parameter to `JavaMarshal.CreateReferenceTrackingHandle()` is the return value of `JavaMarshal.GetContext()` * The `context` parameter to `JavaMarshal.CreateReferenceTrackingHandle()` is stored within `StronglyConnectedComponent.Context`. * The `markCrossReferences` parameter of `JavaMarshal.Initialize()` is called by the GC bridge and given a native array of `StronglyConnectedComponent` instances, which contains `Context`. The short of it is that the proposed GC bridge doesn't contain direct access to the `IJavaPeerable` instances in play. Instead, it has access to "context" which must contain the JNI Object Reference information that the `markCrossReferences` callback needs access to. Furthermore, the `context` pointer value *cannot change*, i.e. it needs to be a native pointer value a'la **malloc**(3), ***not*** a value which can be moved by the GC. (The *contents* can change; the pointer value cannot.)) While we're still prototyping this, what we currently believe we need is the JNI object reference, JNI object reference type, and (maybe?) the JNI Weak Global Reference value and "refs added" values. Update `IJavaPeerable` to add a `JniObjectReferenceControlBlock` property which can be used as the `context` parameter: partial interface IJavaPeerable { IntPtr JniObjectReferenceControlBlock => 0; } This supports usage of: IJavaPeerable value = … GCHandle handle = JavaMarshal.CreateReferenceTrackingHandle( value, value.JniObjectReferenceControlBlock );
I updated this to use: |
Context: dotnet/runtime#114184
Context: https://github.com/AaronRobinsonMSFT/BridgeSandbox
Context: dotnet/runtime@main...BrzVlad:runtime:feature-clr-gcbridge
So far, I:
https://github.com/jonathanpeppers/runtime/tree/gcbridge_impl
Put the relevant arm64 packs in
packages/
folder and configured aNuGet.config
to use them.Setup
build-tools\scripts\custom-runtime.targets
to use the10.0.0-dev dotnet/runtime packs.
In
ManagedValueManager.cs
...Call a new native method on startup:
In
AddPeer(IJavaPeerable value)
callJavaMarshal.CreateReferenceTrackingHandle()
In
RemovePeer(IJavaPeerable value)
call:Update
I added a second commit, of what it could look like if everything was managed/C# code.