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

Manually resolving JsonTypeInfo + Serialize methods slower than call Serializer directly #80750

Closed
brunolins16 opened this issue Jan 17, 2023 · 2 comments
Labels

Comments

@brunolins16
Copy link
Member

Description

While working on update ASP.NET Core to use aot/trimmer-safe S.T.J.Serializer methods I found some performance differences between calling the serializer methods directly in comparison with call the options.GetTypeInfo and provide the obtained TypeInfo to the serializer method.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Toolchains.CsProj;
using BenchmarkDotNet.Toolchains.DotNetCli;
using System.Text.Json;
using System.Text.Json.Serialization.Metadata;

[MemoryDiagnoser]
public class JsonTypeInfoBenchmark
{
    private Poco _pocoInstance = new Poco();
    private JsonSerializerOptions _options = new JsonSerializerOptions() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };

    [Benchmark]
    public string SerializerWithTypeInfo()
    {
        var typeInfo = _options.GetTypeInfo(typeof(Poco));
        return JsonSerializer.Serialize(_pocoInstance, typeInfo);
    }

    [Benchmark]
    public string Serialize()
    {
        return JsonSerializer.Serialize(_pocoInstance, _options);
    }

    public class Poco { }
}

Data

BenchmarkDotNet=v0.13.4, OS=Windows 11 (10.0.22623.1095)
11th Gen Intel Core i7-11850H 2.50GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-alpha.1.23061.8
  [Host]     : .NET 8.0.0 (8.0.23.5802), X64 RyuJIT AVX2
  Job-SZDHAS : .NET 8.0.0 (8.0.23.6702), X64 RyuJIT AVX2

Toolchain=.NET 8.0  
Method Mean Error StdDev Gen0 Allocated
SerializerWithTypeInfo 130.4 ns 2.60 ns 5.00 ns 0.0024 32 B
Serialize 102.7 ns 2.09 ns 3.37 ns 0.0025 32 B

Analysis

Both, ASP.NET Core benchmarks and this simple benchmark, try to serialize the same type (eg.: Poco) multiple times and, I did not have a chance to do a deep investigation, but we are missing cache of the lastTypeInfo when the JsonTypeInfo is obtained manually might be related the performance difference or maybe it is unrelated.

@brunolins16 brunolins16 added the tenet-performance Performance related issue label Jan 17, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 17, 2023
@ghost
Copy link

ghost commented Jan 17, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

While working on update ASP.NET Core to use aot/trimmer-safe S.T.J.Serializer methods I found some performance differences between calling the serializer methods directly in comparison with call the options.GetTypeInfo and provide the obtained TypeInfo to the serializer method.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Toolchains.CsProj;
using BenchmarkDotNet.Toolchains.DotNetCli;
using System.Text.Json;
using System.Text.Json.Serialization.Metadata;

[MemoryDiagnoser]
public class JsonTypeInfoBenchmark
{
    private Poco _pocoInstance = new Poco();
    private JsonSerializerOptions _options = new JsonSerializerOptions() { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };

    [Benchmark]
    public string SerializerWithTypeInfo()
    {
        var typeInfo = _options.GetTypeInfo(typeof(Poco));
        return JsonSerializer.Serialize(_pocoInstance, typeInfo);
    }

    [Benchmark]
    public string Serialize()
    {
        return JsonSerializer.Serialize(_pocoInstance, _options);
    }

    public class Poco { }
}

Data

BenchmarkDotNet=v0.13.4, OS=Windows 11 (10.0.22623.1095)
11th Gen Intel Core i7-11850H 2.50GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK=8.0.100-alpha.1.23061.8
  [Host]     : .NET 8.0.0 (8.0.23.5802), X64 RyuJIT AVX2
  Job-SZDHAS : .NET 8.0.0 (8.0.23.6702), X64 RyuJIT AVX2

Toolchain=.NET 8.0  
Method Mean Error StdDev Gen0 Allocated
SerializerWithTypeInfo 130.4 ns 2.60 ns 5.00 ns 0.0024 32 B
Serialize 102.7 ns 2.09 ns 3.37 ns 0.0025 32 B

Analysis

Both, ASP.NET Core benchmarks and this simple benchmark, try to serialize the same type (eg.: Poco) multiple times and, I did not have a chance to do a deep investigation, but we are missing cache of the lastTypeInfo when the JsonTypeInfo is obtained manually might be related the performance difference or maybe it is unrelated.

Author: brunolins16
Assignees: -
Labels:

area-System.Text.Json, tenet-performance

Milestone: -

@eiriktsarpalis
Copy link
Member

The JsonSerializer.(De)serialize methods use a one-element LRU cache when resolving metadata for the root-level type:

/// <summary>
/// Return the TypeInfo for root API calls.
/// This has an LRU cache that is intended only for public API calls that specify the root type.
/// </summary>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal JsonTypeInfo GetTypeInfoForRootType(Type type)
{
JsonTypeInfo? jsonTypeInfo = _lastTypeInfo;
if (jsonTypeInfo?.Type != type)
{
_lastTypeInfo = jsonTypeInfo = GetTypeInfoInternal(type);
}
return jsonTypeInfo;
}

Hitting that cache saves a lookup to the ConcurrentDictionary<Type, JsonTypeInfo> used to store all metadata, but this is an optimization that only really works when only one root-level type ever gets serialized (as is the case in microbenchmarks).

Even though we could extend the same caching scheme to JsonSerializerOptions.GetTypeInfo, this method is not restricted to just root-level type metadata resolution. The same concern applies to the JsonSerializer.GetConverter method, which can be used in custom converters throughout the serialization graph. For this reason, I don't think we should try to optimize these methods in a similar way -- when resolving metadata tokens it is generally speaking expected for users to cache the results in their own components.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Jan 18, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 18, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants