perf: Optimize JToken to Dictionary conversion performance (#9096)

* perf: optimize jToken conversion performance * fix: fix problems that may cause infinite loop * fix: add unit tests for code coverage
dotnet · Aug 19, 2023 · e7a2777 · e7a2777
1 parent fb4fe3f
commit e7a2777
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 30 deletions.
diff --git a/src/Docfx.Common/ConvertToObjectHelper.cs b/src/Docfx.Common/ConvertToObjectHelper.cs
@@ -1,36 +1,65 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
 using System.Dynamic;
 
 using Newtonsoft.Json.Linq;
+using System.Runtime.CompilerServices;
 
 namespace Docfx.Common;
 
 public static class ConvertToObjectHelper
 {
-    public static object ConvertExpandoObjectToObject(object raw)
+#nullable enable
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    public static object? ConvertJObjectToObject(object raw)
     {
-        return ConvertExpandoObjectToObjectCore(raw, new Dictionary<object, object>());
+        switch (raw)
+        {
+            case null:
+                return null;
+            case JToken jToken:
+                return ConvertJTokenToObject(jToken);
+            default:
+                return raw; //  if other type object passed. Return object itself.
+        }
     }
 
-    public static object ConvertJObjectToObject(object raw)
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    private static object? ConvertJTokenToObject([NotNull] JToken jToken)
     {
-        if (raw is JValue jValue)
-        {
-            return jValue.Value;
-        }
-        if (raw is JArray jArray)
-        {
-            return jArray.Select(ConvertJObjectToObject).ToArray();
-        }
-        if (raw is JObject jObject)
+        switch (jToken.Type)
         {
-            return jObject.ToObject<Dictionary<string, object>>().ToDictionary(p => p.Key, p => ConvertJObjectToObject(p.Value));
+            case JTokenType.Array:
+                return ConvertJArrayToObjectArray((JArray)jToken);
+            case JTokenType.Object:
+                return ConvertJObjectToDictionary((JObject)jToken);
+            default:
+                if (jToken is JValue jValue)
+                    return jValue.Value;
+                else
+                    throw new ArgumentException($"Not expected object type passed. JTokenType: {jToken.Type}, Text: {jToken}");
         }
-        return raw;
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    private static object?[] ConvertJArrayToObjectArray([NotNull] JArray jArray)
+    {
+        return jArray.Select(ConvertJTokenToObject).ToArray();
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
+    private static IDictionary<string, object?> ConvertJObjectToDictionary(JObject jObject)
+    {
+        var dictionary = (IDictionary<string, JToken>)jObject;
+        return dictionary.ToDictionary(p => p.Key,
+                                       p => p.Value == null
+                                         ? null
+                                         : ConvertJTokenToObject(p.Value));
+    }
+#nullable restore
+
     public static object ConvertStrongTypeToObject(object raw)
     {
         return ConvertJObjectToObject(ConvertStrongTypeToJObject(raw));
@@ -46,6 +75,11 @@ public static object ConvertStrongTypeToJObject(object raw)
         return JToken.FromObject(raw, JsonUtility.DefaultSerializer.Value);
     }
 
+    public static object ConvertExpandoObjectToObject(object raw)
+    {
+        return ConvertExpandoObjectToObjectCore(raw, new Dictionary<object, object>());
+    }
+
     public static object ConvertToDynamic(object obj)
     {
         return ConvertToDynamicCore(obj, new Dictionary<object, object>());

diff --git a/test/Docfx.Common.Tests/ConvertToObjectHelperTest.cs b/test/Docfx.Common.Tests/ConvertToObjectHelperTest.cs
@@ -1,7 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using FluentAssertions;
 using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
 using Xunit;
 
 namespace Docfx.Common.Tests;
@@ -69,6 +71,21 @@ public void ConvertObjectWithCircularReferenceToDynamic()
         Assert.Equal("value", ((Dictionary<string, object>)obj["key1"])["key"]);
     }
 
+    [Fact]
+    public void ConvertJObjectToObject_UnexpectedType()
+    {
+        // Arrange
+        var jToken = new JProperty("name", "dummy");
+
+        // Act
+        var action = () => { ConvertToObjectHelper.ConvertJObjectToObject(jToken); };
+
+        // Assert
+        action.Should()
+              .Throw<ArgumentException>()
+              .WithMessage("Not expected object type passed. JTokenType: Property, Text: \"name\": \"dummy\"");
+    }
+
     private sealed class ComplexType
     {
         public string String { get; set; }

diff --git a/test/docfx.Tests/JsonConverterTest.cs b/test/docfx.Tests/JsonConverterTest.cs
@@ -136,22 +136,6 @@ public void TestManifestItemCollectionConverterCouldSerializeAndDeserialize()
             "{\"files\":[{\"source_relative_path\":\"a\",\"output\":{}},{\"source_relative_path\":\"b\",\"output\":{}}]}",
             JsonUtility.Serialize(JsonUtility.FromJsonString<Manifest>(JsonUtility.Serialize(manifest))));
     }
-
-    private static object ConvertJObjectToObject(object raw)
-    {
-        if (raw is JValue jValue) { return jValue.Value; }
-
-        if (raw is JArray jArray)
-        {
-            return jArray.Select(ConvertJObjectToObject).ToArray();
-        }
-
-        if (raw is JObject jObject)
-        {
-            return jObject.ToObject<Dictionary<string, object>>().ToDictionary(p => p.Key, p => ConvertJObjectToObject(p.Value));
-        }
-        return raw;
-    }
 }
 
 internal class SkipEmptyOrNullContractResolver : DefaultContractResolver