Improve JSON validation perf

[PranavSenthilnathan]

Fixes #111039

With the addition of string value segment APIs to the JSON writer (#101356), validation in UTF8JsonWriter needs to also validate that values are not written after uncompleted string segments. This extra check for every write impacts perf, especially when writing values in tight loops. The linked bug above shows some such scenarios (e.g. serializing collections). This PR improves performance for validation by (1) refactoring exception throwing call sites and (2) changing the validation check when writing JSON values.

Perf

The commits measured are main (bc2ac2f), pre-StringValueSegment (before the regression was introduced, e5878e9), and this PR. All 4 test cases from #111039 are shown here.

WriteJson<HashSet>.SerializeToWriter

Method	Toolchain	Mode	Mean	Ratio
SerializeToWriter	main	SourceGen	1.774 us	1.12
SerializeToWriter	pre-StringValueSegment	SourceGen	1.583 us	1.00
SerializeToWriter	PR	SourceGen	1.456 us	0.92

WriteJson<ImmutableDictionary<String, String>>.SerializeToWriter

Method	Toolchain	Mode	Mean	Ratio
SerializeToWriter	main	SourceGen	5.966 us	1.33
SerializeToWriter	pre-StringValueSegment	SourceGen	4.483 us	1.00
SerializeToWriter	PR	SourceGen	4.450 us	0.99

WriteJson<LoginViewModel>.SerializeToWriter

Method	Toolchain	Mode	Mean	Ratio
SerializeToWriter	main	SourceGen	64.03 ns	1.07
SerializeToWriter	pre-StringValueSegment	SourceGen	59.82 ns	1.00
SerializeToWriter	PR	SourceGen	56.46 ns	0.94

Perf_Basic.WriteBasicUtf8

Method	Toolchain	Formatted	SkipValidation	DataSize	Mean	Ratio
WriteBasicUtf8	main	False	False	100000	984.8 us	1.01
WriteBasicUtf8	pre-StringValueSegment	False	False	100000	978.0 us	1.00
WriteBasicUtf8	PR	False	False	100000	771.0 us	0.79

Changes in this PR

Exceptions

Exceptions are now using the throw helper pattern with minimal arguments. This reduces the code size in the validation methods and defers the performance penalty to only the exceptional scenarios. With this change the JIT also seems more willing to inline business logic which gives perf improvements in tight loop source generated methods like this:

private void HashSetStringSerializeHandler(global::System.Text.Json.Utf8JsonWriter writer, global::System.Collections.Generic.HashSet<string>? value)
{
    if (value is null)
    {
        writer.WriteNullValue();
        return;
    }

    writer.WriteStartArray();

    foreach (string element in value)
    {
        writer.WriteStringValue(element);
    }

    writer.WriteEndArray();
}

Validation for values

The validation check for writing values is now a single check instead of multiple checks based on enclosing container type. For this, _isObject is now changed to _enclosingContainer, which can be either object, array, root/none, or partial value. This removes the need to check CurrentDepth separately to determine whether the writer is at the root level. In addition, the actual enum values are:

/// <summary>
/// The type of container that is enclosing the current position. The underlying values have been chosen
/// to allow <see cref="CanWriteValue"/> to be done using bitwise operations and must be kept in sync with <see cref="JsonTokenType"/>.
/// </summary>
internal enum EnclosingContainerType : byte
{
    /// <summary>
    /// Root level.
    /// </summary>
    None = JsonTokenType.None,

    /// <summary>
    /// JSON object.
    /// </summary>
    Object = JsonTokenType.PropertyName,

    /// <summary>
    /// JSON array. This is chosen large enough to not conflict with any <see cref="JsonTokenType"/> value.
    /// </summary>
    Array =                 0b000_1_0000,

    /// <summary>
    /// Partial UTF-8 string. This is a container if viewed as an array of "utf-8 string segment"-typed values. This array can only be one level deep
    /// so <see cref="_bitStack"/> does not need to store its state.
    /// <see cref="IsWritingPartialString"/> relies on the value of the partial string members being the largest values of this enum.
    /// </summary>
    Utf8StringSequence =    0b001_0_0000,

    /// <summary>
    /// Partial UTF-16 string. This is a container if viewed as an array of "utf-16 string segment"-typed values. This array can only be one level deep
    /// so <see cref="_bitStack"/> does not need to store its state.
    /// <see cref="IsWritingPartialString"/> relies on the value of the partial string members being the largest values of this enum.
    /// </summary>
    Utf16StringSequence =   0b010_0_0000,

    /// <summary>
    /// Partial Base64 string. This is a container if viewed as an array of "base64 string segment"-typed values. This array can only be one level deep
    /// so <see cref="_bitStack"/> does not need to store its state.
    /// <see cref="IsWritingPartialString"/> relies on the value of the partial string members being the largest values of this enum.
    /// </summary>
    Base64StringSequence =  0b011_0_0000,
}

These were chosen in such a way that the value validation can be reduced to:

private bool CanWriteValue => _enclosingContainer == EnclosingContainerType.Array | (byte)_enclosingContainer == (byte)_tokenType;

Writing values in arrays is always valid and writing values in partial values is never valid. Otherwise, in objects and at the root level, the only valid previous tokens are property and none respectively. This can be checked with equality since we picked the enum values for EnclosingContainerType for this purpose.

[Copilot]

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Helpers.cs:71

• The validation check for _tokenType == StringSegmentSentinel should be consistent with other checks. Ensure that the logic is clear and maintainable.

private void OnValidateWritingValueFailed()

[eiriktsarpalis]

Here is a rough inductive proof that this works:

Nice! Would the hypothetical inclusion of new token types in the future render this optimization obsolete?

Would it be possible to simplify this technique if we moved StringSegmentSentinel to instead be represented as enclosing container type(s)? Technically a sequence of string segments does denote an array-like container where you can only append segments of the same encoding.

[PranavSenthilnathan]

Would it be possible to simplify this technique if we moved StringSegmentSentinel to instead be represented as enclosing container type(s)? Technically a sequence of string segments does denote an array-like container where you can only append segments of the same encoding.

Yep, that is much simpler, I updated PR description with the changes and the perf (which didn't change much).

Nice! Would the hypothetical inclusion of new token types in the future render this optimization obsolete?

The new version still depends on JsonTokenType.None and JsonTokenType.PropertyName but I don't think it will be difficult to find another simple bit-check if we do add something new. The previous code certainly risked this issue.

[Copilot]

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)

• src/libraries/System.Text.Json/src/System/Text/Json/BitStack.cs: Evaluated as low risk

Comments suppressed due to low confidence (1)

src/libraries/System.Text.Json/src/System/Text/Json/Writer/Utf8JsonWriter.WriteValues.Helpers.cs:51

• The condition should be checked for null or invalid state before proceeding.

if (_enclosingContainer == EnclosingContainerType.PartialValue)

[eiriktsarpalis]

I would recommend merging #111041 before continuing work on this PR. Functional completeness first, then we can make it faster.

[eiriktsarpalis]

What are we peeking at in this case? It seems we're just returning a boolean so perhaps this could be expressed as a property, i.e. IsInArray or something?

[PranavSenthilnathan]

It looks at the top of the stack assuming the stack is backed by an array. IsInArray sounds like it's checking containment. I also like it being a method since it's symmetric with Push/PushToArray and Peek/PeekInArray. I'm open to better naming but I left it as is with more comments.

[eiriktsarpalis]

This looks great. Do you have updated benchmark results to share after including the most recent changes?

I would also recommend comparing the full benchmark suite from dotnet/performance in case a particular workflow has slipped.

Thanks!

[PranavSenthilnathan]

@EgorBot -amd -arm -profiler --envvars DOTNET_JitDisasm:System.Text.Json.JsonDocument:WriteString(byref,System.Text.Json.Utf8JsonWriter)

using BenchmarkDotNet.Attributes;
using System.Buffers;
using System.Text.Json;
using System.Text;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(args);

public class Perf_ParseThenWrite
{
    const string jsonString =
"""
{
    "tags": [
      "culpa",
    "laboris",
    "nulla",
    "exercitation",
    "do",
    "pariatur",
    "occaecat",
    "aliquip",
    "est",
    "et",
    "officia",
    "minim",
    "quis",
    "est",
    "ullamco",
    "consequat",
    "id",
    "esse",
    "irure",
    "reprehenderit",
    "proident",
    "labore",
    "et",
    "in",
    "consequat",
    "officia",
    "exercitation",
    "aute",
    "do",
    "consequat",
    "laborum",
    "officia",
    "consectetur",
    "deserunt",
    "ex",
    "do",
    "nostrud",
    "deserunt",
    "consectetur",
    "adipisicing",
    "et",
    "non",
    "veniam",
    "dolor",
    "Lorem",
    "sunt",
    "amet",
    "cillum",
    "excepteur",
    "mollit"
    ]
}
""";

    private byte[] _dataUtf8;
    private Utf8JsonWriter _writer;

    public bool IsDataIndented = true;

    [GlobalSetup]
    public void Setup()
    {
        _dataUtf8 = Encoding.UTF8.GetBytes(jsonString);

        var abw = new ArrayBufferWriter<byte>();
        _writer = new Utf8JsonWriter(abw, new JsonWriterOptions { Indented = IsDataIndented });
    }

    [GlobalCleanup]
    public void CleanUp()
    {
        _writer.Dispose();
    }

    [Benchmark]
    public void ParseThenWrite()
    {
        _writer.Reset();

        using (JsonDocument document = JsonDocument.Parse(_dataUtf8))
        {
            document.WriteTo(_writer);
        }
    }
}

[PranavSenthilnathan]

Do you have updated benchmark results to share after including the most recent changes?

Updated results for latest commit. Looks basically the same as before.

I would also recommend comparing the full benchmark suite from dotnet/performance in case a particular workflow has slipped.

Only Perf_ParseThenWrite seemed to have a new regression, around 5% slower. After talking offline, we decided to get this PR in and reassess if a regression shows up in the perf CI.

[eiriktsarpalis]

Remind me, why do the bits of this value overlap with the previous two container types? Couldn't it just be an independent value like 1 << 7?

[PranavSenthilnathan]

That made it seem like no more values could be added to the enum, but actually an equivalently simple encoding that would still work is 1<<4, 2<<4, 3<<4 and 4<<4 for these last 4 members.

[eiriktsarpalis]

Presumably you mean 1 <<4, 1 << 5, 1 << 6, etc?

[PranavSenthilnathan]

No, I meant they don't need to be flags-style as long as they're >= 16 (avoiding bit overlap with json token type).

[PranavSenthilnathan]

Changed to hex for readability and renumbered to 0x10, 0x20, 0x30 and 0x40.

[eiriktsarpalis]

Thanks!

[pentp]

NoInlining should not be applied to throw helpers, otherwise RyuJIt will not detect the callsite as cold code.

These helpers should probably follow the pattern like throw GetInvalidOperationException(); to make them small, but with no return (thus not inlined), so the callsites can be optimized properly.