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

Text.Json Slowdown when using custom JsonConverter #45187

Closed
CryoMyst opened this issue Nov 25, 2020 · 2 comments
Closed

Text.Json Slowdown when using custom JsonConverter #45187

CryoMyst opened this issue Nov 25, 2020 · 2 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@CryoMyst
Copy link

Description

When using a custom JsonConverter on a class with many writes slowdown will occur and memory will spike when writing to a filestream.

Configuration

  • Windows 10
  • .Net 5
  • Visual Studio 2019 + LINQPad

LINQPad Example

async Task Main()
{
	await JsonSerializer.SerializeAsync(File.OpenWrite(Path.GetTempFileName()), new CustomClass(), new JsonSerializerOptions() { WriteIndented = true });
}

[JsonConverter(typeof(CustomClassConverter))]
class CustomClass
{ 
	
}

class CustomClassConverter : JsonConverter<CustomClass>
{
	public override bool CanConvert(Type typeToConvert)
	{
		return typeToConvert == typeof(CustomClass);
	}

	public override CustomClass? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
	{
		throw new NotImplementedException();
	}

	public override void Write(Utf8JsonWriter writer, CustomClass value, JsonSerializerOptions options)
	{
		var lastTime = DateTime.Now;
		var timeout = lastTime + TimeSpan.FromSeconds(10);
		
		for (int e = 0; e < 1000000; e++)
		{
			if (e % 10000 == 0 || DateTime.Now >= timeout)
			{
				Debug.WriteLine($"e:{e} Time spend:{DateTime.Now - lastTime}");
				lastTime = DateTime.Now;
				timeout = lastTime + TimeSpan.FromSeconds(10);
			}
			
			for (int i = 0; i < 200; i++)
			{
				writer.WriteNull("TestName");
			}
		}
	}
}

This slows down on my machine around e=280k

Regression?

This produces an OverflowException when run on on Core 3.1

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Nov 25, 2020
@layomia layomia added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Nov 25, 2020
@eiriktsarpalis eiriktsarpalis added untriaged New issue has not been triaged by the area owner and removed needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Apr 19, 2021
@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 23, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Jul 23, 2021
@eiriktsarpalis
Copy link
Member

This is a by-design limitation of custom converters. Because the publicly exposed converter methods do not support async serialization, serialization data is written to an intermediate PooledByteBufferWriter which cannot be flushed to the underlying stream while a custom converter is running. If a custom converter is serializing a large amount of data, then it is expected that this will manifest as increased buffer allocations.

We have been discussing potential improvements in future releases that might allow users author custom converters that support async serialization out of the box cc @steveharter.

@eiriktsarpalis
Copy link
Member

Closing in favor of #36785.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants