[Mono.Android] call new Java "GC Bridge" APIs #10125
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,3 +35,5 @@ Makefile eol=lf | |
| *.wixproj eol=crlf | ||
| *.wxs eol=crlf | ||
| *.rtf eol=crlf | ||
|
|
||
| packages/* filter=lfs diff=lfs merge=lfs -text | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| <Project> | ||
| <!-- Use version in local packages folder --> | ||
| <Target Name="UpdateRuntimeVersion" BeforeTargets="ProcessFrameworkReferences"> | ||
| <PropertyGroup> | ||
| <_Version>10.0.0-dev</_Version> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <KnownFrameworkReference Update="Microsoft.NETCore.App" | ||
| Condition=" '%(KnownFrameworkReference.TargetFramework)' == 'net10.0' " | ||
| DefaultRuntimeFrameworkVersion="$(_Version)" | ||
| LatestRuntimeFrameworkVersion="$(_Version)" | ||
| TargetingPackVersion="$(_Version)" | ||
| /> | ||
| <KnownRuntimePack | ||
| Update="Microsoft.NETCore.App" | ||
| Condition=" '%(KnownRuntimePack.TargetFramework)' == 'net10.0' " | ||
| LatestRuntimeFrameworkVersion="$(_Version)" | ||
| /> | ||
| </ItemGroup> | ||
| </Target> | ||
| </Project> |
Submodule Java.Interop
updated
7 files
Git LFS file not shown
Git LFS file not shown
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| using System.Reflection; | ||
| using System.Runtime.CompilerServices; | ||
| using System.Runtime.InteropServices; | ||
| using System.Runtime.InteropServices.Java; | ||
| using System.Threading; | ||
| using Android.Runtime; | ||
| using Java.Interop; | ||
|
|
@@ -20,10 +21,11 @@ class ManagedValueManager : JniRuntime.JniValueManager | |
| { | ||
| const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; | ||
|
|
||
| Dictionary<int, List<GCHandle>>? RegisteredInstances = new (); | ||
|
|
||
| internal unsafe ManagedValueManager () | ||
| { | ||
| JavaMarshal.Initialize (&FinishBridgeProcessing); | ||
| } | ||
|
|
||
| public override void WaitForGCBridgeProcessing () | ||
|
|
@@ -35,7 +37,7 @@ public override void CollectPeers () | |
| if (RegisteredInstances == null) | ||
| throw new ObjectDisposedException (nameof (ManagedValueManager)); | ||
|
|
||
| var peers = new List<GCHandle> (); | ||
|
|
||
| lock (RegisteredInstances) { | ||
| foreach (var ps in RegisteredInstances.Values) { | ||
|
|
@@ -48,7 +50,8 @@ public override void CollectPeers () | |
| List<Exception>? exceptions = null; | ||
| foreach (var peer in peers) { | ||
| try { | ||
| if (peer.Target is IDisposable disposable) | ||
| disposable.Dispose (); | ||
| } | ||
| catch (Exception e) { | ||
| exceptions = exceptions ?? new List<Exception> (); | ||
|
|
@@ -74,33 +77,35 @@ public override void AddPeer (IJavaPeerable value) | |
| } | ||
| int key = value.JniIdentityHashCode; | ||
| lock (RegisteredInstances) { | ||
| List<GCHandle>? peers; | ||
| if (!RegisteredInstances.TryGetValue (key, out peers)) { | ||
| peers = new List<GCHandle> () { | ||
| CreateReferenceTrackingHandle (value) | ||
| }; | ||
| RegisteredInstances.Add (key, peers); | ||
| return; | ||
| } | ||
|
|
||
| for (int i = peers.Count - 1; i >= 0; i--) { | ||
| var p = peers [i]; | ||
| if (p.Target is not IJavaPeerable peer) | ||
| continue; | ||
| if (!JniEnvironment.Types.IsSameObject (peer.PeerReference, value.PeerReference)) | ||
| continue; | ||
| if (Replaceable (p)) { | ||
| peers [i] = CreateReferenceTrackingHandle (value); | ||
| } else { | ||
| WarnNotReplacing (key, value, peer); | ||
| } | ||
| return; | ||
| } | ||
| peers.Add (CreateReferenceTrackingHandle (value)); | ||
| } | ||
| } | ||
|
|
||
| static bool Replaceable (GCHandle handle) | ||
| { | ||
| if (handle.Target is not IJavaPeerable peer) | ||
| return true; | ||
| return peer.JniManagedPeerState.HasFlag (JniManagedPeerStates.Replaceable); | ||
| } | ||
|
|
@@ -132,14 +137,14 @@ void WarnNotReplacing (int key, IJavaPeerable ignoreValue, IJavaPeerable keepVal | |
| int key = GetJniIdentityHashCode (reference); | ||
|
|
||
| lock (RegisteredInstances) { | ||
| List<GCHandle>? peers; | ||
| if (!RegisteredInstances.TryGetValue (key, out peers)) | ||
| return null; | ||
|
|
||
| for (int i = peers.Count - 1; i >= 0; i--) { | ||
| var p = peers [i]; | ||
| if (p.Target is IJavaPeerable peer && JniEnvironment.Types.IsSameObject (reference, peer.PeerReference)) | ||
| return peer; | ||
| } | ||
| if (peers.Count == 0) | ||
| RegisteredInstances.Remove (key); | ||
|
|
@@ -157,14 +162,15 @@ public override void RemovePeer (IJavaPeerable value) | |
|
|
||
| int key = value.JniIdentityHashCode; | ||
| lock (RegisteredInstances) { | ||
| List<GCHandle>? peers; | ||
| if (!RegisteredInstances.TryGetValue (key, out peers)) | ||
| return; | ||
|
|
||
| for (int i = peers.Count - 1; i >= 0; i--) { | ||
| var p = peers [i]; | ||
| if (object.ReferenceEquals (value, p.Target)) { | ||
| peers.RemoveAt (i); | ||
| FreeHandle (p); | ||
| } | ||
| } | ||
| if (peers.Count == 0) | ||
|
|
@@ -251,13 +257,34 @@ public override List<JniSurfacedPeerInfo> GetSurfacedPeers () | |
| var peers = new List<JniSurfacedPeerInfo> (RegisteredInstances.Count); | ||
| foreach (var e in RegisteredInstances) { | ||
| foreach (var p in e.Value) { | ||
| if (p.Target is not IJavaPeerable peer) | ||
| continue; | ||
| peers.Add (new JniSurfacedPeerInfo (e.Key, new WeakReference<IJavaPeerable> (peer))); | ||
| } | ||
| } | ||
| return peers; | ||
| } | ||
| } | ||
|
|
||
| static GCHandle CreateReferenceTrackingHandle (IJavaPeerable value) => | ||
| JavaMarshal.CreateReferenceTrackingHandle (value, value.JniObjectReferenceControlBlock); | ||
|
|
||
| static unsafe void FreeHandle (GCHandle handle) | ||
| { | ||
| IntPtr context = JavaMarshal.GetContext (handle); | ||
| NativeMemory.Free ((void*) context); | ||
| } | ||
|
|
||
| [UnmanagedCallersOnly] | ||
| internal static unsafe void FinishBridgeProcessing (nint sccsLen, StronglyConnectedComponent* sccs, nint ccrsLen, ComponentCrossReference* ccrs) | ||
| { | ||
| Java.Lang.JavaSystem.Gc (); | ||
|
Member
Author
There was a problem hiding this comment. This is the C# binding of |
||
|
|
||
| JavaMarshal.ReleaseMarkCrossReferenceResources ( | ||
| new Span<StronglyConnectedComponent> (sccs, (int) sccsLen), | ||
| new Span<ComponentCrossReference> (ccrs, (int) ccrsLen)); | ||
| } | ||
|
|
||
| const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; | ||
|
|
||
| static readonly Type[] XAConstructorSignature = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) }; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@jonathanpeppers Note that the method that you register here will be called directly from the GC while the world is stopped. This means that you shouldn't register a managed method here. Instead there should be a native method that is not blocking (aka it dispatches the work of doing the java GC to a separate thread) and then returns so the C# GC can conclude. Once this separate thread finishes with the java gc, only then it can callback into managed so it does the
FinishCrossReferenceProcessing(according to the new version of the api)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 will revert f4a2148, I was already kind of skeptical that everything could be done in managed code.
/cc @grendello I think we need to be able to run these lines in our CoreCLR native code:
android/src/native/mono/monodroid/osbridge.cc
Lines 983 to 989 in 1f89124
Then at the end, it would call the UCO
FinishBridgeProcessing()method.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.
@BrzVlad how hard it is to register an
ecallwith CoreCLR? We can register a standard p/invoke method here, butecallwould be faster. I seem to recall it's not as straightforward in CoreCLR as it is in MonoVM, though?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.
By
ecalldo you mean mono'smono_add_internal_call? It is not clear to me how would you plan to use it in this context ?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.
Yep, equivalent to Mono's
icall. I assume there's a way to obtain its address from native code and use it to register in the above call.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.
Maybe I'm missing some context, but
icallsare used for managed code to call into native runtime code, that is registered to the runtime. While inJavaMarshal.Initialize (..);you are registering a native callback to be called by the runtime during GC. This native code is called directly, there should be no overhead or transitions involved, so I don't understand how anicallwould be relevant here.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.
OK, so it's me who misunderstood - I thought the callback would be called by the managed code at some point, not the native runtime. If it's a direct call from native runtime then the point is moot. Thanks for the clarification :) (I'm only now getting acquainted with this new GC bridge)