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

Using JSON Source Gen with an Object property roots all JsonConverters #52943

Closed
eerhardt opened this issue May 18, 2021 · 1 comment · Fixed by #52934
Closed

Using JSON Source Gen with an Object property roots all JsonConverters #52943

eerhardt opened this issue May 18, 2021 · 1 comment · Fixed by #52934
Assignees
Labels
area-System.Text.Json untriaged New issue has not been triaged by the area owner

Comments

@eerhardt
Copy link
Member

Using System.Text.Json source generation in my application isn't providing for size reductions in the app because all the JsonConverters are still being rooted in the app.

Similar to #52662, but now because I have the object Value property in my WeatherForecast class.

Repro

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using ConsoleApp13;
using ConsoleApp13.JsonSourceGeneration;

[assembly: JsonSerializable(typeof(WeatherForecast))]

namespace ConsoleApp13
{
    class Program
    {
        static async Task Main(string[] args)
        {
            string f = @"{""TemperatureC"":0,""Summary"":""hi""}";

            var forecast = JsonSerializer.Deserialize(f, new JsonContext().WeatherForecast);
            Console.WriteLine(forecast);
        }
    }

    public record WeatherForecast
    {
        public DateTime Date { get; set; }

        public int TemperatureC { get; set; }

        public string Summary { get; set; }

        public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

        public object Value { get; set; }
    }
}

dotnet publish -r win-x64 -c Release -p:PublishTrimmed=true -p:DebuggerSupport=false -p:TrimmerSingleWarn=false

View the outputted assemblies, and notice that System.Text.Json.dll is ~275KB and still has all its JsonConverters in it.

Analysis

From looking at the results, the reason everything is still rooted is because of the following call graph:

  • The source gen'd JsonContext class for WeatherForecast has a public JsonTypeInfo<object> Object property that references JsonMetadataServices.ObjectConverter.
  • JsonMetadataServices.ObjectConverter is a System.Text.Json.Serialization.Converters.ObjectConverter instance.
  • ObjectConverter takes a direct dependency on JsonNodeConverter.Instance
  • JsonNodeConverter creates new JsonValue<T>
  • JsonValue<T>.ToString calls JsonSerializer.Serialize without source gen information

That this point, since JsonSerializer.Serialize is called without source gen information, it now roots all JsonConverters, and the size of the app grows by a lot.

(Note that this pattern exists in ASP.NET Blazor WASM)

cc @layomia @steveharter @marek-safar @vitek-karas @CoffeeFlux

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels May 18, 2021
@ghost
Copy link

ghost commented May 18, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Using System.Text.Json source generation in my application isn't providing for size reductions in the app because all the JsonConverters are still being rooted in the app.

Similar to #52662, but now because I have the object Value property in my WeatherForecast class.

Repro

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Threading.Tasks;
using ConsoleApp13;
using ConsoleApp13.JsonSourceGeneration;

[assembly: JsonSerializable(typeof(WeatherForecast))]

namespace ConsoleApp13
{
    class Program
    {
        static async Task Main(string[] args)
        {
            string f = @"{""TemperatureC"":0,""Summary"":""hi""}";

            var forecast = JsonSerializer.Deserialize(f, new JsonContext().WeatherForecast);
            Console.WriteLine(forecast);
        }
    }

    public record WeatherForecast
    {
        public DateTime Date { get; set; }

        public int TemperatureC { get; set; }

        public string Summary { get; set; }

        public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

        public object Value { get; set; }
    }
}

dotnet publish -r win-x64 -c Release -p:PublishTrimmed=true -p:DebuggerSupport=false -p:TrimmerSingleWarn=false

View the outputted assemblies, and notice that System.Text.Json.dll is ~275KB and still has all its JsonConverters in it.

Analysis

From looking at the results, the reason everything is still rooted is because of the following call graph:

  • The source gen'd JsonContext class for WeatherForecast has a public JsonTypeInfo<object> Object property that references JsonMetadataServices.ObjectConverter.
  • JsonMetadataServices.ObjectConverter is a System.Text.Json.Serialization.Converters.ObjectConverter instance.
  • ObjectConverter takes a direct dependency on JsonNodeConverter.Instance
  • JsonNodeConverter creates new JsonValue<T>
  • JsonValue<T>.ToString calls JsonSerializer.Serialize without source gen information

That this point, since JsonSerializer.Serialize is called without source gen information, it now roots all JsonConverters, and the size of the app grows by a lot.

(Note that this pattern exists in ASP.NET Blazor WASM)

cc @layomia @steveharter @marek-safar @vitek-karas @CoffeeFlux

Author: eerhardt
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@steveharter steveharter self-assigned this May 19, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 19, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants