From 49fd492394c79128f9432d9f107146e3649514c7 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 18 Jul 2022 13:51:25 +0100 Subject: [PATCH 1/2] Apply a variant of the visitor pattern for reflection-based metadata instantiation. --- .../src/System.Text.Json.csproj | 1 + .../Metadata/DefaultJsonTypeInfoResolver.cs | 42 +++++++---------- .../Metadata/JsonTypeInfo.Cache.cs | 17 ++++--- .../Serialization/Metadata/JsonTypeInfo.cs | 22 +++++++-- .../Serialization/Metadata/TypeWitness.cs | 46 +++++++++++++++++++ 5 files changed, 88 insertions(+), 40 deletions(-) create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 91398f24764d2..e09d4f9a49592 100644 --- a/src/libraries/System.Text.Json/src/System.Text.Json.csproj +++ b/src/libraries/System.Text.Json/src/System.Text.Json.csproj @@ -263,6 +263,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs index 44ea6c1b06fa0..0e73eccd751c8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs @@ -3,8 +3,7 @@ using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Reflection; -using System.Runtime.ExceptionServices; +using System.Text.Json.Reflection; using System.Threading; namespace System.Text.Json.Serialization.Metadata @@ -36,6 +35,10 @@ private DefaultJsonTypeInfoResolver(bool mutable) } /// + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "The ctor is marked RequiresUnreferencedCode.")] + [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", + Justification = "The ctor is marked RequiresDynamicCode.")] public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options) { if (type == null) @@ -64,36 +67,23 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options return typeInfo; } - [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", - Justification = "The ctor is marked RequiresUnreferencedCode.")] - [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", - Justification = "The ctor is marked RequiresDynamicCode.")] - private JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) + [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] + [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] + private static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) { - MethodInfo methodInfo = typeof(DefaultJsonTypeInfoResolver).GetMethod(nameof(CreateReflectionJsonTypeInfo), BindingFlags.NonPublic | BindingFlags.Static)!; -#if NETCOREAPP - return (JsonTypeInfo)methodInfo.MakeGenericMethod(type).Invoke(null, BindingFlags.NonPublic | BindingFlags.DoNotWrapExceptions, null, new[] { options }, null)!; -#else - try - { - return (JsonTypeInfo)methodInfo.MakeGenericMethod(type).Invoke(null, new[] { options })!; - } - catch (TargetInvocationException ex) - { - // Some of the validation is done during construction (i.e. validity of JsonConverter, inner types etc.) - // therefore we need to unwrap TargetInvocationException for better user experience - ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); - throw null!; - } -#endif + return ReflectionJsonTypeInfoBuilder.Instance.Visit(type, options); } [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] - private static JsonTypeInfo CreateReflectionJsonTypeInfo(JsonSerializerOptions options) + private sealed class ReflectionJsonTypeInfoBuilder : ITypeVisitor { - JsonConverter converter = GetConverterForType(typeof(T), options); - return new ReflectionJsonTypeInfo(converter, options); + public static ReflectionJsonTypeInfoBuilder Instance { get; } = new(); + public JsonTypeInfo Visit(JsonSerializerOptions options) + { + JsonConverter converter = GetConverterForType(typeof(T), options); + return new ReflectionJsonTypeInfo(converter, options); + } } /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index eb991681847a7..80c936c4b0d5f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text.Json.Reflection; @@ -63,15 +62,13 @@ internal JsonPropertyInfo CreatePropertyUsingReflection(Type propertyType) // If a JsonTypeInfo has already been cached for the property type, // avoid reflection-based initialization by delegating construction // of JsonPropertyInfo construction to the property type metadata. - return jsonTypeInfo.CreateJsonPropertyInfo(declaringTypeInfo: this, Options); + jsonPropertyInfo = jsonTypeInfo.CreateJsonPropertyInfo(declaringTypeInfo: this, Options); } else { // Metadata for `propertyType` has not been registered yet. // Use reflection to instantiate the correct JsonPropertyInfo - s_createJsonPropertyInfo ??= typeof(JsonTypeInfo).GetMethod(nameof(CreateJsonPropertyInfo), BindingFlags.NonPublic | BindingFlags.Static)!; - MethodInfo factoryInfo = s_createJsonPropertyInfo.MakeGenericMethod(propertyType); - jsonPropertyInfo = (JsonPropertyInfo)factoryInfo.Invoke(null, new object[] { this, Options })!; + jsonPropertyInfo = JsonPropertyInfoBuilder.Instance.Visit(propertyType, (this, Options)); } Debug.Assert(jsonPropertyInfo.PropertyType == propertyType); @@ -83,10 +80,12 @@ internal JsonPropertyInfo CreatePropertyUsingReflection(Type propertyType) /// private protected abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options); - private static JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options) - => new JsonPropertyInfo(declaringTypeInfo.Type, declaringTypeInfo, options); - - private static MethodInfo? s_createJsonPropertyInfo; + private sealed class JsonPropertyInfoBuilder : ITypeVisitor<(JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options), JsonPropertyInfo> + { + public static JsonPropertyInfoBuilder Instance { get; } = new(); + public JsonPropertyInfo Visit((JsonTypeInfo declaringTypeInfo, JsonSerializerOptions options) state) + => new JsonPropertyInfo(state.declaringTypeInfo.Type, state.declaringTypeInfo, state.options); + } // AggressiveInlining used although a large method it is only called from one location and is on a hot path. [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index 056a9b2cf6844..5ee07b4929696 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -4,9 +4,9 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; +using System.Text.Json.Reflection; using System.Threading; namespace System.Text.Json.Serialization.Metadata @@ -533,11 +533,25 @@ public static JsonTypeInfo CreateJsonTypeInfo(JsonSerializerOptions option ThrowHelper.ThrowArgumentNullException(nameof(options)); } + return CreateCustomJsonTypeInfo(options); + } + + [RequiresUnreferencedCode(MetadataFactoryRequiresUnreferencedCode)] + [RequiresDynamicCode(MetadataFactoryRequiresUnreferencedCode)] + private static JsonTypeInfo CreateCustomJsonTypeInfo(JsonSerializerOptions options) + { JsonConverter converter = DefaultJsonTypeInfoResolver.GetConverterForType(typeof(T), options, resolveJsonConverterAttribute: false); return new CustomJsonTypeInfo(converter, options); } - private static MethodInfo? s_createJsonTypeInfo; + [RequiresUnreferencedCode(MetadataFactoryRequiresUnreferencedCode)] + [RequiresDynamicCode(MetadataFactoryRequiresUnreferencedCode)] + private sealed class CustomJsonTypeInfoBuilder : ITypeVisitor + { + public static CustomJsonTypeInfoBuilder Instance { get; } = new(); + public JsonTypeInfo Visit(JsonSerializerOptions options) + => CreateCustomJsonTypeInfo(options); + } /// /// Creates a blank instance. @@ -576,9 +590,7 @@ public static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions o ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(type), type, null, null); } - s_createJsonTypeInfo ??= typeof(JsonTypeInfo).GetMethod(nameof(CreateJsonTypeInfo), new Type[] { typeof(JsonSerializerOptions) })!; - return (JsonTypeInfo)s_createJsonTypeInfo.MakeGenericMethod(type) - .Invoke(null, new object[] { options })!; + return CustomJsonTypeInfoBuilder.Instance.Visit(type, options); } /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs new file mode 100644 index 0000000000000..649cfe349dc89 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs @@ -0,0 +1,46 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; + +namespace System.Text.Json.Reflection +{ + /// + /// Encapsulates a generic type parameter that can be unpacked on-demand using a generic visitor, + /// encoding an existential type. Can use reflection to instantiate type parameters that are not known at compile time. + /// Uses concepts taken from https://www.microsoft.com/en-us/research/publication/generalized-algebraic-data-types-and-object-oriented-programming/ + /// + internal abstract class TypeWitness + { + public abstract Type Type { get; } + public abstract TResult Accept(ITypeVisitor builder, TState state); + + [RequiresUnreferencedCode("Uses Type.MakeGenericType to instantiate a generic TypeWitness using reflection.")] + [RequiresDynamicCode("Uses Type.MakeGenericType to instantiate a generic TypeWitness using reflection.")] + public static TypeWitness Create(Type type) => (TypeWitness)Activator.CreateInstance(typeof(TypeWitness<>).MakeGenericType(type))!; + } + + internal sealed class TypeWitness : TypeWitness + { + public override Type Type => typeof(T); + public override TResult Accept(ITypeVisitor builder, TState state) + => builder.Visit(state); + } + + /// + /// Given a input, visits the generic parameter of a + /// and returns a output. + /// + internal interface ITypeVisitor + { + TResult Visit(TState state); + } + + internal static class TypeVisitorExtensions + { + [RequiresUnreferencedCode("Uses Type.MakeGenericType to instantiate a generic TypeWitness using reflection.")] + [RequiresDynamicCode("Uses Type.MakeGenericType to instantiate a generic TypeWitness using reflection.")] + public static TResult Visit(this ITypeVisitor visitor, Type type, TState state) + => TypeWitness.Create(type).Accept(visitor, state); + } +} From 02eb3de85ad0b426ee2fe292a2a34b7df053c9c2 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 18 Jul 2022 15:47:16 +0100 Subject: [PATCH 2/2] Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs --- .../src/System/Text/Json/Serialization/Metadata/TypeWitness.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs index 649cfe349dc89..e426493e05509 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/TypeWitness.cs @@ -8,7 +8,7 @@ namespace System.Text.Json.Reflection /// /// Encapsulates a generic type parameter that can be unpacked on-demand using a generic visitor, /// encoding an existential type. Can use reflection to instantiate type parameters that are not known at compile time. - /// Uses concepts taken from https://www.microsoft.com/en-us/research/publication/generalized-algebraic-data-types-and-object-oriented-programming/ + /// Uses concepts taken from https://www.microsoft.com/research/publication/generalized-algebraic-data-types-and-object-oriented-programming/ /// internal abstract class TypeWitness {