Passing a Stream Utf8JsonWriter to a JsonSerializer.Serialize method results in Pending bytes being misreported

[mattchidley]

Description

When using JsonSerializer to serialize to a stream, if more than one custom converter is in play we are seeing that the internal buffer is not able to be written to the stream until the entirety of the object has been serialized to the buffer. This can result in significant memory usage and high allocations in ArrayPool.Rent and Resize called from PooledByteBufferWriter. It seems to me like the serialization state/context gets trapped when a second custom converter is in play both sync and async -- and we lose the ability to write to the stream and clear the buffer until the entire object has been serialized. This can be extremely problematic when serializing large objects with custom converters. If we had a 5gb object in memory, we'd effectively need a similarly large buffer just to hold the serialized representation of the object.

I am aware of this issue here -- and understand that a workaround can be to return IAsyncEnumerable in an async context... but not much info provided for sync.

My question is: in a sync context, is there a recommended approach to buffer/stream outside of what is posted on the docs? The docs recommend creating a writer for the stream and then passing the writer to the serializer -- this is also not ideal and can cause extremely high CPU if flushing after serializing using a custom converter, especially if there's a large result with many properties requiring custom conversion. I'm currently playing around with wrapping the stream with a MS's BCL BufferedStream, but would really appreciate any recommendations from your side to remedy these perf issues.

Configuration

Here is a sample where you can observe the buffer being filled and not cleared/flushed until the entire payload is serialized to the buffer:

using System.Text.Json;
using System.Text.Json.Serialization;

const string fileName = "testFile.json";
const int numTestObjects = 1000;
const int stringLength = 64;
Random random = new Random();

var fileStream = File.Create(fileName);
JsonSerializer.Serialize(fileStream, CreateManyTestObjects(), new JsonSerializerOptions { Converters = { new ParentConverter(), new ChildConverter() }});
File.Delete(fileName);

string RandomString(int length)
{
    const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    return new string(Enumerable.Repeat(chars, length)
        .Select(s => s[random.Next(s.Length)]).ToArray());
}

IEnumerable<Parent> CreateManyTestObjects()
{
    for (var i = 0; i < numTestObjects; i++)
    {
        yield return new Parent(new Child(RandomString(stringLength)));
    }
}

class Parent
{
    public Child Child { get; }
    public Parent(Child child)
    {
        Child = child;
    }
}

class Child
{
    public string Prop { get; }
    public Child(string prop)
    {
        Prop = prop;
    }
}

class ParentConverter : JsonConverter<Parent>
{
    public override Parent Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => 
        throw new NotImplementedException();

    public override void Write(Utf8JsonWriter writer, Parent value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();
        writer.WritePropertyName(nameof(value.Child));
        
        // this will call the converter
        JsonSerializer.Serialize(writer, value.Child, options);
        
        writer.WriteEndObject();
    }
}

class ChildConverter : JsonConverter<Child>
{
    public override Child Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => 
        throw new NotImplementedException();

    public override void Write(Utf8JsonWriter writer, Child value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();
        writer.WritePropertyName(nameof(value.Prop));
        writer.WriteStringValue(value.Prop);
        writer.WriteEndObject();
    }
}

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

Issue Details

---

Description

When using JsonSerializer to serialize to a stream, if more than one custom converter is in play we are seeing that the internal buffer is not able to be written to the stream until the entirety of the object has been serialized to the buffer. This can result in significant memory usage and high allocations in ArrayPool.Rent and Resize called from PooledByteBufferWriter. It seems to me like the serialization state/context gets trapped when a second custom converter is in play both sync and async -- and we lose the ability to write to the stream and clear the buffer until the entire object has been serialized. This can be extremely problematic when serializing large objects with custom converters. If we had a 5gb object in memory, we'd effectively need a similarly large buffer just to hold the serialized representation of the object.

I am aware of this issue here -- and understand that a workaround can be to return IAsyncEnumerable in an async context... but not much info provided for sync.

My question is: in a sync context, is there a recommended approach to buffer/stream outside of what is posted on the docs? The docs recommend creating a writer for the stream and then passing the writer to the serializer -- this is also not ideal and can cause extremely high CPU if flushing after serializing using a custom converter. I'm currently playing around with wrapping the stream with a MS's BCL PooledBufferedStream, but would really appreciate any recommendations from your side to remedy these perf issues.

Configuration

Here is a sample where you can observe the buffer being filled and not cleared/flushed until the entire payload is serialized to the buffer:

`
using System.Text.Json;
using System.Text.Json.Serialization;

const string fileName = "testFile.json";
const int numTestObjects = 1000;
const int stringLength = 64;
Random random = new Random();

var fileStream = File.Create(fileName);
JsonSerializer.Serialize(fileStream, CreateManyTestObjects(), new JsonSerializerOptions { Converters = { new ParentConverter(), new ChildConverter() }});
File.Delete(fileName);

string RandomString(int length)
{
const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
return new string(Enumerable.Repeat(chars, length)
.Select(s => s[random.Next(s.Length)]).ToArray());
}

IEnumerable CreateManyTestObjects()
{
for (var i = 0; i < numTestObjects; i++)
{
yield return new Parent(new Child(RandomString(stringLength)));
}
}

class Parent
{
public Child Child { get; }
public Parent(Child child)
{
Child = child;
}
}

class Child
{
public string Prop { get; }
public Child(string prop)
{
Prop = prop;
}
}

class ParentConverter : JsonConverter
{
public override Parent Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
throw new NotImplementedException();

public override void Write(Utf8JsonWriter writer, Parent value, JsonSerializerOptions options)
{
    writer.WriteStartObject();
    writer.WritePropertyName(nameof(value.Child));
    
    // this will call the converter
    JsonSerializer.Serialize(writer, value.Child, options);
    
    writer.WriteEndObject();
}

}

class ChildConverter : JsonConverter
{
public override Child Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
throw new NotImplementedException();

public override void Write(Utf8JsonWriter writer, Child value, JsonSerializerOptions options)
{
    writer.WriteStartObject();
    writer.WritePropertyName(nameof(value.Prop));
    writer.WriteStringValue(value.Prop);
    writer.WriteEndObject();
}

}
`

Author:	mattchidley
Assignees:	-
Labels:	area-System.Text.Json, tenet-performance, untriaged
Milestone:	-

[eiriktsarpalis]

Actually this seems like a bug with Utf8JsonWriter when used with the relevant JsonSerializer.Serialize overload. Minimal reproduction:

using System;
using System.Collections.Generic;
using System.Text.Json;
using System.Text.Json.Serialization;

var options = new JsonSerializerOptions { DefaultBufferSize = 512 };
options.Converters.Add(new MyStringConverter());

using var stream = Console.OpenStandardOutput();
JsonSerializer.Serialize(stream, CreateManyTestObjects(), options);
//await JsonSerializer.Serialize(stream, CreateManyTestObjects(), options); // issue also impacts async method

IEnumerable<string> CreateManyTestObjects()
{
    int i = 0;
    while (true)
    {
        if (++i % 10_000 == 0)
        {
            System.Threading.Thread.Sleep(500);
        }
        yield return "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
    }
}

class MyStringConverter : JsonConverter<string>
{
    public override string Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
        throw new NotImplementedException();

    public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(value);
        // JsonSerializer.Serialize(writer, value); // replace with this line to trigger bug
    }
}

When passing the Utf8JsonWriter instance to the nested Serialize method it seems like it's misreporting the pending bytes as committed:

which causes the ShouldFlush check in the enumerable converter to not fire. Compare with a regular converter:

cc @layomia @steveharter

[mattchidley]

Just to add for anyone who might follow this -- a large enough property could cause an OutOfMemoryException when trying to resize the buffer from Utf8JsonWriter.Grow

As noted in my initial report, creating a BufferedStream implementation to flush the writer could be a workaround to prevent OOM. So instead of serializing directly to the stream, we would construct a Utf8JsonWriter of the bufferedstream and pass the writer to the serializer instead of the stream.

@eiriktsarpalis -- we're looking to Microsoft here to recommend a workaround or possible move up the bug fix timeline?

[eiriktsarpalis]

possible move up the bug fix timeline?

We will try to get this fixed for .NET 7, but it doesn't meet the bar for .NET 6 servicing.

[mattchidley]

@eiriktsarpalis would this issue also be present in an async context? We noticed your comment here and are hoping that these are two separate issue fixes. It's important to stress here that in both sync and async contexts we can balloon a buffer enough to cause an out of memory crash.

[eiriktsarpalis]

Hi @mattchidley, I believe the two issues are unrelated.