Skip to content

[perf] Cache validated W3C tracestate per-thread - #134648

Open
martincostello wants to merge 2 commits into
dotnet:mainfrom
martincostello:w3c-tracestate-caching
Open

martincostello wants to merge 2 commits into
dotnet:mainfrom
martincostello:w3c-tracestate-caching

Conversation

@martincostello

@martincostello martincostello commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

ValidateTraceState re-parses and rebuilds the tracestate value on every call, even when Activity.TraceStateString is typically set once for the lifetime of a span and its children. A single incoming request that fanned out to several outgoing calls would redo the same work work for each.

Adds a last-(input,output)-pair cache to skip re-parsing when the value hasn't changed since the last call on the same thread.

Benchmarks

BenchmarkDotNet v0.15.4, Windows 11 (10.0.26200.9457)
13th Gen Intel Core i7-13700H 2.90GHz, 1 CPU, 20 logical and 14 physical cores
.NET SDK 11.0.100-rc.1.26420.103
  [Host] : .NET 11.0.0 (11.0.0-rc.1.26420.103, 11.0.26.42103), X64 RyuJIT x86-64-v3

Job=MediumRun  Toolchain=InProcessEmitToolchain  IterationCount=15  LaunchCount=2  WarmupCount=10
Build Method Mean Error StdDev Gen0 Allocated
main Inject 163.0 ns 7.20 ns 10.77 ns 0.0119 152 B
PR Inject 28.4 ns 1.09 ns 1.60 ns - -

Models one downstream HttpClient call's header injection when the current span's TraceStateString is unchanged from a prior call in the same fan-out.

Benchmark code
using System.Diagnostics;
using BenchmarkDotNet.Attributes;

namespace System.Diagnostics.Microbenchmarks;

[MemoryDiagnoser]
public class W3CTraceStateInjectBenchmarks
{
    private readonly DistributedContextPropagator _propagator = DistributedContextPropagator.CreateDefaultPropagator();
    private readonly Dictionary<string, string> _carrier = new();
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private Activity _activity = null!;

    private static readonly DistributedContextPropagator.PropagatorSetterCallback s_setter =
        static (object? carrier, string name, string value) => ((Dictionary<string, string>)carrier!)[name] = value;

    [GlobalSetup]
    public void Setup()
    {
        _source = new ActivitySource(nameof(W3CTraceStateInjectBenchmarks));
        _listener = new ActivityListener
        {
            ShouldListenTo = _ => true,
            Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData,
        };
        ActivitySource.AddActivityListener(_listener);

        _activity = _source.StartActivity("incoming-request")!;
        _activity.TraceStateString = "congo=t61rcWkgMzE,rojo=00f067aa0ba902b7,vendor3=abcdef1234567890";
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        _activity.Dispose();
        _source.Dispose();
        _listener.Dispose();
    }

    [Benchmark]
    public void Inject() => _propagator.Inject(_activity, _carrier, s_setter);
}

I also had Claude Code verify that the changes work well across many requests like in an HTTP server to avoid cache thrashing. The code and results for that are shown below.

Scenario main (3 runs) main median PR (3 runs) PR median
8t, burst=1 119.1 / 108.1 / 121.0 ns 119.1 ns 162.6 / 131.2 / 101.2 ns 131.2 ns
8t, burst=3 324.6 / 159.5 / 341.2 ns 324.6 ns 245.4 / 293.3 / 125.5 ns 245.4 ns
8t, burst=10 76.4 / 352.1 / 86.1 ns 86.1 ns 168.5 / 163.7 / 216.4 ns 168.5 ns
16t, burst=1 171.0 / 247.5 / 136.3 ns 171.0 ns 152.9 / 138.9 / 236.4 ns 152.9 ns
16t, burst=3 290.6 / 245.3 / 228.2 ns 245.3 ns 199.1 / 200.6 / 209.8 ns 200.6 ns
16t, burst=10 664.6 / 353.9 / 332.8 ns 353.9 ns 273.4 / 266.3 / 274.2 ns 273.4 ns
Harness code
using System.Diagnostics;

// Models two things:
//  1. The originally-raised scenario: many concurrent threads, each with its own distinct
//     tracestate, racing against a shared cache slot (addressed by using [ThreadStatic]).
//  2. Review finding: a real thread-pool thread doesn't use one tracestate forever - it serves a
//     SEQUENCE of different requests over its lifetime, each request making a 'burst' of a few
//     fan-out calls with the SAME value before the thread moves on to a different request (and a
//     different value). This models that with a per-thread pool of 'valuesPerThread' distinct
//     values, held for 'burstSize' consecutive calls each before cycling to the next.
//     burstSize=1 is the worst case (every call is a different request - always a cache miss); a
//     large burstSize approaches the (unrealistically generous) "one value forever" case.

DistributedContextPropagator propagator = DistributedContextPropagator.CreateDefaultPropagator();
var setter = new DistributedContextPropagator.PropagatorSetterCallback(
    static (object? carrier, string name, string value) => ((Dictionary<string, string>)carrier!)[name] = value);

const int IterationsPerThread = 100_000;

double RunScenario(string label, int threadCount, int valuesPerThread, int burstSize)
{
    // Each scenario allocates on the order of hundreds of MB total across its threads (more for
    // the no-cache baseline). Settle the heap first so one scenario's GC pressure doesn't bleed
    // into the next scenario's timing within the same process.
    GC.Collect(2, GCCollectionMode.Forced, blocking: true, compacting: true);
    GC.WaitForPendingFinalizers();
    GC.Collect(2, GCCollectionMode.Forced, blocking: true, compacting: true);

    var activitiesPerThread = new Activity[threadCount][];
    var carriers = new Dictionary<string, string>[threadCount];
    using var source = new ActivitySource("contention-check");
    using var listener = new ActivityListener
    {
        ShouldListenTo = _ => true,
        Sample = (ref ActivityCreationOptions<ActivityContext> _) => ActivitySamplingResult.AllData,
    };
    ActivitySource.AddActivityListener(listener);

    for (int t = 0; t < threadCount; t++)
    {
        var activities = new Activity[valuesPerThread];
        for (int v = 0; v < valuesPerThread; v++)
        {
            Activity activity = source.StartActivity("op")!;
            // Every (thread, value-slot) combination gets its own distinct tracestate, so no two
            // threads and no two values within a thread's rotation ever coincidentally collide.
            activity.TraceStateString = $"thread{t}_v{v}=t61rcWkgMzE{v},rojo{v}=00f067aa0ba902b7{v},vendor3_{v}=abcdef1234567890{v}";
            activities[v] = activity;
        }
        activitiesPerThread[t] = activities;
        carriers[t] = new Dictionary<string, string>();
    }

    var threads = new Thread[threadCount];
    var barrier = new Barrier(threadCount + 1);
    long[] allocatedPerThread = new long[threadCount];

    for (int t = 0; t < threadCount; t++)
    {
        int threadIndex = t;
        threads[t] = new Thread(() =>
        {
            Activity[] activities = activitiesPerThread[threadIndex];
            Dictionary<string, string> carrier = carriers[threadIndex];

            barrier.SignalAndWait(); // release point

            long before = GC.GetAllocatedBytesForCurrentThread();
            int valueIndex = 0;
            int burstRemaining = burstSize;
            for (int i = 0; i < IterationsPerThread; i++)
            {
                propagator.Inject(activities[valueIndex], carrier, setter);

                if (--burstRemaining == 0)
                {
                    valueIndex = (valueIndex + 1) % activities.Length;
                    burstRemaining = burstSize;
                }
            }
            long after = GC.GetAllocatedBytesForCurrentThread();
            allocatedPerThread[threadIndex] = after - before;
        });
        threads[t].Start();
    }

    barrier.SignalAndWait(); // main thread releases all workers at (roughly) the same instant
    var sw = Stopwatch.StartNew();
    foreach (Thread th in threads) th.Join();
    sw.Stop();

    foreach (Activity[] activities in activitiesPerThread)
        foreach (Activity a in activities)
            a.Dispose();

    long totalAllocated = 0;
    foreach (long a in allocatedPerThread) totalAllocated += a;

    long totalOps = (long)threadCount * IterationsPerThread;
    double nsPerOp = sw.Elapsed.TotalMilliseconds * 1_000_000 / totalOps;
    double bytesPerOp = totalAllocated / (double)totalOps;

    Console.WriteLine($"{label,-58} threads={threadCount,-3} {sw.ElapsedMilliseconds,6} ms total  {nsPerOp,8:F1} ns/op  {bytesPerOp,8:F1} B/op  (total alloc {totalAllocated / 1024.0:F0} KB)");
    return nsPerOp;
}

RunScenario("(warmup)", 1, 1, int.MaxValue);

Console.WriteLine("--- Cross-thread isolation only (one value per thread, forever - NOT a server-workload claim) ---");
RunScenario("Single thread, same value (best case)", 1, 1, int.MaxValue);
RunScenario("8 threads,  1 value/thread forever", 8, 1, int.MaxValue);
RunScenario("16 threads, 1 value/thread forever", 16, 1, int.MaxValue);

Console.WriteLine("--- Realistic thread-pool reuse: each thread cycles through 8/16 distinct values ---");
RunScenario("8 threads,  8 values/thread, burst=1", 8, 8, 1);
RunScenario("8 threads,  8 values/thread, burst=3", 8, 8, 3);
RunScenario("8 threads,  8 values/thread, burst=10", 8, 8, 10);
RunScenario("16 threads, 16 values/thread, burst=1", 16, 16, 1);
RunScenario("16 threads, 16 values/thread, burst=3", 16, 16, 3);
RunScenario("16 threads, 16 values/thread, burst=10", 16, 16, 10);

`ValidateTraceState` re-parses and rebuilds the `tracestate` value on every call, even when `Activity.TraceStateString` is typically set once for the lifetime of a span and its children. A single incoming request that fanned out to several outgoing calls would redo the same work work for each.

Adds a last-(input,output)-pair cache to skip re-parsing when the value hasn't changed since the last call on the same thread.
@martincostello
martincostello requested a balanced review from Copilot September 25, 2026 10:49
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 25, 2026
@azure-pipelines

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

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/area-system-diagnostics-tracing
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.

Copilot review overview

🟡 Changes recommended

Unbounded raw tracestate values can remain rooted per thread, and the server-workload claim needs representative validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Caches validated W3C tracestate values per thread to reduce repeated parsing and allocation during propagation.

Changes:

  • Adds a thread-local input/output cache.
  • Separates cached validation from core parsing.

Assessment: The optimization is focused, but retains unbounded raw input and the contention benchmark does not model cache misses.

File Description
W3CPropagator.cs Adds per-thread tracestate validation caching.

Do not cache values larger than the maximum allowed length.

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.

Copilot review overview

🟢 Approval recommended

The optimization preserves validation behavior and has focused benchmark evidence; the remaining field-placement comment is non-blocking.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Low severity Move thread-static fields before methods

src/​libraries/​System.Diagnostics.DiagnosticSource/​src/​System/​Diagnostics/​W3CPropagator.cs:162

Please place these thread-static fields with the other fields/constants at the top of W3CPropagator. Declaring fields between methods makes the type layout harder to scan and conflicts with this repository's C# rule that fields are declared before other members.

@martincostello
martincostello marked this pull request as ready for review September 25, 2026 11:41

This branch has not been deployed

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

Labels

area-System.Diagnostics.Tracing community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants