[libmonodroid] Enable GC bridge on desktop Java. #31
Conversation
There is still some weird GC behavior that happens only on desktop. It seems like calling `System.gc()` there doesn't properly collect as much references as it could which mean that Java/managed peer pairs are kept around longer than they should be. Nonetheless, having this in still means we have a much better story for the designer and XF Previewer memory wise.
|
Hi @garuma, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
|
This is where we have too many moving parts. :-) The goal, slightly delayed by open-sourcing, was to delete the xamarin-android GC bridge processing logic and replace it with the ~same logic from Java.Interop, which, coincidentally, already has (partial?) support for AppDomains. The problem is twofold:
Would you mind seeing if the Java.Interop approach works for you? (And maybe start that integration work? ;-) |
|
Seems to me like it's about the same thing, just a slightly different way to iterate over domains when doing bridging. Up to you if you feel like this part of Java.Interop can be productized faster than reviewing+merging this PR. Alternatively this can provide a baseline that Java.Interop can compare to once it's ready to be productized. Also of importance, is the designer custom control support code already fully mirrored in Java.Interop? I know some of it is there but maybe not all. |
| mono.mono_field_static_set_value (jnienv_vtable, android_runtime_jnienv_bridge_processing_field, &true_value); | ||
|
|
||
| ref_val = 1; | ||
| /* add java refs for items on the list which reference each other */ | ||
| for (i = 0; i < num_sccs; i++) { | ||
| scc = sccs [i]; | ||
|
|
||
| /* only handle objects part of our current domain */ |
There was a problem hiding this comment.
Why only handle objects in the current domain?
In retrospect, it's unlikely I'll be able to merge Java.Interop faster than reviewing+merging this PR, in part because this PR requires Windows build support, which Would it be possible for you to update this PR to remove the AppDomain-specific collection code -- as discussed on Slack, there doesn't appear to be an actual use case for AppDomain-specific collections -- and then we can merge this PR? Additionally, "someone" (volunteers? ;-) is going to need to ensure that |
|
👍 |
Instead of having a top level loop on domains and process references per-domain we instead use the same technique than Java.Interop and instead set the static bridge_processing field across all domains and then process all references in one go. This is safe as manipulating MonoObjet fields (mono_field_set_value/mono_field_get_value/...) don't seem to need to be invoked in a specific appdomain context. Only the bridge_processing field, being static, needs to be set across domains.
|
@jonpryor Updated as per discussion. It should now be closer in spirit to Java.Interop |
| android_runtime_jnienv = runtime; | ||
| android_runtime_jnienv_bridge_processing_field = mono.mono_class_get_field_from_name (runtime, "BridgeProcessing"); | ||
| MonoClass *android_runtime_jnienv = runtime; | ||
| MonoClassField *android_runtime_jnienv_bridge_processing_field = mono.mono_class_get_field_from_name (runtime, "BridgeProcessing"); |
There was a problem hiding this comment.
Can we shorten this name so that the =s aren't screwed up? :-D
|
@jonpryor done |
The XAIntegrationDebug configuration build was broken, for two
reasons:
1. `make xa-all` wasn't restoring nuget packages:
repl.cs(43,24): error CS0234: The type or namespace name `Terminal'
does not exist in the namespace `Mono'.
Are you missing an assembly reference?
2. `generator-Tests` was part of the XAIntegrationDebug
configuration, but `generator` was not. This meant that
`generator-Tests` project couldn't build.
BaseGeneratorTest.cs(7,23): error CS0234: The type or namespace
name `Binder' does not exist in the namespace `Xamarin.Android'.
Are you missing an assembly reference?
Fix the XAIntegrationDebug and XAIntegrationRelease
configuration mappings.
There is still some weird GC behavior that happens only on desktop. It seems like calling
System.gc()there doesn't properly collect as much references as it could which mean that Java/managed peer pairs are kept around longer than they should be.Nonetheless, having this in still means we have a much better story for the designer and XF Previewer memory wise.