Only sort properties if necessary.

dotnet · Feb 2, 2023 · df8b0b3 · df8b0b3
1 parent 83d9048
commit df8b0b3
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 39 deletions.
diff --git a/...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs b/...t.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.Helpers.cs
@@ -13,7 +13,7 @@ public partial class DefaultJsonTypeInfoResolver
     {
         [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)]
         [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)]
-        private static JsonTypeInfo CreateCore(Type type, JsonConverter converter, JsonSerializerOptions options)
+        private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converter, JsonSerializerOptions options)
         {
             JsonTypeInfo typeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, converter, options);
             typeInfo.NumberHandling = GetNumberHandlingForType(typeInfo.Type);

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs
@@ -2,9 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
-using System.Text.Json.Reflection;
 using System.Threading;
 
 namespace System.Text.Json.Serialization.Metadata
@@ -86,7 +84,7 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options
         private static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options)
         {
             JsonConverter converter = GetConverterForType(type, options);
-            return CreateCore(type, converter, options);
+            return CreateTypeInfoCore(type, converter, options);
         }
 
         /// <summary>

diff --git a/...tem.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs b/...tem.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.Helpers.cs
@@ -29,7 +29,7 @@ private static JsonTypeInfo<T> CreateCore<T>(JsonConverter converter, JsonSerial
         /// </summary>
         private static JsonTypeInfo<T> CreateCore<T>(JsonSerializerOptions options, JsonObjectInfoValues<T> objectInfo)
         {
-            JsonConverter converter = GetConverter(objectInfo);
+            JsonConverter<T> converter = GetConverter(objectInfo);
             JsonTypeInfo<T> typeInfo = new JsonTypeInfo<T>(converter, options);
             if (objectInfo.ObjectWithParameterizedConstructorCreator != null)
             {
@@ -63,7 +63,7 @@ private static JsonTypeInfo<T> CreateCore<T>(
             object? createObjectWithArgs = null,
             object? addFunc = null)
         {
-            JsonConverter converter = new JsonMetadataServicesConverter<T>(converterCreator());
+            JsonConverter<T> converter = new JsonMetadataServicesConverter<T>(converterCreator());
             JsonTypeInfo<T> typeInfo = new JsonTypeInfo<T>(converter, options);
             if (collectionInfo is null)
             {
@@ -142,6 +142,8 @@ private static void PopulateProperties(JsonTypeInfo typeInfo, Func<JsonSerialize
                 return;
             }
 
+            // TODO update the source generator so that all property
+            // hierarchy resolution is happening at compile time.
             JsonTypeInfo.PropertyHierarchyResolutionState state = new();
 
             foreach (JsonPropertyInfo jsonPropertyInfo in properties)

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
@@ -212,23 +212,16 @@ public Action<object>? OnDeserialized
         /// It is required that added <see cref="JsonPropertyInfo"/> entries are unique up to <see cref="JsonPropertyInfo.Name"/>,
         /// however this will only be validated on serialization, once the metadata instance gets locked for further modification.
         /// </remarks>
-        public IList<JsonPropertyInfo> Properties
-        {
-            get
-            {
-                _isUserModifiedPropertyList = true;
-                return _properties ??= new(this);
-            }
-        }
-
+        public IList<JsonPropertyInfo> Properties => _properties ??= new(this);
         private JsonPropertyInfoList? _properties;
-        private bool _isUserModifiedPropertyList;
 
         internal void SortProperties()
         {
             Debug.Assert(!IsConfigured);
-            _properties?.SortProperties();
-            PropertyCache?.List?.StableSortByKey(static propInfo => propInfo.Value.Order);
+            Debug.Assert(_properties != null && _properties.Count > 0);
+
+            _properties.SortProperties();
+            PropertyCache?.List.StableSortByKey(static propInfo => propInfo.Value.Order);
         }
 
         /// <summary>
@@ -594,11 +587,11 @@ internal void Configure()
                 $"ConverterStrategy from PropertyInfoForTypeInfo.EffectiveConverter.ConverterStrategy ({PropertyInfoForTypeInfo.EffectiveConverter.ConverterStrategy}) does not match converter's ({Converter.ConverterStrategy})");
             if (Kind == JsonTypeInfoKind.Object)
             {
-                InitializePropertyCache();
+                ConfigureProperties();
 
                 if (Converter.ConstructorIsParameterized)
                 {
-                    InitializeConstructorParameters(ParameterInfoValues ?? Array.Empty<JsonParameterInfoValues>(), sourceGenMode: Options.TypeInfoResolver is JsonSerializerContext);
+                    ConfigureConstructorParameters();
                 }
             }
 
@@ -633,10 +626,9 @@ internal string GetDebugInfo()
             if (propCacheInitialized)
             {
                 sb.AppendLine("  Properties: {");
-                foreach (var property in PropertyCache!.List)
+                foreach (JsonPropertyInfo pi in PropertyCache!.Values)
                 {
-                    JsonPropertyInfo pi = property.Value;
-                    sb.AppendLine($"    {property.Key}:");
+                    sb.AppendLine($"    {pi.Name}:");
                     sb.AppendLine($"{pi.GetDebugInfo(indent: 6)},");
                 }
 
@@ -791,6 +783,9 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name)
         internal abstract object? DeserializeAsObject(Stream utf8Json);
         internal abstract IAsyncEnumerable<object?> DeserializeAsyncEnumerableAsObject(Stream utf8Json, CancellationToken cancellationToken);
 
+        /// <summary>
+        /// Used by the built-in resolvers to add property metadata applying conflict resolution.
+        /// </summary>
         internal void AddProperty(JsonPropertyInfo jsonPropertyInfo, ref PropertyHierarchyResolutionState state)
         {
             Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName can be null in custom JsonPropertyInfo instances and should never be passed in this method");
@@ -902,17 +897,18 @@ public ParameterLookupValue(JsonPropertyInfo jsonPropertyInfo)
             public JsonPropertyInfo JsonPropertyInfo { get; }
         }
 
-        internal void InitializePropertyCache()
+        internal void ConfigureProperties()
         {
             Debug.Assert(Kind == JsonTypeInfoKind.Object);
             Debug.Assert(PropertyCache is null);
             Debug.Assert(ExtensionDataProperty is null);
 
             IList<JsonPropertyInfo> properties = (IList<JsonPropertyInfo>?)_properties ?? Array.Empty<JsonPropertyInfo>();
-            int numberOfRequiredProperties = 0;
-            bool isOrderSpecified = false;
 
-            PropertyCache = CreatePropertyCache(capacity: properties.Count);
+            JsonPropertyDictionary<JsonPropertyInfo> propertyCache = CreatePropertyCache(capacity: properties.Count);
+            int numberOfRequiredProperties = 0;
+            bool arePropertiesSorted = true;
+            int previousPropertyOrder = int.MinValue;
 
             foreach (JsonPropertyInfo property in properties)
             {
@@ -939,9 +935,13 @@ internal void InitializePropertyCache()
                         property.RequiredPropertyIndex = numberOfRequiredProperties++;
                     }
 
-                    isOrderSpecified |= property.Order != 0;
+                    if (arePropertiesSorted)
+                    {
+                        arePropertiesSorted = previousPropertyOrder <= property.Order;
+                        previousPropertyOrder = property.Order;
+                    }
 
-                    if (!PropertyCache.TryAddValue(property.Name, property))
+                    if (!propertyCache.TryAddValue(property.Name, property))
                     {
                         ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameConflict(Type, property.Name);
                     }
@@ -950,13 +950,15 @@ internal void InitializePropertyCache()
                 property.EnsureConfigured();
             }
 
-            if (isOrderSpecified && _isUserModifiedPropertyList)
+            NumberOfRequiredProperties = numberOfRequiredProperties;
+            PropertyCache = propertyCache;
+
+            if (!arePropertiesSorted)
             {
-                // We only need to sort again if we know that the user has accessed properties.
+                // Properties have been configured by the user and require sorting.
                 SortProperties();
             }
 
-            NumberOfRequiredProperties = numberOfRequiredProperties;
             // Override global UnmappedMemberHandling configuration
             // if type specifies an extension data property.
             EffectiveUnmappedMemberHandling = UnmappedMemberHandling ??
@@ -965,11 +967,15 @@ internal void InitializePropertyCache()
                     : JsonUnmappedMemberHandling.Skip);
         }
 
-        internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
+        internal void ConfigureConstructorParameters()
         {
-            Debug.Assert(ParameterCache is null);
-            Debug.Assert(PropertyCache is not null);
             Debug.Assert(Kind == JsonTypeInfoKind.Object);
+            Debug.Assert(Converter.ConstructorIsParameterized);
+            Debug.Assert(PropertyCache is not null);
+            Debug.Assert(ParameterCache is null);
+
+            JsonParameterInfoValues[] jsonParameters = ParameterInfoValues ?? Array.Empty<JsonParameterInfoValues>();
+            bool sourceGenMode = Options.TypeInfoResolver is JsonSerializerContext;
 
             var parameterCache = new JsonPropertyDictionary<JsonParameterInfo>(Options.PropertyNameCaseInsensitive, jsonParameters.Length);
 
@@ -1018,7 +1024,7 @@ internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonPara
                     JsonParameterInfo jsonParameterInfo = CreateConstructorParameter(parameterInfo, jsonPropertyInfo, sourceGenMode, Options);
                     parameterCache.Add(jsonPropertyInfo.Name, jsonParameterInfo);
                 }
-                // It is invalid for the extension data property to bind with a constructor argument.
+                // It is invalid for the extension data property to bind to a constructor argument.
                 else if (ExtensionDataProperty != null &&
                     StringComparer.OrdinalIgnoreCase.Equals(paramToCheck.Name, ExtensionDataProperty.Name))
                 {

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs
@@ -8,7 +8,6 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
-using System.Runtime.CompilerServices;
 using System.Text.Json.Serialization;
 using System.Text.Json.Serialization.Metadata;
 
@@ -226,9 +225,9 @@ public static void ThrowJsonException_JsonRequiredPropertyMissing(JsonTypeInfo p
             // Soft cut-off length - once message becomes longer than that we won't be adding more elements
             const int CutOffLength = 50;
 
-            for (int propertyIdx = 0; propertyIdx < parent.PropertyCache.List.Count; propertyIdx++)
+            foreach (KeyValuePair<string, JsonPropertyInfo> kvp in parent.PropertyCache.List)
             {
-                JsonPropertyInfo property = parent.PropertyCache.List[propertyIdx].Value;
+                JsonPropertyInfo property = kvp.Value;
 
                 if (!property.IsRequired || requiredPropertiesSet[property.RequiredPropertyIndex])
                 {