Uh oh!
There was an error while loading. Please reload this page.
[TrimmableTypeMap] Implement runtime TypeManager, ValueManager, and JavaConvert - #10967
Conversation
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 0 code issues. CI is still running (dotnet-android and Xamarin.Android-PR in progress). Review will need to be re-evaluated once CI completes.
Code review
- ✅ No null-forgiving operator (
!) usage - ✅ All new files have
#nullable enable - ✅ Proper Mono style (tabs, space before
()and[]) - ✅ Namespace style consistent with existing files
- ✅ Feature flag with ILLink substitutions follows established pattern
- ✅
Interlocked.CompareExchange+Debug.Assertfor single-instance safety in bothTrimmableTypeMapandJavaMarshalValueManager - ✅ JNI callback delegate properly rooted in static field (
s_onRegisterNatives) - ✅
OnRegisterNativescallsEnvironment.FailFaston error (unrecoverable state) - ✅ All TODO comments reference tracking issues (#10794 or java-interop #1391)
- ✅
TypeMapExceptionis sealed - ✅
TryGetTypeuses[NotNullWhen(true)]annotation - ✅ 255/255 generator tests pass
- ✅
Mono.Android.dllbuilds locally
Architecture notes
- Clean encapsulation: all proxy attribute access goes through
TrimmableTypeMap—JavaPeerProxyis not leaked to callers JavaMarshalValueManager(renamed fromManagedValueManager) takesTrimmableTypeMap?in constructor — immutable, no settable propertyTrimmableTypeMapTypeManagerdelegates all lookups toTrimmableTypeMap- RegisterNatives bootstrap: Java → managed callback → proxy →
IAndroidCallableWrapper.RegisterNatives(JniType)→ UCO function pointers - All behind
RuntimeFeature.TrimmableTypeMap(defaultfalse) — zero impact when off
Review generated by android-reviewer from review guidelines.
e27ad18 to
d1e28a1Compared1e28a1 to
b07f9b9Compare
jonathanpeppers
left a comment
There was a problem hiding this comment.
This file grew:
"lib/arm64-v8a/lib_System.Private.CoreLib.dll.so": {
-- "Size": 633928++ "Size": 691720
},It's probably OK, but I wonder what did it.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
a956a46 to
a8f3e62CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
a8f3e62 to
d2e4bb1Comparesimonrozsival
commented
Mar 20, 2026
Re: the |
- JavaPeerProxy / JavaPeerProxy<T> — AOT-safe proxy attribute base - IAndroidCallableWrapper — RegisterNatives(JniType) for ACW types - JavaPeerContainerFactory<T> — AOT-safe array/list/collection/dict - TypeMapException — error reporting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Proxy types now extend JavaPeerProxy<T> instead of JavaPeerProxy. TargetType and GetContainerFactory() are inherited from the generic base. Generator references TrimmableTypeMap for ActivateInstance and RegisterMethod. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add TrimmableTypeMap class with core typemap functionality: TryGetType, TryCreatePeer, GetInvokerType, GetContainerFactory, ActivateInstance. Add TrimmableTypeMapTypeManager delegating to TrimmableTypeMap. Rename ManagedValueManager to JavaMarshalValueManager. Add proxy-based peer creation in TryConstructPeer via TrimmableTypeMap.TryCreatePeer. Add RuntimeFeature.TrimmableTypeMap feature switch with ILLink substitutions. Wire into JNIEnvInit (CoreCLR) and JavaInteropRuntime + JreRuntime (NativeAOT). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add registerNatives(Class) native method to mono.android.Runtime.java so JCW static initializer blocks can trigger native method registration. Add to TrimmableTypeMap: - RegisterBootstrapNativeMethod() registers the JNI callback during init - OnRegisterNatives() resolves the proxy and calls IAndroidCallableWrapper.RegisterNatives(JniType) to bind UCO ptrs - RegisterMethod() helper for per-method registration (TODO: batch) Wire RegisterBootstrapNativeMethod() call in JNIEnvInit after runtime creation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When TrimmableTypeMap is available, use JavaPeerContainerFactory from the proxy for IList<T>, ICollection<T>, IDictionary<K,V> marshaling and array creation instead of MakeGenericType/Array.CreateInstance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ranks 1-3: direct new T[], T[][], T[][][] — fully AOT-safe. Rank 4+: when dynamic code is supported (CoreCLR), falls back to MakeArrayType + CreateInstanceFromArrayType. Throws on NativeAOT. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
71ed636 to
2b80737CompareCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TrimmableTypeMapTypeManager.GetSimpleReferences now walks the base type chain when a type has no TypeMap entry. This handles managed-only subclasses like JavaProxyThrowable (extends Error but has no [Register]) by resolving to the nearest registered ancestor's JNI name. Without this, error wrapping (RaisePendingException → JavaProxyThrowable) crashes with NotSupportedException before the real exception is visible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The trimmable path uses proxy.CreateInstance() which creates a fully initialized peer. Override CreatePeer instead of TryConstructPeer to bypass the GetUninitializedObject + reflection pattern. This fixes JavaCast<T> where the pre-allocated shell from GetUninitializedObject was returned with Handle == IntPtr.Zero because CreateInstance created a separate object, ignoring the shell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
387bfcb to
2566369CompareILLink/ILC needs the feature switch check as a standalone if statement to constant-propagate and eliminate the dead code branch. Compound conditions like 'RuntimeFeature.TrimmableTypeMap && x' may not be optimized correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The runtime's GetOrCreateExternalTypeMapping already handles assembly resolution via System.Runtime.InteropServices.TypeMappingEntryAssembly config property. It loads the entry assembly, walks its TypeMapAssemblyTargetAttribute attributes, and recursively loads each referenced assembly. The explicit pre-loading loop was redundant and cost ~70ms at startup (46ms Assembly.Load + 24ms GetCustomAttributesData). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
60ca22d to
c0bb477Compare…Manager Two changes to prevent TrimmableTypeMap types from leaking into MonoVM builds: 1. Add ILLink substitutions for IsMonoRuntime and IsCoreClrRuntime. Without these, the trimmer can't eliminate runtime-specific branches (e.g. the CoreCLR JavaMarshalValueManager path stays in MonoVM builds). 2. Remove TrimmableTypeMap? _typeMap field from JavaMarshalValueManager. Access via TrimmableTypeMap.Instance singleton instead. The field reference was rooting the entire TrimmableTypeMap type graph even when the feature switch was false. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c0bb477 to
d1ae3a5Compare…al/trimmable-typemap-runtime-pr
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
[FeatureSwitchDefinition] on the properties already tells the trimmer to substitute these getters, making the XML entries unnecessary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
commented
Mar 24, 2026
Good call — removed all the |
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Found 2 suggestions (no blockers):
- 💡 Patterns:
TrimmableTypeMap.Instancethrows on null, making downstream?.andis not nullguards dead code — 3 locations (JNIEnv.cs:30,JavaMarshalValueManager.cs:512,JavaConvert.cs:152) - 💡 Nullable: Use
provider.Name.IsNullOrEmpty()extension method (TrimmableTypeMap.cs:101)
👍 Well-structured PR with clean separation between legacy and trimmable paths. Good error handling — FailFast for critical JNI registration failures, InvalidOperationException with context for activation errors. Native/managed JnienvInitializeArgs struct kept in sync. All TODOs reference GitHub issues. Feature switch correctly uses [FeatureSwitchDefinition] for trimmer integration.
CI: Public checks all ✅ green. Internal Xamarin.Android-PR still in progress.
Review generated by android-reviewer from review guidelines.
| { | ||
| if (RuntimeFeature.TrimmableTypeMap) { | ||
| var typeMap = TrimmableTypeMap.Instance; | ||
| if (typeMap is not null && targetType is not null) { |
There was a problem hiding this comment.
🤖 💡 Patterns — TrimmableTypeMap.Instance throws InvalidOperationException if s_instance is null, so typeMap is guaranteed non-null here. The typeMap is not null check is dead code.
If the intent is defensive (gracefully fall back to base.CreatePeer if uninitialized), access s_instance directly instead of Instance. Otherwise, remove the null check.
Same pattern in JNIEnv.ArrayCreateInstance (Instance?.GetContainerFactory) and JavaConvert.TryGetContainerFactory (Instance?.GetContainerFactory).
Rule: Method names must reflect behavior (Postmortem #4)
| internal static bool TryGetJniNameForType (Type type, [NotNullWhen (true)] out string? jniName) | ||
| { | ||
| if (type.GetCustomAttributes (typeof (IJniNameProviderAttribute), inherit: false) is [IJniNameProviderAttribute provider, ..] | ||
| && !string.IsNullOrEmpty (provider.Name)) { |
There was a problem hiding this comment.
🤖 💡 Nullable — Per repo conventions, use the NullableExtensions instance method:
&&!provider.Name.IsNullOrEmpty()){instead of string.IsNullOrEmpty (provider.Name). The extension integrates with [NotNullWhen] for NRT flow analysis.
Rule: Use IsNullOrEmpty() extension
Uh oh!
There was an error while loading. Please reload this page.
Closes#10791
Summary
Adds the runtime-side support for the trimmable typemap: type resolution, peer creation, native method registration, and AOT-safe collection marshaling. All behind
RuntimeFeature.TrimmableTypeMap(defaults tofalse).New runtime types
JavaPeerProxy/JavaPeerProxy<T>— AOT-safe proxy attribute base. Generated proxy types extend this and provideCreateInstance()for peer creation andGetContainerFactory()for collection marshaling.IAndroidCallableWrapper—RegisterNatives(JniType)interface for ACW proxy types to register JNI native methods.JavaPeerContainerFactory— AOT-safe factories for arrays, lists, collections, and dictionaries withoutMakeGenericType().TrimmableTypeMap— Central class owning theTypeMappingdictionary. Encapsulates all proxy attribute access: peer creation, invoker resolution, container factories, and native method registration bootstrap.TrimmableTypeMapTypeManager—JniTypeManagersubclass delegating type lookups toTrimmableTypeMap.RegisterNativeMembersthrowsUnreachableException(JCW static blocks handle registration).Type resolution
GetProxyForManagedType(Type)resolves managed type → JNI name viaIJniNameProviderAttribute(shared interface for[Register]and[JniTypeSignature]) → TypeMap dictionary → proxy. Results cached inConcurrentDictionary.TryGetJniNameForTypeshared betweenTrimmableTypeMapandTrimmableTypeMapTypeManager.ActivateInstanceuses JNIGetObjectClass→GetJniTypeNameFromClass→ TypeMap lookup for constructor activation (proxy types have self-applied attribute, not the target type).TypeMap dictionary initialization
GetOrCreateExternalTypeMapping<Java.Lang.Object>()handles assembly resolution automatically via theSystem.Runtime.InteropServices.TypeMappingEntryAssemblyruntimeconfig property (set inTrimmable.targets). No manualAssembly.Loadpre-loading is needed — the runtime walksTypeMapAssemblyTargetAttributeattributes from the entry assembly recursively.Native interop refactoring
RegisterJniNativespassed via init args —Initializesetsargs->registerJniNativesFn(null whenTrimmableTypeMap=true). Eliminates thecreate_delegatecall forRegisterJniNatives, letting the trimmer remove it cleanly in the trimmable path.registerNativesstub —Host::Java_mono_android_Runtime_registerNativesno-op for the trimmable path (managed code handles registration viaInitialize).jnienv_register_jni_nativescall site.Initializeis now the singlecreate_delegateentry point from native code.RegisterNatives bootstrap
mono.android.Runtime.registerNatives(Class)Java native method addedTrimmableTypeMap.Initialize()registers the JNI callback during init (behind explicitif (RuntimeFeature.TrimmableTypeMap)guard for trimmer compatibility)OnRegisterNativesresolves the proxy and callsIAndroidCallableWrapper.RegisterNatives()to bind UCO function pointersAOT-safe JavaConvert
JavaConvert.GetJniHandleConverter()usesJavaPeerContainerFactoryforIList,ICollection,IDictionaryJNIEnv.ArrayCreateInstance()uses factory pathPeer creation
JavaMarshalValueManager(renamed fromManagedValueManager) overridesCreatePeerfor the trimmable path — callsproxy.CreateInstance()directly, bypassingGetUninitializedObjectGetSimpleReferenceswalks base type chain for managed-only subclasses without[Register]Wiring
RuntimeFeature.TrimmableTypeMapfeature switch with ILLink substitutionsJNIEnvInit(CoreCLR) andJavaInteropRuntime+JreRuntime(NativeAOT) create the new managers when the feature is onJavaMarshalValueManager(renamed fromManagedValueManager) gets proxy-based peer creation inTryConstructPeerDependencies
Test coverage
Trimmer size regression prevention
IsMonoRuntimeandIsCoreClrRuntime— enables trimmer to eliminate runtime-specific branchesJavaMarshalValueManager: removeTrimmableTypeMap?field, useTrimmableTypeMap.InstancesingletonTrimmableTypeMap: private ctor,Initialize()static method,Instancethrows if not initializedTrimmableTypeMapTypeManager: no constructor parameter, uses singletonTrimmableTypeMaptype references outside feature guards — clean trimming for MonoVM/NativeAOT