Skip to content

Avoid duplicate PropertyRef[] allocation in UTF-8 property cache publish#130656

Merged
eiriktsarpalis merged 2 commits into
dotnet:mainfrom
artl93:artl93-stj-utf8-cache-double-toarray
Jul 14, 2026
Merged

Avoid duplicate PropertyRef[] allocation in UTF-8 property cache publish#130656
eiriktsarpalis merged 2 commits into
dotnet:mainfrom
artl93:artl93-stj-utf8-cache-double-toarray

Conversation

@artl93

@artl93 artl93 commented Jul 14, 2026

Copy link
Copy Markdown
Member

JsonTypeInfo.UpdateUtf8PropertyCache calls cacheBuilder.ToArray() twice: it
builds the PropertyRef[], asserts its length, then builds a second identical
array and publishes that one, discarding the first.

 PropertyRef[] newCache = cacheBuilder.ToArray();
 Debug.Assert(newCache.Length <= PropertyRefCacheBuilder.MaxCapacity);
-_utf8PropertyCache = cacheBuilder.ToArray();
+_utf8PropertyCache = newCache;

ToArray() allocates a fresh array each call and the builder isn't mutated between
the two calls, so the arrays are identical — behavior is unchanged and one
PropertyRef[] allocation per cache publish is removed.

Measured allocation impact

Allocation per UTF-8 property-cache publish, forcing a fresh JsonTypeInfo (new
DefaultJsonTypeInfoResolver) per deserialize so the publish path runs each op.
GC.GetAllocatedBytesForCurrentThread, 20,000 iters × 7 reps, .NET 11 preview, arm64:

Type property count (N) Baseline B/op Fixed B/op Saved
1 6809.7 6761.6 48 B
7 22886.6 22695.3 191 B
32 47522.3 46729.9 792 B

Each row's saving is exactly one eliminated PropertyRef[N] (24 B array header +
N × 24 B PropertyRef structs), scaling with the type's property count.

This is a publish/warm-up-path saving, bounded per JsonTypeInfo — STJ pools caching
contexts by options-equality, so steady-state repeated-serialization benchmarks show
it amortized to ~0/op. No steady-state or per-request regression or gain is claimed.

Validation

  • Behavior is byte-for-byte identical: the published array has the same contents,
    ordering, case-insensitivity, duplicate-property handling, escaping, and exception
    paths as before — only the redundant second allocation is removed.
  • Full System.Text.Json test suite passes locally (52,124 tests, 0 failures).

Concurrency

  • UpdateUtf8PropertyCache is optimistic last-writer-wins. The losing-race path
    (currentCache != cacheBuilder.OriginalCache) publishes nothing and allocates
    neither array — in both baseline and fixed. This change only touches the winning
    path, removing one duplicate array there.
  • A first-use race (18 threads × 5,000 rounds = 90,000 ops deserializing the same
    7-property type through one shared options instance) showed 0 correctness mismatches
    and 0 exceptions on both baseline and fixed.
  • Caveat: GC.GetAllocatedBytesForCurrentThread is thread-local, so the concurrency
    probe validates correctness, not per-op bytes; the allocation figures above come
    from the single-threaded probe.

Note

This PR description was drafted with assistance from GitHub Copilot.

UpdateUtf8PropertyCache called cacheBuilder.ToArray() twice, allocating
two identical PropertyRef[] arrays and discarding the first. Store the
result once and reuse it for both the length assertion and the cache
assignment. Behavior is unchanged; only the redundant allocation is removed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e3f810bd-cfc7-4046-84ce-23318cf5d1a3
Copilot AI review requested due to automatic review settings July 14, 2026 00:24
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes a redundant PropertyRef[] allocation when publishing JsonTypeInfo’s UTF-8 property-name cache by reusing the already-materialized array rather than calling PropertyRefCacheBuilder.ToArray() twice.

Changes:

  • Reuse newCache for _utf8PropertyCache publication instead of allocating/publishing a second identical array.
  • Preserve the existing capacity invariant check (Debug.Assert(newCache.Length <= PropertyRefCacheBuilder.MaxCapacity)).
Show a summary per file
File Description
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs Publishes the already-created PropertyRef[] (newCache) to avoid a duplicate ToArray() allocation during cache update.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@PranavSenthilnathan

Copy link
Copy Markdown
Member

This change looks good to have even if the perf numbers don't reflect it. The previous code is wastefully allocating an extra array.

@artl93 artl93 changed the title WIP: DO NOT REVIEW - STJ: avoid duplicate PropertyRef[] allocation in UTF-8 property cache publish (warm-up-only experiment) Avoid duplicate PropertyRef[] allocation in UTF-8 property cache publish Jul 14, 2026
@artl93

artl93 commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@EgorBot -intel -arm

using System.Text;
using System.Text.Json;
using System.Text.Json.Serialization.Metadata;
using BenchmarkDotNet.Attributes;

[MemoryDiagnoser]
public class JsonUtf8CachePublish
{
    private readonly byte[] _utf8 = Encoding.UTF8.GetBytes(
        "{\"A\":1,\"B\":\"b\",\"C\":\"c\",\"D\":4,\"E\":true,\"F\":6.0,\"G\":\"g\"}");

    // Fresh DefaultJsonTypeInfoResolver per op defeats STJ's options-equality
    // metadata caching, forcing a genuine UTF-8 property-cache publish each op.
    [Benchmark]
    public Poco Deserialize_FreshResolver()
    {
        var options = new JsonSerializerOptions { TypeInfoResolver = new DefaultJsonTypeInfoResolver() };
        return JsonSerializer.Deserialize<Poco>(_utf8, options)!;
    }

    public sealed class Poco
    {
        public int A { get; set; }
        public string? B { get; set; }
        public string? C { get; set; }
        public int D { get; set; }
        public bool E { get; set; }
        public double F { get; set; }
        public string? G { get; set; }
    }
}

Note

Benchmark request drafted with assistance from GitHub Copilot.

@eiriktsarpalis eiriktsarpalis marked this pull request as ready for review July 14, 2026 08:06
@eiriktsarpalis eiriktsarpalis enabled auto-merge (squash) July 14, 2026 08:06
Copilot AI review requested due to automatic review settings July 14, 2026 08:07
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants