Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Optimize JToken to Dictionary conversion performance #9096

Merged
merged 3 commits into from
Aug 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions src/Docfx.Common/ConvertToObjectHelper.cs
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why not also cast here?

Suggested change
if (jToken is JValue jValue)
return ((JValue)jToken).Value;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why not also cast here?
There is some types that derived from JToken but not JValue. (e.g. JContainer, JProperty)
(Thought these types should not be passed to this function)

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));
Expand All @@ -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>());
Expand Down
17 changes: 17 additions & 0 deletions test/Docfx.Common.Tests/ConvertToObjectHelperTest.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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; }
Expand Down
16 changes: 0 additions & 16 deletions test/docfx.Tests/JsonConverterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down