Cache parameterized ctor delegates in class info rather than converter

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

@@ -12,11 +12,9 @@ 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)
+        internal override void CreateConstructorDelegate(JsonClassInfo classInfo, JsonSerializerOptions options)
         {
-            _createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo)!;
+            classInfo.CreateObjectWithParameterizedCtor = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo)!;
         }
 
         protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)
@@ -35,13 +33,15 @@ protected override object CreateObject(ref ReadStackFrame frame)
         {
             object[] arguments = (object[])frame.CtorArgumentState!.Arguments!;
 
-            if (_createObject == null)
+            var createObject = (JsonClassInfo.ParameterizedConstructorDelegate<T>)frame.JsonClassInfo.CreateObjectWithParameterizedCtor!;
+
+            if (createObject == null)
             {
                 // This means this constructor has more than 64 parameters.
                 ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo, TypeToConvert);
             }
 
-            object obj = _createObject(arguments)!;
+            object obj = createObject(arguments)!;
 
             ArrayPool<object>.Shared.Return(arguments, clearArray: true);
             return obj;

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

@@ -12,17 +12,17 @@ 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)
+        internal override void CreateConstructorDelegate(JsonClassInfo classInfo, JsonSerializerOptions options)
         {
-            _createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo)!;
+            classInfo.CreateObjectWithParameterizedCtor = options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo)!;
         }
 
         protected override object CreateObject(ref ReadStackFrame frame)
         {
+            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)!;
+            return createObject!(arguments.Arg0, arguments.Arg1, arguments.Arg2, arguments.Arg3)!;
         }
 
         protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)

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,7 +161,13 @@ public JsonClassInfo(Type type, JsonSerializerOptions options)
 
                         if (converter.ConstructorIsParameterized)
                         {
-                            converter.CreateConstructorDelegate(options);
+                            // Create and cache the constructor delegate for this type.
+                            // We depend on the first converter created for this type to perform the initialization.
+                            // Subsequent instances of the converter will not perform this initialization since we will
+                            // not go down this code path (as the type will already be in the JsonClassInfo cache).
+                            // The converter is used because it has generic type support.
+                            converter.CreateConstructorDelegate(this, options);
+
                             InitializeConstructorParameters(converter.ConstructorInfo);
                         }
                     }

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

@@ -80,6 +80,6 @@ internal bool ShouldFlush(Utf8JsonWriter writer, ref WriteStack state)
 
         internal ConstructorInfo ConstructorInfo { get; set; } = null!;
 
-        internal virtual void CreateConstructorDelegate(JsonSerializerOptions options) { }
+        internal virtual void CreateConstructorDelegate(JsonClassInfo classInfo, JsonSerializerOptions options) { }
     }
 }

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();
+            }
         }
     }
 }