Skip to content

Commit

Permalink
Further JsonPropertyInfo caching cleanup (#67754)
Browse files Browse the repository at this point in the history
  • Loading branch information
@krwq
krwq authored Apr 8, 2022
1 parent eb223d8 commit 20c4bde
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 44 deletions.
48 changes: 31 additions & 17 deletions ...ibraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.ComponentModel;
using System.Diagnostics;
using System.Reflection;
using System.Text.Json.Reflection;

namespace System.Text.Json.Serialization.Metadata
{
Expand Down Expand Up @@ -35,7 +36,7 @@ internal static JsonPropertyInfo GetPropertyPlaceholder()
Debug.Assert(!info.ShouldDeserialize);
Debug.Assert(!info.ShouldSerialize);

info.NameAsString = string.Empty;
info.Name = string.Empty;

return info;
}
Expand Down Expand Up @@ -65,16 +66,27 @@ internal static JsonPropertyInfo CreateIgnoredPropertyPlaceholder(

private bool _isConfigured;

internal void Configure()
internal void EnsureConfigured()
{
if (_isConfigured)
{
return;
}

Configure();

_isConfigured = true;
}

internal virtual void Configure()
{
if (!IsForTypeInfo)
{
CacheNameAsUtf8BytesAndEscapedNameSection();
}

if (IsIgnored)
{
_isConfigured = true;
return;
}

Expand All @@ -84,18 +96,17 @@ internal void Configure()
}
else
{
PropertyTypeCanBeNull = PropertyType.CanBeNull();
DetermineNumberHandlingForProperty();
DetermineIgnoreCondition(IgnoreCondition);
DetermineSerializationCapabilities(IgnoreCondition);
}

_isConfigured = true;
}

internal void GetPolicies(JsonIgnoreCondition? ignoreCondition)
internal void GetPolicies()
{
Debug.Assert(MemberInfo != null);
DetermineSerializationCapabilities(ignoreCondition);
DeterminePropertyName();
DetermineIgnoreCondition(ignoreCondition);

JsonPropertyOrderAttribute? orderAttr = GetAttribute<JsonPropertyOrderAttribute>(MemberInfo);
if (orderAttr != null)
Expand All @@ -122,7 +133,7 @@ private void DeterminePropertyName()
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this);
}

NameAsString = name;
Name = name;
}
else if (Options.PropertyNamingPolicy != null)
{
Expand All @@ -132,16 +143,19 @@ private void DeterminePropertyName()
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this);
}

NameAsString = name;
Name = name;
}
else
{
NameAsString = MemberInfo.Name;
Name = MemberInfo.Name;
}
}

Debug.Assert(NameAsString != null);
internal void CacheNameAsUtf8BytesAndEscapedNameSection()
{
Debug.Assert(Name != null);

NameAsUtf8Bytes = Encoding.UTF8.GetBytes(NameAsString);
NameAsUtf8Bytes = Encoding.UTF8.GetBytes(Name);
EscapedNameSection = JsonHelpers.GetEscapedPropertyNameSection(NameAsUtf8Bytes, Options.Encoder);
}

Expand Down Expand Up @@ -347,8 +361,8 @@ internal abstract void Initialize(
internal bool IsForTypeInfo { get; set; }

// There are 3 copies of the property name:
// 1) NameAsString. The unescaped property name.
// 2) NameAsUtf8Bytes. The Utf8 version of NameAsString. Used during during deserialization for property lookup.
// 1) Name. The unescaped property name.
// 2) NameAsUtf8Bytes. The Utf8 version of Name. Used during during deserialization for property lookup.
// 3) EscapedNameSection. The escaped verson of NameAsUtf8Bytes plus the wrapping quotes and a trailing colon. Used during serialization.

/// <summary>
Expand All @@ -357,10 +371,10 @@ internal abstract void Initialize(
/// the value specified in JsonPropertyNameAttribute,
/// or the value returned from PropertyNamingPolicy(clrPropertyName).
/// </summary>
internal string NameAsString { get; set; } = null!;
internal string Name { get; set; } = null!;

/// <summary>
/// Utf8 version of NameAsString.
/// Utf8 version of Name.
/// </summary>
internal byte[] NameAsUtf8Bytes { get; set; } = null!;

Expand Down
35 changes: 17 additions & 18 deletions ...aries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ internal override void Initialize(
DeclaringType = parentClassType;
MemberInfo = memberInfo;
IsVirtual = isVirtual;
IgnoreCondition = ignoreCondition;

if (memberInfo != null)
{
Expand Down Expand Up @@ -113,12 +114,7 @@ internal override void Initialize(
}
}

// TODO (perf): can we pre-compute some of these values during source gen?
_converterIsExternalAndPolymorphic = !converter.IsInternalConverter && PropertyType != converter.TypeToConvert;
PropertyTypeCanBeNull = PropertyType.CanBeNull();
_propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType;

GetPolicies(ignoreCondition);
GetPolicies();
}
else
{
Expand All @@ -136,23 +132,21 @@ internal void InitializeForSourceGen(JsonSerializerOptions options, JsonProperty
// Property name settings.
if (propertyInfo.JsonPropertyName != null)
{
NameAsString = propertyInfo.JsonPropertyName;
Name = propertyInfo.JsonPropertyName;
}
else if (options.PropertyNamingPolicy == null)
{
NameAsString = ClrName;
Name = ClrName;
}
else
{
NameAsString = options.PropertyNamingPolicy.ConvertName(ClrName);
if (NameAsString == null)
Name = options.PropertyNamingPolicy.ConvertName(ClrName);
if (Name == null)
{
ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this);
}
}

NameAsUtf8Bytes ??= Encoding.UTF8.GetBytes(NameAsString!);
EscapedNameSection ??= JsonHelpers.GetEscapedPropertyNameSection(NameAsUtf8Bytes, Options.Encoder);
SrcGen_IsPublic = propertyInfo.IsPublic;
SrcGen_HasJsonInclude = propertyInfo.HasJsonInclude;
SrcGen_IsExtensionData = propertyInfo.IsExtensionData;
Expand Down Expand Up @@ -189,14 +183,19 @@ internal void InitializeForSourceGen(JsonSerializerOptions options, JsonProperty
DeclaringType = declaringType;
IgnoreCondition = propertyInfo.IgnoreCondition;
MemberType = propertyInfo.IsProperty ? MemberTypes.Property : MemberTypes.Field;

_converterIsExternalAndPolymorphic = !ConverterBase.IsInternalConverter && PropertyType != ConverterBase.TypeToConvert;
PropertyTypeCanBeNull = typeof(T).CanBeNull();
_propertyTypeEqualsTypeToConvert = ConverterBase.TypeToConvert == typeof(T);
ConverterStrategy = Converter!.ConverterStrategy;
DetermineIgnoreCondition(IgnoreCondition);
NumberHandling = propertyInfo.NumberHandling;
DetermineSerializationCapabilities(IgnoreCondition);
}
}

internal override void Configure()
{
base.Configure();

if (!IsForTypeInfo && !IsIgnored)
{
_converterIsExternalAndPolymorphic = !ConverterBase.IsInternalConverter && PropertyType != ConverterBase.TypeToConvert;
_propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType;
}
}

Expand Down
18 changes: 10 additions & 8 deletions src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,21 +208,23 @@ internal virtual void Configure()
JsonConverter converter = PropertyInfoForTypeInfo.ConverterBase;
converter.ConfigureJsonTypeInfo(this, Options);
PropertyInfoForTypeInfo.DeclaringTypeNumberHandling = NumberHandling;
PropertyInfoForTypeInfo.Configure();
PropertyInfoForTypeInfo.EnsureConfigured();

// Source gen currently when initializes properties
// also assigns JsonPropertyInfo's JsonTypeInfo which causes SO if there are any
// cycles in the object graph. For that reason properties cannot be added immediately.
// This is a no-op for ReflectionJsonTypeInfo
LateAddProperties();

DataExtensionProperty?.EnsureConfigured();

if (converter.ConverterStrategy == ConverterStrategy.Object && PropertyCache != null)
{
foreach (var jsonPropertyInfoKv in PropertyCache.List)
{
JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value!;
jsonPropertyInfo.DeclaringTypeNumberHandling = NumberHandling;
jsonPropertyInfo.Configure();
jsonPropertyInfo.EnsureConfigured();
}

if (converter.ConstructorIsParameterized)
Expand All @@ -246,14 +248,14 @@ internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDiction
string memberName = jsonPropertyInfo.ClrName!;

// The JsonPropertyNameAttribute or naming policy resulted in a collision.
if (!propertyCache!.TryAdd(jsonPropertyInfo.NameAsString, jsonPropertyInfo))
if (!propertyCache!.TryAdd(jsonPropertyInfo.Name, jsonPropertyInfo))
{
JsonPropertyInfo other = propertyCache[jsonPropertyInfo.NameAsString]!;
JsonPropertyInfo other = propertyCache[jsonPropertyInfo.Name]!;

if (other.IsIgnored)
{
// Overwrite previously cached property since it has [JsonIgnore].
propertyCache[jsonPropertyInfo.NameAsString] = jsonPropertyInfo;
propertyCache[jsonPropertyInfo.Name] = jsonPropertyInfo;
}
else if (
// Does the current property have `JsonIgnoreAttribute`?
Expand Down Expand Up @@ -353,18 +355,18 @@ internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonPara
ThrowHelper.ThrowInvalidOperationException_MultiplePropertiesBindToConstructorParameters(
Type,
parameterInfo.Name!,
matchingEntry.JsonPropertyInfo.NameAsString,
matchingEntry.JsonPropertyInfo.Name,
matchingEntry.DuplicateName);
}

Debug.Assert(matchingEntry.JsonPropertyInfo != null);
JsonPropertyInfo jsonPropertyInfo = matchingEntry.JsonPropertyInfo;
JsonParameterInfo jsonParameterInfo = CreateConstructorParameter(parameterInfo, jsonPropertyInfo, sourceGenMode, Options);
parameterCache.Add(jsonPropertyInfo.NameAsString, jsonParameterInfo);
parameterCache.Add(jsonPropertyInfo.Name, jsonParameterInfo);
}
// It is invalid for the extension data property to bind with a constructor argument.
else if (DataExtensionProperty != null &&
StringComparer.OrdinalIgnoreCase.Equals(paramToCheck.Name, DataExtensionProperty.NameAsString))
StringComparer.OrdinalIgnoreCase.Equals(paramToCheck.Name, DataExtensionProperty.Name))
{
ThrowHelper.ThrowInvalidOperationException_ExtensionDataCannotBindToCtorParam(DataExtensionProperty);
}
Expand Down
2 changes: 1 addition & 1 deletion ...System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private void CacheMember(
}

JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, Options);
Debug.Assert(jsonPropertyInfo.NameAsString != null);
Debug.Assert(jsonPropertyInfo.Name != null);

if (hasExtensionAttribute)
{
Expand Down

0 comments on commit 20c4bde

Please sign in to comment.