Cache parameterized ctor delegates in class info rather than converter (

#34248) (#34353)

* Cache parameterized ctor delegates in class info rather than converter

* Address review feedback - move delegate assignment to start of deserialization

* Address review feedback - nullability

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Large.cs

@@ -12,45 +12,47 @@ namespace System.Text.Json.Serialization.Converters
     /// </summary>
     internal sealed class LargeObjectWithParameterizedConstructorConverter<T> : ObjectWithParameterizedConstructorConverter<T> where T : notnull
     {
-        private JsonClassInfo.ParameterizedConstructorDelegate<T>? _createObject;
-
-        internal override void CreateConstructorDelegate(JsonSerializerOptions options)
-        {
-            _createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo)!;
-        }
-
         protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)
         {
             bool success = jsonParameterInfo.ReadJson(ref state, ref reader, out object? arg0);
 
             if (success)
             {
-                ((object[])state.Current.CtorArgumentState!.Arguments!)[jsonParameterInfo.Position] = arg0!;
+                ((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.Position] = arg0!;
             }
 
             return success;
         }
 
         protected override object CreateObject(ref ReadStackFrame frame)
         {
-            object[] arguments = (object[])frame.CtorArgumentState!.Arguments!;
+            object[] arguments = (object[])frame.CtorArgumentState!.Arguments;
+
+            var createObject = (JsonClassInfo.ParameterizedConstructorDelegate<T>?)frame.JsonClassInfo.CreateObjectWithParameterizedCtor;
 
-            if (_createObject == null)
+            if (createObject == null)
             {
                 // This means this constructor has more than 64 parameters.
-                ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo, TypeToConvert);
+                ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo!, TypeToConvert);
             }
 
-            object obj = _createObject(arguments)!;
+            object obj = createObject(arguments);
 
             ArrayPool<object>.Shared.Return(arguments, clearArray: true);
             return obj;
         }
 
         protected override void InitializeConstructorArgumentCaches(ref ReadStack state, JsonSerializerOptions options)
         {
-            object[] arguments = ArrayPool<object>.Shared.Rent(state.Current.JsonClassInfo.ParameterCount);
-            foreach (JsonParameterInfo jsonParameterInfo in state.Current.JsonClassInfo.ParameterCache!.Values)
+            JsonClassInfo classInfo = state.Current.JsonClassInfo;
+
+            if (classInfo.CreateObjectWithParameterizedCtor == null)
+            {
+                classInfo.CreateObjectWithParameterizedCtor = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo!);
+            }
+
+            object[] arguments = ArrayPool<object>.Shared.Rent(classInfo.ParameterCount);
+            foreach (JsonParameterInfo jsonParameterInfo in classInfo.ParameterCache!.Values)
             {
                 if (jsonParameterInfo.ShouldDeserialize)
                 {

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.Small.cs

@@ -12,17 +12,12 @@ namespace System.Text.Json.Serialization.Converters
     /// </summary>
     internal sealed class SmallObjectWithParameterizedConstructorConverter<T, TArg0, TArg1, TArg2, TArg3> : ObjectWithParameterizedConstructorConverter<T> where T : notnull
     {
-        private JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>? _createObject;
-
-        internal override void CreateConstructorDelegate(JsonSerializerOptions options)
-        {
-            _createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo)!;
-        }
-
         protected override object CreateObject(ref ReadStackFrame frame)
         {
-            var arguments = (Arguments<TArg0, TArg1, TArg2, TArg3>)frame.CtorArgumentState!.Arguments!;
-            return _createObject!(arguments.Arg0, arguments.Arg1, arguments.Arg2, arguments.Arg3)!;
+            var createObject = (JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>)
+                frame.JsonClassInfo.CreateObjectWithParameterizedCtor!;
+            var arguments = (Arguments<TArg0, TArg1, TArg2, TArg3>)frame.CtorArgumentState!.Arguments;
+            return createObject!(arguments.Arg0, arguments.Arg1, arguments.Arg2, arguments.Arg3);
         }
 
         protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)
@@ -72,9 +67,17 @@ protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref
 
         protected override void InitializeConstructorArgumentCaches(ref ReadStack state, JsonSerializerOptions options)
         {
+            JsonClassInfo classInfo = state.Current.JsonClassInfo;
+
+            if (classInfo.CreateObjectWithParameterizedCtor == null)
+            {
+                classInfo.CreateObjectWithParameterizedCtor =
+                    options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo!);
+            }
+
             var arguments = new Arguments<TArg0, TArg1, TArg2, TArg3>();
 
-            foreach (JsonParameterInfo parameterInfo in state.Current.JsonClassInfo.ParameterCache!.Values)
+            foreach (JsonParameterInfo parameterInfo in classInfo.ParameterCache!.Values)
             {
                 if (parameterInfo.ShouldDeserialize)
                 {

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectWithParameterizedConstructorConverter.cs

@@ -429,7 +429,7 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
 
             if (state.Current.JsonClassInfo.ParameterCount != state.Current.JsonClassInfo.ParameterCache!.Count)
             {
-                ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(ConstructorInfo, TypeToConvert);
+                ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(ConstructorInfo!, TypeToConvert);
             }
 
             // Set current JsonPropertyInfo to null to avoid conflicts on push.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonClassInfo.cs

@@ -20,6 +20,8 @@ internal sealed partial class JsonClassInfo
 
         public ConstructorDelegate? CreateObject { get; private set; }
 
+        public object? CreateObjectWithParameterizedCtor { get; set; }
+
         public ClassType ClassType { get; private set; }
 
         public JsonPropertyInfo? DataExtensionProperty { get; private set; }
@@ -159,8 +161,7 @@ public JsonClassInfo(Type type, JsonSerializerOptions options)
 
                         if (converter.ConstructorIsParameterized)
                         {
-                            converter.CreateConstructorDelegate(options);
-                            InitializeConstructorParameters(converter.ConstructorInfo);
+                            InitializeConstructorParameters(converter.ConstructorInfo!);
                         }
                     }
                     break;

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs

@@ -78,8 +78,6 @@ internal bool ShouldFlush(Utf8JsonWriter writer, ref WriteStack state)
         // Whether a type (ClassType.Object) is deserialized using a parameterized constructor.
         internal virtual bool ConstructorIsParameterized => false;
 
-        internal ConstructorInfo ConstructorInfo { get; set; } = null!;
-
-        internal virtual void CreateConstructorDelegate(JsonSerializerOptions options) { }
+        internal ConstructorInfo? ConstructorInfo { get; set; }
     }
 }

src/libraries/System.Text.Json/tests/Serialization/ConstructorTests.Cache.cs

@@ -24,53 +24,66 @@ public void MultipleThreadsLooping()
         [Fact]
         public void MultipleThreads()
         {
-            // Use local options to avoid obtaining already cached metadata from the default options.
-            var options = new JsonSerializerOptions();
-
             // Verify the test class has >32 properties since that is a threshold for using the fallback dictionary.
             Assert.True(typeof(ClassWithConstructor_SimpleAndComplexParameters).GetProperties(BindingFlags.Instance | BindingFlags.Public).Length > 32);
 
-            void DeserializeObjectMinimal()
+            void DeserializeObject(string json, Type type, JsonSerializerOptions options)
+            {
+                var obj = Serializer.Deserialize(json, type, options);
+                ((ITestClassWithParameterizedCtor)obj).Verify();
+            }
+
+            void DeserializeObjectMinimal(Type type, JsonSerializerOptions options)
             {
-                var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(@"{""MyDecimal"" : 3.3}", options);
+                string json = (string)type.GetProperty("s_json_minimal").GetValue(null);
+                var obj = Serializer.Deserialize(json, type, options);
+                ((ITestClassWithParameterizedCtor)obj).VerifyMinimal();
             };
 
-            void DeserializeObjectFlipped()
+            void DeserializeObjectFlipped(Type type, JsonSerializerOptions options)
             {
-                var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(
-                    ClassWithConstructor_SimpleAndComplexParameters.s_json_flipped, options);
-                obj.Verify();
+                string json = (string)type.GetProperty("s_json_flipped").GetValue(null);
+                DeserializeObject(json, type, options);
             };
 
-            void DeserializeObjectNormal()
+            void DeserializeObjectNormal(Type type, JsonSerializerOptions options)
             {
-                var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(
-                    ClassWithConstructor_SimpleAndComplexParameters.s_json, options);
-                obj.Verify();
+                string json = (string)type.GetProperty("s_json").GetValue(null);
+                DeserializeObject(json, type, options);
             };
 
-            void SerializeObject()
+            void SerializeObject(Type type, JsonSerializerOptions options)
             {
                 var obj = ClassWithConstructor_SimpleAndComplexParameters.GetInstance();
                 JsonSerializer.Serialize(obj, options);
             };
 
-            const int ThreadCount = 8;
-            const int ConcurrentTestsCount = 4;
-            Task[] tasks = new Task[ThreadCount * ConcurrentTestsCount];
-
-            for (int i = 0; i < tasks.Length; i += ConcurrentTestsCount)
+            void RunTest(Type type)
             {
-                // Create race condition to populate the sorted property cache with different json ordering.
-                tasks[i + 0] = Task.Run(() => DeserializeObjectMinimal());
-                tasks[i + 1] = Task.Run(() => DeserializeObjectFlipped());
-                tasks[i + 2] = Task.Run(() => DeserializeObjectNormal());
+                // Use local options to avoid obtaining already cached metadata from the default options.
+                var options = new JsonSerializerOptions();
 
-                // Ensure no exceptions on serialization
-                tasks[i + 3] = Task.Run(() => SerializeObject());
-            };
+                const int ThreadCount = 8;
+                const int ConcurrentTestsCount = 4;
+                Task[] tasks = new Task[ThreadCount * ConcurrentTestsCount];
+
+                for (int i = 0; i < tasks.Length; i += ConcurrentTestsCount)
+                {
+                    // Create race condition to populate the sorted property cache with different json ordering.
+                    tasks[i + 0] = Task.Run(() => DeserializeObjectMinimal(type, options));
+                    tasks[i + 1] = Task.Run(() => DeserializeObjectFlipped(type, options));
+                    tasks[i + 2] = Task.Run(() => DeserializeObjectNormal(type, options));
+
+                    // Ensure no exceptions on serialization
+                    tasks[i + 3] = Task.Run(() => SerializeObject(type, options));
+                };
+
+                Task.WaitAll(tasks);
+            }
 
-            Task.WaitAll(tasks);
+            RunTest(typeof(ClassWithConstructor_SimpleAndComplexParameters));
+            RunTest(typeof(Person_Class));
+            RunTest(typeof(Parameterized_Class_With_ComplexTuple));
         }
 
         [Fact]

src/libraries/System.Text.Json/tests/Serialization/ConstructorTests.ParameterMatching.cs

@@ -8,14 +8,14 @@
 
 namespace System.Text.Json.Serialization.Tests
 {
-    public class ConstructorTests_StringTValue : ConstructorTests
+    public class ConstructorTests_String : ConstructorTests
     {
-        public ConstructorTests_StringTValue() : base(DeserializationWrapper.StringTValueSerializer) { }
+        public ConstructorTests_String() : base(DeserializationWrapper.StringDeserializer) { }
     }
 
-    public class ConstructorTests_StreamTValue : ConstructorTests
+    public class ConstructorTests_Stream : ConstructorTests
     {
-        public ConstructorTests_StreamTValue() : base(DeserializationWrapper.StreamTValueSerializer) { }
+        public ConstructorTests_Stream() : base(DeserializationWrapper.StreamDeserializer) { }
     }
 
     public abstract partial class ConstructorTests
@@ -301,7 +301,6 @@ public void Null_AsArgument_To_ParameterThat_CanNotBeNull()
             Assert.Throws<JsonException>(() => Serializer.Deserialize<ClassWrapper_For_Int_Point_3D_String>(@"{""MyPoint3DStruct"":null,""MyString"":""1""}"));
         }
 
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/33928")]
         [Fact]
         public void OtherPropertiesAreSet()
         {

src/libraries/System.Text.Json/tests/Serialization/DeserializationWrapper.cs

@@ -14,20 +14,27 @@ public abstract class DeserializationWrapper
     {
         private static readonly JsonSerializerOptions _optionsWithSmallBuffer = new JsonSerializerOptions { DefaultBufferSize = 1 };
 
-        public static DeserializationWrapper StringTValueSerializer => new StringTValueSerializerWrapper();
-        public static DeserializationWrapper StreamTValueSerializer => new StreamTValueSerializerWrapper();
+        public static DeserializationWrapper StringDeserializer => new StringDeserializerWrapper();
+        public static DeserializationWrapper StreamDeserializer => new StreamDeserializerWrapper();
 
         protected internal abstract T Deserialize<T>(string json, JsonSerializerOptions options = null);
 
-        private class StringTValueSerializerWrapper : DeserializationWrapper
+        protected internal abstract object Deserialize(string json, Type type, JsonSerializerOptions options = null);
+
+        private class StringDeserializerWrapper : DeserializationWrapper
         {
             protected internal override T Deserialize<T>(string json, JsonSerializerOptions options = null)
             {
                 return JsonSerializer.Deserialize<T>(json, options);
             }
+
+            protected internal override object Deserialize(string json, Type type, JsonSerializerOptions options = null)
+            {
+                return JsonSerializer.Deserialize(json, type, options);
+            }
         }
 
-        private class StreamTValueSerializerWrapper : DeserializationWrapper
+        private class StreamDeserializerWrapper : DeserializationWrapper
         {
             protected internal override T Deserialize<T>(string json, JsonSerializerOptions options = null)
             {
@@ -44,6 +51,22 @@ protected internal override T Deserialize<T>(string json, JsonSerializerOptions
                     }
                 }).GetAwaiter().GetResult();
             }
+
+            protected internal override object Deserialize(string json, Type type, JsonSerializerOptions options = null)
+            {
+                if (options == null)
+                {
+                    options = _optionsWithSmallBuffer;
+                }
+
+                return Task.Run(async () =>
+                {
+                    using (MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes(json)))
+                    {
+                        return await JsonSerializer.DeserializeAsync(stream, type, options);
+                    }
+                }).GetAwaiter().GetResult();
+            }
         }
     }
 }