Skip to content

Commit 8f6b899

Browse files
authored
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
1 parent 54ee8af commit 8f6b899

File tree

9 files changed

+180
-84
lines changed

9 files changed

+180
-84
lines changed

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,45 +12,47 @@ namespace System.Text.Json.Serialization.Converters
1212
/// </summary>
1313
internal sealed class LargeObjectWithParameterizedConstructorConverter<T> : ObjectWithParameterizedConstructorConverter<T> where T : notnull
1414
{
15-
private JsonClassInfo.ParameterizedConstructorDelegate<T>? _createObject;
16-
17-
internal override void CreateConstructorDelegate(JsonSerializerOptions options)
18-
{
19-
_createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo)!;
20-
}
21-
2215
protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)
2316
{
2417
bool success = jsonParameterInfo.ReadJson(ref state, ref reader, out object? arg0);
2518

2619
if (success)
2720
{
28-
((object[])state.Current.CtorArgumentState!.Arguments!)[jsonParameterInfo.Position] = arg0!;
21+
((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.Position] = arg0!;
2922
}
3023

3124
return success;
3225
}
3326

3427
protected override object CreateObject(ref ReadStackFrame frame)
3528
{
36-
object[] arguments = (object[])frame.CtorArgumentState!.Arguments!;
29+
object[] arguments = (object[])frame.CtorArgumentState!.Arguments;
30+
31+
var createObject = (JsonClassInfo.ParameterizedConstructorDelegate<T>?)frame.JsonClassInfo.CreateObjectWithParameterizedCtor;
3732

38-
if (_createObject == null)
33+
if (createObject == null)
3934
{
4035
// This means this constructor has more than 64 parameters.
41-
ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo, TypeToConvert);
36+
ThrowHelper.ThrowNotSupportedException_ConstructorMaxOf64Parameters(ConstructorInfo!, TypeToConvert);
4237
}
4338

44-
object obj = _createObject(arguments)!;
39+
object obj = createObject(arguments);
4540

4641
ArrayPool<object>.Shared.Return(arguments, clearArray: true);
4742
return obj;
4843
}
4944

5045
protected override void InitializeConstructorArgumentCaches(ref ReadStack state, JsonSerializerOptions options)
5146
{
52-
object[] arguments = ArrayPool<object>.Shared.Rent(state.Current.JsonClassInfo.ParameterCount);
53-
foreach (JsonParameterInfo jsonParameterInfo in state.Current.JsonClassInfo.ParameterCache!.Values)
47+
JsonClassInfo classInfo = state.Current.JsonClassInfo;
48+
49+
if (classInfo.CreateObjectWithParameterizedCtor == null)
50+
{
51+
classInfo.CreateObjectWithParameterizedCtor = options.MemberAccessorStrategy.CreateParameterizedConstructor<T>(ConstructorInfo!);
52+
}
53+
54+
object[] arguments = ArrayPool<object>.Shared.Rent(classInfo.ParameterCount);
55+
foreach (JsonParameterInfo jsonParameterInfo in classInfo.ParameterCache!.Values)
5456
{
5557
if (jsonParameterInfo.ShouldDeserialize)
5658
{

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,12 @@ namespace System.Text.Json.Serialization.Converters
1212
/// </summary>
1313
internal sealed class SmallObjectWithParameterizedConstructorConverter<T, TArg0, TArg1, TArg2, TArg3> : ObjectWithParameterizedConstructorConverter<T> where T : notnull
1414
{
15-
private JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>? _createObject;
16-
17-
internal override void CreateConstructorDelegate(JsonSerializerOptions options)
18-
{
19-
_createObject = options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo)!;
20-
}
21-
2215
protected override object CreateObject(ref ReadStackFrame frame)
2316
{
24-
var arguments = (Arguments<TArg0, TArg1, TArg2, TArg3>)frame.CtorArgumentState!.Arguments!;
25-
return _createObject!(arguments.Arg0, arguments.Arg1, arguments.Arg2, arguments.Arg3)!;
17+
var createObject = (JsonClassInfo.ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>)
18+
frame.JsonClassInfo.CreateObjectWithParameterizedCtor!;
19+
var arguments = (Arguments<TArg0, TArg1, TArg2, TArg3>)frame.CtorArgumentState!.Arguments;
20+
return createObject!(arguments.Arg0, arguments.Arg1, arguments.Arg2, arguments.Arg3);
2621
}
2722

2823
protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref Utf8JsonReader reader, JsonParameterInfo jsonParameterInfo)
@@ -72,9 +67,17 @@ protected override bool ReadAndCacheConstructorArgument(ref ReadStack state, ref
7267

7368
protected override void InitializeConstructorArgumentCaches(ref ReadStack state, JsonSerializerOptions options)
7469
{
70+
JsonClassInfo classInfo = state.Current.JsonClassInfo;
71+
72+
if (classInfo.CreateObjectWithParameterizedCtor == null)
73+
{
74+
classInfo.CreateObjectWithParameterizedCtor =
75+
options.MemberAccessorStrategy.CreateParameterizedConstructor<T, TArg0, TArg1, TArg2, TArg3>(ConstructorInfo!);
76+
}
77+
7578
var arguments = new Arguments<TArg0, TArg1, TArg2, TArg3>();
7679

77-
foreach (JsonParameterInfo parameterInfo in state.Current.JsonClassInfo.ParameterCache!.Values)
80+
foreach (JsonParameterInfo parameterInfo in classInfo.ParameterCache!.Values)
7881
{
7982
if (parameterInfo.ShouldDeserialize)
8083
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
429429

430430
if (state.Current.JsonClassInfo.ParameterCount != state.Current.JsonClassInfo.ParameterCache!.Count)
431431
{
432-
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(ConstructorInfo, TypeToConvert);
432+
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(ConstructorInfo!, TypeToConvert);
433433
}
434434

435435
// Set current JsonPropertyInfo to null to avoid conflicts on push.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ internal sealed partial class JsonClassInfo
2020

2121
public ConstructorDelegate? CreateObject { get; private set; }
2222

23+
public object? CreateObjectWithParameterizedCtor { get; set; }
24+
2325
public ClassType ClassType { get; private set; }
2426

2527
public JsonPropertyInfo? DataExtensionProperty { get; private set; }
@@ -159,8 +161,7 @@ public JsonClassInfo(Type type, JsonSerializerOptions options)
159161

160162
if (converter.ConstructorIsParameterized)
161163
{
162-
converter.CreateConstructorDelegate(options);
163-
InitializeConstructorParameters(converter.ConstructorInfo);
164+
InitializeConstructorParameters(converter.ConstructorInfo!);
164165
}
165166
}
166167
break;

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,6 @@ internal bool ShouldFlush(Utf8JsonWriter writer, ref WriteStack state)
7878
// Whether a type (ClassType.Object) is deserialized using a parameterized constructor.
7979
internal virtual bool ConstructorIsParameterized => false;
8080

81-
internal ConstructorInfo ConstructorInfo { get; set; } = null!;
82-
83-
internal virtual void CreateConstructorDelegate(JsonSerializerOptions options) { }
81+
internal ConstructorInfo? ConstructorInfo { get; set; }
8482
}
8583
}

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

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,53 +24,66 @@ public void MultipleThreadsLooping()
2424
[Fact]
2525
public void MultipleThreads()
2626
{
27-
// Use local options to avoid obtaining already cached metadata from the default options.
28-
var options = new JsonSerializerOptions();
29-
3027
// Verify the test class has >32 properties since that is a threshold for using the fallback dictionary.
3128
Assert.True(typeof(ClassWithConstructor_SimpleAndComplexParameters).GetProperties(BindingFlags.Instance | BindingFlags.Public).Length > 32);
3229

33-
void DeserializeObjectMinimal()
30+
void DeserializeObject(string json, Type type, JsonSerializerOptions options)
31+
{
32+
var obj = Serializer.Deserialize(json, type, options);
33+
((ITestClassWithParameterizedCtor)obj).Verify();
34+
}
35+
36+
void DeserializeObjectMinimal(Type type, JsonSerializerOptions options)
3437
{
35-
var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(@"{""MyDecimal"" : 3.3}", options);
38+
string json = (string)type.GetProperty("s_json_minimal").GetValue(null);
39+
var obj = Serializer.Deserialize(json, type, options);
40+
((ITestClassWithParameterizedCtor)obj).VerifyMinimal();
3641
};
3742

38-
void DeserializeObjectFlipped()
43+
void DeserializeObjectFlipped(Type type, JsonSerializerOptions options)
3944
{
40-
var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(
41-
ClassWithConstructor_SimpleAndComplexParameters.s_json_flipped, options);
42-
obj.Verify();
45+
string json = (string)type.GetProperty("s_json_flipped").GetValue(null);
46+
DeserializeObject(json, type, options);
4347
};
4448

45-
void DeserializeObjectNormal()
49+
void DeserializeObjectNormal(Type type, JsonSerializerOptions options)
4650
{
47-
var obj = Serializer.Deserialize<ClassWithConstructor_SimpleAndComplexParameters>(
48-
ClassWithConstructor_SimpleAndComplexParameters.s_json, options);
49-
obj.Verify();
51+
string json = (string)type.GetProperty("s_json").GetValue(null);
52+
DeserializeObject(json, type, options);
5053
};
5154

52-
void SerializeObject()
55+
void SerializeObject(Type type, JsonSerializerOptions options)
5356
{
5457
var obj = ClassWithConstructor_SimpleAndComplexParameters.GetInstance();
5558
JsonSerializer.Serialize(obj, options);
5659
};
5760

58-
const int ThreadCount = 8;
59-
const int ConcurrentTestsCount = 4;
60-
Task[] tasks = new Task[ThreadCount * ConcurrentTestsCount];
61-
62-
for (int i = 0; i < tasks.Length; i += ConcurrentTestsCount)
61+
void RunTest(Type type)
6362
{
64-
// Create race condition to populate the sorted property cache with different json ordering.
65-
tasks[i + 0] = Task.Run(() => DeserializeObjectMinimal());
66-
tasks[i + 1] = Task.Run(() => DeserializeObjectFlipped());
67-
tasks[i + 2] = Task.Run(() => DeserializeObjectNormal());
63+
// Use local options to avoid obtaining already cached metadata from the default options.
64+
var options = new JsonSerializerOptions();
6865

69-
// Ensure no exceptions on serialization
70-
tasks[i + 3] = Task.Run(() => SerializeObject());
71-
};
66+
const int ThreadCount = 8;
67+
const int ConcurrentTestsCount = 4;
68+
Task[] tasks = new Task[ThreadCount * ConcurrentTestsCount];
69+
70+
for (int i = 0; i < tasks.Length; i += ConcurrentTestsCount)
71+
{
72+
// Create race condition to populate the sorted property cache with different json ordering.
73+
tasks[i + 0] = Task.Run(() => DeserializeObjectMinimal(type, options));
74+
tasks[i + 1] = Task.Run(() => DeserializeObjectFlipped(type, options));
75+
tasks[i + 2] = Task.Run(() => DeserializeObjectNormal(type, options));
76+
77+
// Ensure no exceptions on serialization
78+
tasks[i + 3] = Task.Run(() => SerializeObject(type, options));
79+
};
80+
81+
Task.WaitAll(tasks);
82+
}
7283

73-
Task.WaitAll(tasks);
84+
RunTest(typeof(ClassWithConstructor_SimpleAndComplexParameters));
85+
RunTest(typeof(Person_Class));
86+
RunTest(typeof(Parameterized_Class_With_ComplexTuple));
7487
}
7588

7689
[Fact]

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88

99
namespace System.Text.Json.Serialization.Tests
1010
{
11-
public class ConstructorTests_StringTValue : ConstructorTests
11+
public class ConstructorTests_String : ConstructorTests
1212
{
13-
public ConstructorTests_StringTValue() : base(DeserializationWrapper.StringTValueSerializer) { }
13+
public ConstructorTests_String() : base(DeserializationWrapper.StringDeserializer) { }
1414
}
1515

16-
public class ConstructorTests_StreamTValue : ConstructorTests
16+
public class ConstructorTests_Stream : ConstructorTests
1717
{
18-
public ConstructorTests_StreamTValue() : base(DeserializationWrapper.StreamTValueSerializer) { }
18+
public ConstructorTests_Stream() : base(DeserializationWrapper.StreamDeserializer) { }
1919
}
2020

2121
public abstract partial class ConstructorTests
@@ -301,7 +301,6 @@ public void Null_AsArgument_To_ParameterThat_CanNotBeNull()
301301
Assert.Throws<JsonException>(() => Serializer.Deserialize<ClassWrapper_For_Int_Point_3D_String>(@"{""MyPoint3DStruct"":null,""MyString"":""1""}"));
302302
}
303303

304-
[ActiveIssue("https://github.com/dotnet/runtime/issues/33928")]
305304
[Fact]
306305
public void OtherPropertiesAreSet()
307306
{

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,27 @@ public abstract class DeserializationWrapper
1414
{
1515
private static readonly JsonSerializerOptions _optionsWithSmallBuffer = new JsonSerializerOptions { DefaultBufferSize = 1 };
1616

17-
public static DeserializationWrapper StringTValueSerializer => new StringTValueSerializerWrapper();
18-
public static DeserializationWrapper StreamTValueSerializer => new StreamTValueSerializerWrapper();
17+
public static DeserializationWrapper StringDeserializer => new StringDeserializerWrapper();
18+
public static DeserializationWrapper StreamDeserializer => new StreamDeserializerWrapper();
1919

2020
protected internal abstract T Deserialize<T>(string json, JsonSerializerOptions options = null);
2121

22-
private class StringTValueSerializerWrapper : DeserializationWrapper
22+
protected internal abstract object Deserialize(string json, Type type, JsonSerializerOptions options = null);
23+
24+
private class StringDeserializerWrapper : DeserializationWrapper
2325
{
2426
protected internal override T Deserialize<T>(string json, JsonSerializerOptions options = null)
2527
{
2628
return JsonSerializer.Deserialize<T>(json, options);
2729
}
30+
31+
protected internal override object Deserialize(string json, Type type, JsonSerializerOptions options = null)
32+
{
33+
return JsonSerializer.Deserialize(json, type, options);
34+
}
2835
}
2936

30-
private class StreamTValueSerializerWrapper : DeserializationWrapper
37+
private class StreamDeserializerWrapper : DeserializationWrapper
3138
{
3239
protected internal override T Deserialize<T>(string json, JsonSerializerOptions options = null)
3340
{
@@ -44,6 +51,22 @@ protected internal override T Deserialize<T>(string json, JsonSerializerOptions
4451
}
4552
}).GetAwaiter().GetResult();
4653
}
54+
55+
protected internal override object Deserialize(string json, Type type, JsonSerializerOptions options = null)
56+
{
57+
if (options == null)
58+
{
59+
options = _optionsWithSmallBuffer;
60+
}
61+
62+
return Task.Run(async () =>
63+
{
64+
using (MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes(json)))
65+
{
66+
return await JsonSerializer.DeserializeAsync(stream, type, options);
67+
}
68+
}).GetAwaiter().GetResult();
69+
}
4770
}
4871
}
4972
}

0 commit comments

Comments
 (0)