-
Notifications
You must be signed in to change notification settings - Fork 573
[TrimmableTypeMap] Add AOT-safe array and collection factories using MakeArrayType and MakeGenericType #12030
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?
Changes from all commits
ca78fa2
db04690
9e0ce62
0c62664
53d50fb
b959f4a
1143044
d41d005
187d9fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,19 +57,6 @@ static class JavaConvert { | |
| } }, | ||
| }; | ||
|
|
||
| static readonly Dictionary<Type, JavaPeerContainerFactory> ScalarContainerFactories = new Dictionary<Type, JavaPeerContainerFactory> { | ||
| { typeof (bool), JavaPeerContainerFactory<bool>.Instance }, | ||
| { typeof (byte), JavaPeerContainerFactory<byte>.Instance }, | ||
| { typeof (sbyte), JavaPeerContainerFactory<sbyte>.Instance }, | ||
| { typeof (char), JavaPeerContainerFactory<char>.Instance }, | ||
| { typeof (short), JavaPeerContainerFactory<short>.Instance }, | ||
| { typeof (int), JavaPeerContainerFactory<int>.Instance }, | ||
| { typeof (long), JavaPeerContainerFactory<long>.Instance }, | ||
| { typeof (float), JavaPeerContainerFactory<float>.Instance }, | ||
| { typeof (double), JavaPeerContainerFactory<double>.Instance }, | ||
| { typeof (string), JavaPeerContainerFactory<string>.Instance }, | ||
| }; | ||
|
|
||
| static Func<IntPtr, JniHandleOwnership, object?>? GetJniHandleConverter (Type? target) | ||
| { | ||
| if (target == null) | ||
|
|
@@ -89,9 +76,14 @@ static class JavaConvert { | |
|
|
||
| if (target.IsGenericType && !target.IsGenericTypeDefinition) { | ||
| if (RuntimeFeature.TrimmableTypeMap) { | ||
| var factoryConverter = TryGetFactoryBasedConverter (target); | ||
| if (factoryConverter != null) | ||
| return factoryConverter; | ||
| if (SafeJavaCollectionFactory.TryGetCollectionType (target, out _)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 For value/value shapes this probe is also slightly at odds with the AOT-safety design: (Rule: Avoid redundant reflection on hot paths — repo-conventions Performance) |
||
| return (h, t) => { | ||
| if (SafeJavaCollectionFactory.TryCreateFromJniHandle (target, h, t, out var collection)) { | ||
| return collection; | ||
| } | ||
| return null; | ||
| }; | ||
| } | ||
| } else { | ||
| var factoryConverter = TryMakeGenericCollectionTypeFactory (target); | ||
| if (factoryConverter != null) | ||
|
|
@@ -110,6 +102,11 @@ static class JavaConvert { | |
|
|
||
| [UnconditionalSuppressMessage ("ReflectionAnalysis", "IL2055:RequiresUnreferencedCode", | ||
| Justification = "The target generic type is expected to be preserved by the trimmer as the target type in marshaling.")] | ||
| [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", | ||
| Justification = "This legacy MakeGenericType() path is only reached from the '!RuntimeFeature.TrimmableTypeMap' branch above, " + | ||
| "i.e. on the reflection-based Mono/CoreCLR runtimes where dynamic code is available. NativeAOT always enables the trimmable " + | ||
| "type map, so under AOT the branch above (SafeJavaCollectionFactory) is used instead and this helper is never executed. " + | ||
| "The AOT-safe generic collection construction lives in SafeJavaCollectionFactory.")] | ||
| static Func<IntPtr, JniHandleOwnership, object?>? TryMakeGenericCollectionTypeFactory (Type target) | ||
| { | ||
| if (target.GetGenericTypeDefinition() == typeof (IDictionary<,>)) { | ||
|
|
@@ -129,73 +126,6 @@ static class JavaConvert { | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// AOT-safe converter using <see cref="JavaPeerContainerFactory"/> from the generated proxy. | ||
| /// Avoids <c>MakeGenericType()</c> by using the pre-typed factory from the proxy attribute. | ||
| /// </summary> | ||
| static Func<IntPtr, JniHandleOwnership, object?>? TryGetFactoryBasedConverter (Type target) | ||
| { | ||
| if (TryGetSingleGenericArgument (target, typeof (IList<>), typeof (JavaList<>), out var listElementType)) { | ||
| var factory = TryGetContainerFactory (listElementType); | ||
| if (factory != null) | ||
| return (h, t) => factory.CreateList (h, t); | ||
| } | ||
|
|
||
| if (TryGetSingleGenericArgument (target, typeof (ICollection<>), typeof (JavaCollection<>), out var collectionElementType)) { | ||
| var factory = TryGetContainerFactory (collectionElementType); | ||
| if (factory != null) | ||
| return (h, t) => factory.CreateCollection (h, t); | ||
| } | ||
|
|
||
| if (TryGetDictionaryArguments (target, out var typeArgs)) { | ||
| var keyFactory = TryGetContainerFactory (typeArgs [0]); | ||
| var valueFactory = TryGetContainerFactory (typeArgs [1]); | ||
| if (keyFactory != null && valueFactory != null) | ||
| return (h, t) => valueFactory.CreateDictionary (keyFactory, h, t); | ||
| } | ||
|
|
||
| return null; | ||
|
|
||
| static bool TryGetSingleGenericArgument (Type target, Type interfaceType, Type wrapperType, [NotNullWhen (true)] out Type? argument) | ||
| { | ||
| if (target.IsGenericType && !target.IsGenericTypeDefinition) { | ||
| var genericDef = target.GetGenericTypeDefinition (); | ||
| if (genericDef == interfaceType || genericDef == wrapperType) { | ||
| argument = target.GetGenericArguments () [0]; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| argument = null; | ||
| return false; | ||
| } | ||
|
|
||
| static bool TryGetDictionaryArguments (Type target, [NotNullWhen (true)] out Type []? arguments) | ||
| { | ||
| if (target.IsGenericType && !target.IsGenericTypeDefinition) { | ||
| var genericDef = target.GetGenericTypeDefinition (); | ||
| if (genericDef == typeof (IDictionary<,>) || genericDef == typeof (JavaDictionary<,>)) { | ||
| arguments = target.GetGenericArguments (); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| arguments = null; | ||
| return false; | ||
| } | ||
|
|
||
| static JavaPeerContainerFactory? TryGetContainerFactory (Type elementType) | ||
| { | ||
| if (ScalarContainerFactories.TryGetValue (elementType, out var scalarFactory)) | ||
| return scalarFactory; | ||
|
|
||
| if (typeof (IJavaPeerable).IsAssignableFrom (elementType)) | ||
| return TrimmableTypeMap.Instance?.GetContainerFactory (elementType); | ||
|
|
||
| return null; | ||
| } | ||
| } | ||
|
|
||
| static Func<IntPtr, JniHandleOwnership, object> GetJniHandleConverterForType ([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type t) | ||
| { | ||
| MethodInfo m = t.GetMethod ("FromJniHandle", BindingFlags.Static | BindingFlags.Public)!; | ||
|
|
||
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.
🤖 💡 Formatting — Use
nameof (elementType)instead of the string literal"elementType"so the argument name stays correct under future renames.(Rule: Prefer
nameoffor argument names — repo-conventions)