Simplify the JsonTypeInfo.CreateObject implemenetation (#2)

* Simplify the JsonTypeInfo.CreateObject implemenetation * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com> Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>
eiriktsarpalis · May 31, 2022 · 77514cb · 77514cb
1 parent 055ce33
commit 77514cb
Show file tree

Hide file tree

Showing 17 changed files with 59 additions and 47 deletions.
diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/ICollectionOfTConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/ICollectionOfTConverter.cs
@@ -38,10 +38,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as List<T> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new List<TElement>();
+                jsonTypeInfo.CreateObject = () => new List<TElement>();
             }
         }
     }

diff --git a/...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/IDictionaryConverter.cs b/...ext.Json/src/System/Text/Json/Serialization/Converters/Collection/IDictionaryConverter.cs
@@ -98,10 +98,10 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TDictionar
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as Dictionary<TKey,TValue> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<string, object?>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<string, object?>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new Dictionary<string, object?>();
+                jsonTypeInfo.CreateObject = () => new Dictionary<string, object?>();
             }
         }
     }

diff --git a/.../System/Text/Json/Serialization/Converters/Collection/IDictionaryOfTKeyTValueConverter.cs b/.../System/Text/Json/Serialization/Converters/Collection/IDictionaryOfTKeyTValueConverter.cs
@@ -40,10 +40,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as Dictionary<TKey,TValue> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new Dictionary<TKey, TValue>();
+                jsonTypeInfo.CreateObject = () => new Dictionary<TKey, TValue>();
             }
         }
     }

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IListConverter.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IListConverter.cs
@@ -77,10 +77,10 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as List<object?> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<object?>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<object?>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new List<object?>();
+                jsonTypeInfo.CreateObject = () => new List<object?>();
             }
         }
     }

diff --git a/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IListOfTConverter.cs b/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/IListOfTConverter.cs
@@ -38,10 +38,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as List<T> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new List<TElement>();
+                jsonTypeInfo.CreateObject = () => new List<TElement>();
             }
         }
     }

diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ISetOfTConverter.cs b/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ISetOfTConverter.cs
@@ -35,10 +35,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
             // Deserialize as HashSet<TElement> for interface types that support it.
-            if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(HashSet<TElement>)))
+            if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(HashSet<TElement>)))
             {
                 Debug.Assert(TypeToConvert.IsInterface);
-                jsonTypeInfo.UntypedCreateObject = () => new HashSet<TElement>();
+                jsonTypeInfo.CreateObject = () => new HashSet<TElement>();
             }
         }
     }

diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonCollectionConverter.cs
@@ -26,7 +26,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
         {
             JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;
 
-            if (typeInfo.UntypedCreateObject is null)
+            if (typeInfo.CreateObject is null)
             {
                 // The contract model was not able to produce a default constructor for two possible reasons:
                 // 1. Either the declared collection type is abstract and cannot be instantiated.
@@ -41,7 +41,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
                 }
             }
 
-            state.Current.ReturnValue = typeInfo.UntypedCreateObject();
+            state.Current.ReturnValue = typeInfo.CreateObject();
             Debug.Assert(state.Current.ReturnValue is TCollection);
         }
 

diff --git a/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs b/....Json/src/System/Text/Json/Serialization/Converters/Collection/JsonDictionaryConverter.cs
@@ -41,7 +41,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
         {
             JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;
 
-            if (typeInfo.UntypedCreateObject is null)
+            if (typeInfo.CreateObject is null)
             {
                 // The contract model was not able to produce a default constructor for two possible reasons:
                 // 1. Either the declared collection type is abstract and cannot be instantiated.
@@ -56,7 +56,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
                 }
             }
 
-            state.Current.ReturnValue = typeInfo.UntypedCreateObject()!;
+            state.Current.ReturnValue = typeInfo.CreateObject()!;
             Debug.Assert(state.Current.ReturnValue is TDictionary);
         }
 

diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ListOfTConverter.cs b/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/ListOfTConverter.cs
@@ -17,12 +17,12 @@ protected override void Add(in TElement value, ref ReadStack state)
 
         protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
         {
-            if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
+            if (state.Current.JsonTypeInfo.CreateObject == null)
             {
                 ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
             }
 
-            state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
+            state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
         }
 
         protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, ref WriteStack state)

diff --git a/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/QueueOfTConverter.cs b/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/QueueOfTConverter.cs
@@ -16,12 +16,12 @@ protected override void Add(in TElement value, ref ReadStack state)
 
         protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
         {
-            if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
+            if (state.Current.JsonTypeInfo.CreateObject == null)
             {
                 ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
             }
 
-            state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
+            state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
         }
     }
 }
diff --git a/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/StackOfTConverter.cs b/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Collection/StackOfTConverter.cs
@@ -17,12 +17,12 @@ protected override void Add(in TElement value, ref ReadStack state)
 
         protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
         {
-            if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
+            if (state.Current.JsonTypeInfo.CreateObject == null)
             {
                 ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
             }
 
-            state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
+            state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
         }
     }
 }
diff --git a/...xt.Json/src/System/Text/Json/Serialization/Converters/Collection/StackOrQueueConverter.cs b/...xt.Json/src/System/Text/Json/Serialization/Converters/Collection/StackOrQueueConverter.cs
@@ -21,7 +21,7 @@ protected sealed override void Add(in object? value, ref ReadStack state)
         protected sealed override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
         {
             JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;
-            Func<object>? constructorDelegate = typeInfo.UntypedCreateObject;
+            Func<object>? constructorDelegate = typeInfo.CreateObject;
 
             if (constructorDelegate == null)
             {

diff --git a/....Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs b/....Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectDefaultConverter.cs
@@ -31,12 +31,12 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
                     ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypeToConvert);
                 }
 
-                if (jsonTypeInfo.UntypedCreateObject == null)
+                if (jsonTypeInfo.CreateObject == null)
                 {
                     ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
                 }
 
-                obj = jsonTypeInfo.UntypedCreateObject()!;
+                obj = jsonTypeInfo.CreateObject()!;
 
                 if (obj is IJsonOnDeserializing onDeserializing)
                 {
@@ -127,12 +127,12 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
                         return true;
                     }
 
-                    if (jsonTypeInfo.UntypedCreateObject == null)
+                    if (jsonTypeInfo.CreateObject == null)
                     {
                         ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
                     }
 
-                    obj = jsonTypeInfo.UntypedCreateObject()!;
+                    obj = jsonTypeInfo.CreateObject()!;
 
                     if (state.Current.MetadataPropertyNames.HasFlag(MetadataPropertyName.Id))
                     {

diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs b/...em.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandlePropertyName.cs
@@ -121,7 +121,7 @@ internal static void CreateDataExtensionProperty(
                     genericArgs[1].UnderlyingSystemType == typeof(JsonElement) ||
                     genericArgs[1].UnderlyingSystemType == typeof(Nodes.JsonNode));
 #endif
-                if (jsonPropertyInfo.JsonTypeInfo.UntypedCreateObject == null)
+                if (jsonPropertyInfo.JsonTypeInfo.CreateObject == null)
                 {
                     // Avoid a reference to the JsonNode type for trimming
                     if (jsonPropertyInfo.PropertyType.FullName == JsonTypeInfo.JsonObjectTypeName)
@@ -135,7 +135,7 @@ internal static void CreateDataExtensionProperty(
                 }
                 else
                 {
-                    extensionData = jsonPropertyInfo.JsonTypeInfo.UntypedCreateObject();
+                    extensionData = jsonPropertyInfo.JsonTypeInfo.CreateObject();
                 }
 
                 jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, extensionData);

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
@@ -28,10 +28,16 @@ public abstract partial class JsonTypeInfo
         /// </summary>
         public Func<object>? CreateObject
         {
-            get => UntypedCreateObjectAbstract;
-            set => UntypedCreateObjectAbstract = value;
+            get => _createObject;
+            set
+            {
+                SetCreateObject(value);
+            }
         }
 
+        private protected abstract void SetCreateObject(Delegate? createObject);
+        private protected Func<object>? _createObject;
+
         /// <summary>
         /// Gets JsonPropertyInfo list. Only applicable when Kind is Object.
         /// </summary>
@@ -69,13 +75,6 @@ public IList<JsonPropertyInfo> Properties
             }
         }
 
-        // Untyped CreateObject is non-virtual public API so we pretend it's virtual by using this indirection
-        // We need it to be abstract so that we can keep typed value in sync
-        internal abstract Func<object>? UntypedCreateObjectAbstract { get; set; }
-
-        // Actual value of UntypedCreateObject, for perf it's non-virtual
-        internal Func<object>? UntypedCreateObject { get; set; }
-
         internal object? CreateObjectWithArgs { get; set; }
 
         // Add method delegate for non-generic Stack and Queue; and types that derive from them.

diff --git a/...libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs b/...libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs
@@ -21,23 +21,36 @@ public abstract class JsonTypeInfo<T> : JsonTypeInfo
         /// </summary>
         public new Func<T>? CreateObject
         {
-            get => _typedCreateObject ??= UntypedCreateObject == null ? null : () => (T)UntypedCreateObject();
+            get => _typedCreateObject;
             set
             {
-                _typedCreateObject = value;
-                UntypedCreateObject = value == null ? null : () => value()!;
+                SetCreateObject(value);
             }
         }
 
-        internal override Func<object>? UntypedCreateObjectAbstract
+        private protected override void SetCreateObject(Delegate? createObject)
         {
-            get => UntypedCreateObject;
-            set
+            Debug.Assert(createObject is null or Func<object> or Func<T>);
+
+            if (createObject is null)
             {
-                UntypedCreateObject = value;
-                // We invalidate the cached typed value
+                _createObject = null;
                 _typedCreateObject = null;
+                return;
             }
+
+            if (createObject is Func<object> untypedDelegate)
+            {
+                _createObject = untypedDelegate;
+                _typedCreateObject = () => (T)untypedDelegate();
+                return;
+            }
+
+            Debug.Assert(createObject is Func<T>);
+
+            Func<T> typedDelegate = (Func<T>)createObject;
+            _createObject = () => typedDelegate()!;
+            _typedCreateObject = typedDelegate;
         }
 
         internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)

diff --git a/...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs b/...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs
@@ -40,7 +40,7 @@ internal ReflectionJsonTypeInfo(JsonConverter converter, JsonSerializerOptions o
                 AddPropertiesAndParametersUsingReflection();
             }
 
-            UntypedCreateObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
+            ((JsonTypeInfo)this).CreateObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
         }
 
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",