Size saving opportunity for System.Text.Json generators with custom converters #84015
Description
Description
I was looking at the generated code from System.Text.Json in a few scenarios and noticed another case where we could get some small but nice size saving improvements, in a way that's proportional to how many custom converters a given user has. Consider:
[JsonConverter(typeof(MyConverter<MyEnum>))]
public enum MyEnum
{
}If this type is directly or indirectly discovered by the generator, you'll end up with the following:
Full generated code (click to expand):
// <auto-generated/>
#nullable enable annotations
#nullable disable warnings
// Suppress warnings about [Obsolete] member usage in generated code.
#pragma warning disable CS0618
public sealed partial class MyContext
{
private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyEnum>? _MyEnum;
/// <summary>
/// Defines the source generated JSON serialization contract metadata for a given type.
/// </summary>
public global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyEnum> MyEnum
{
get => _MyEnum ??= Create_MyEnum(Options, makeReadOnly: true);
}
private global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyEnum> Create_MyEnum(global::System.Text.Json.JsonSerializerOptions options, bool makeReadOnly)
{
global::System.Text.Json.Serialization.Metadata.JsonTypeInfo<global::MyEnum>? jsonTypeInfo = null;
global::System.Text.Json.Serialization.JsonConverter? customConverter;
if (options.Converters.Count > 0 && (customConverter = GetRuntimeProvidedCustomConverter(options, typeof(global::MyEnum))) != null)
{
jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyEnum>(options, customConverter);
}
else
{
global::System.Text.Json.Serialization.JsonConverter converter = new global::MyConverter<global::MyEnum>();
global::System.Type typeToConvert = typeof(global::MyEnum);
if (!converter.CanConvert(typeToConvert))
{
throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
}
jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyEnum> (options, converter);
}
if (makeReadOnly)
{
jsonTypeInfo.MakeReadOnly();
}
return jsonTypeInfo;
}
}In particular, this bit:
global::System.Text.Json.Serialization.JsonConverter converter = new global::MyConverter<global::MyEnum>();
global::System.Type typeToConvert = typeof(global::MyEnum);
if (!converter.CanConvert(typeToConvert))
{
throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
}That extra check is done inline for every single type that has a custom converter, which means you also repeat the full codegen for that interpolation + throw expression. I'm thinking there could be 2 possible options to fix this, each with pros and cons.
Option 1
The generator could simply emit a single shared helper once if there is at least a type using a custom converter in the JSON context geing generated. Then, it'd just reuse that shared helper every time it's emitting code to create a custom type converter:
[MethodImpl(MethodImplOptions.NoInlining)]
private static void ValidateCustomConverter(JsonConverter converter, Type typeToConvert)
{
if (!converter.CanConvert(typeToConvert))
{
throw new global::System.InvalidOperationException(string.Format("The converter '{0}' is not compatible with the type '{1}'.", converter.GetType(), typeToConvert));
}
}And then all other callsites would simply call it:
global::System.Text.Json.Serialization.JsonConverter converter = new global::MyConverter<global::MyEnum>();
ValidateCustomConverter(converter, typeof(global::MyEnum));
jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyEnum> (options, converter);Option 2
If we can add the check to JsonMetadataServices.CreateValueInfo, this would be even simpler and likely use less size still:
global::System.Text.Json.Serialization.JsonConverter converter = new global::MyConverter<global::MyEnum>();
jsonTypeInfo = global::System.Text.Json.Serialization.Metadata.JsonMetadataServices.CreateValueInfo<global::MyEnum> (options, converter);And then that CreateValueInfo call would just throw if the input converter doesn't support type T. Which I feel would make sense to test anyway in case it doesn't already? As in, why would you want a value info for a converter that's just invalid anyway?
As a bonus point, this would also simplify the generator a bit, since it wouldn't have to generate that check at all anymore.
I don't expect this to bring in massive size deltas, but still seems like an improvement that would make sense?