Reduce Activity tag storage allocations

[artl93]

Motivation

A sampled Activity currently allocates both a TagsLinkedList container and a separate first DiagNode<KeyValuePair<string, object?>> when its first tag is recorded. OpenTelemetry-style server/client spans commonly carry semantic-convention tags, so this redundant object is paid by nearly every exported span.

Change

Make TagsLinkedList inherit DiagNode<KeyValuePair<string, object?>>, so the list container is also its first node. Later tags continue to use the existing linked-node representation.

The change preserves tag order, duplicate handling, lookup, mutation, enumeration, string/object/null formatting, listener behavior, and the public API. Empty storage clears ignored or removed keys/values. Repopulating an empty list reuses the co-located node without allocating and discarding a node, enumerable inputs dispose their enumerator, and empty state is published before clearing the stored value for lock-free readers.

BDN + EgorBot

Exact paired local BenchmarkDotNet run:

• Base: c1b09d933f7d720f7ae50f80b4ec6980bb93d96f
• Benchmarked product revision: f4564b1b85c6328e94c18a240af7e976d733bb90
• Current PR head: 6e0474c0bca (invariant-culture test-only follow-up; product source is identical to the benchmarked revision)
• BenchmarkDotNet 0.16.0-preview.1, .NET 11, Apple M5 Pro/macOS arm64
• 3 launches, 8 warmups, 12 measured iterations

Tags	Base allocation	Final allocation	Delta	Base mean	Final mean
0	328 B	328 B	0 B	84.70 ns	84.66 ns
1	408 B	376 B	-32 B	97.57 ns	89.39 ns
5	568 B	536 B	-32 B	148.55 ns	141.51 ns
20	1168 B	1136 B	-32 B	477.59 ns	462.94 ns

No-listener and listener-present-but-unsampled controls allocate 0 B in both revisions and are effectively unchanged in the final run. The deterministic acceptance result is one fewer 32-byte object per tagged Activity.

Exactly one current-head cross-platform EgorBot request (-amd -osx_arm64) is posted in this comment. EgorBot acknowledged it; results are pending.

Local evidence provenance:

• Base DLL SHA-256: fbcdac15a0c5252cf75fc7fdd5f282b75e1ce0da74808678fb933286d01d6e40
• Final DLL SHA-256: 2dc17805a732615a306e40a5bd5821c8a3d3af4adaec218fb197550d55fd168a
• Full paired BDN log SHA-256: 5cb691bdf1c61bbeda3c0bb33811512200bdbb904f7b002d8fd5eca8e1b363d0
• Raw Markdown/CSV/full JSON reports are preserved for tags, no-listener, and unsampled controls.

Validation

./dotnet.sh build src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj --no-restore
./dotnet.sh build /t:test src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj --no-restore /p:XUnitOptions="-class System.Diagnostics.Tests.ActivityTests"
./dotnet.sh build /t:test src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj --no-restore

• Product build: succeeded, 0 warnings/errors
• Focused ActivityTests: 102 passed
• Full DiagnosticSource suite: 452 passed
• Added coverage for concurrent first-tag publication, empty/remove/repopulate storage, ignored null sets, enumerator disposal paths, culture-independent stress data, and string/object/null formatting
• All review threads are resolved
• Independent skeptical final reviews found no code defect and confirmed the [Experiment] Reduce Activity Links/Events first-node allocation (depends on #130610) #130617 merge-order disclosure below

Risks

• The representation is internal, but it relies on _last is null as the empty-list invariant; dedicated tests pin empty/removal/repopulation behavior and cleared references.
• Lock-free reads retain the same best-effort concurrency semantics as before; mutation remains serialized by the existing list lock, and first publication still uses Interlocked.CompareExchange.
• Follow-on PR [Experiment] Reduce Activity Links/Events first-node allocation (depends on #130610) #130617 applies the same first-node co-location pattern to links/events and overlaps these same three files. This PR must merge first. PR [Experiment] Reduce Activity Links/Events first-node allocation (depends on #130610) #130617 should then be rebased/retargeted onto current main and drop its copied/older tag commits; it must not be merged first. Its link/event allocation savings are separate and additive, not evidence for this tag change. The reciprocal stack note is posted on [Experiment] Reduce Activity Links/Events first-node allocation (depends on #130610) #130617.
• Exact-head CI completed 2,232,142 tests. Two unrelated known failures remain in raw checks: TransactionManagerTest.DefaultTimeout_MaxTimeout_Set_Get (#105124) and the x86 Zstd OOM (#130948). Build Analysis succeeded and matched both KBEs. The raw runtime check remains red pending a failed-job rerun; EgorBot results are also pending. This PR remains draft.

AI disclosure

Note

This PR implementation, benchmarks, and description were prepared with GitHub Copilot assistance and reviewed by the author.

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @dotnet/area-system-diagnostics-tracing
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR experiments with reducing allocations in System.Diagnostics.Activity tag storage by co-locating the first tag node with the tag-list container and updating tag formatting logic.

Changes:

• Refactors Activity.TagsLinkedList to inherit from DiagNode<KeyValuePair<string, object?>>, eliminating a separate first-node allocation.
• Updates TagsLinkedList.ToString() to use ValueStringBuilder instead of a cached StringBuilder.
• Adds a new test asserting concurrent AddTag calls preserve all tag values.

Show a summary per file

File	Description
src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs	Adds a new concurrent-add tags test.
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagLinkedList.cs	Makes DiagNode<T> non-sealed to enable deriving the tag list from it.
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs	Implements the co-located-first-node tag list and updates tag-list string formatting.

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 3

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 2

[Copilot]

Copilot's findings

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

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (1)

src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs:242

• This test uses Parallel.For and assumes multithreading support, but it is a plain [Fact]. The file already guards other multithreaded tests with PlatformDetection.IsMultithreadingSupported; without the guard, this can fail on threadless targets (e.g., some WASM/mobile configurations).

        [Fact]
        public void ConcurrentTagAddsPreserveAllValues()
        {
            const int Count = 1_000;
            Activity activity = new Activity("activity");

            Parallel.For(0, Count, i => activity.AddTag(i.ToString(), i));

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

[Copilot]

Copilot's findings

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

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 2

[artl93]

@EgorBot -amd -osx_arm64

using System.Diagnostics;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

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

[MemoryDiagnoser]
public class ActivityTagBenchmarks
{
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private ActivityContext _parent;
    private KeyValuePair<string, object?>[] _stringTags = null!;
    private KeyValuePair<string, object?>[] _objectTags = null!;

    [Params(0, 1, 5, 20)]
    public int TagCount { get; set; }

    [Params(false, true)]
    public bool ObjectValues { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _source = new ActivitySource("EgorBot.Activity.Tags");
        _parent = ActivityContext.Parse(
            "00-0123456789abcdef0123456789abcdef-0123456789abcdef-01",
            "vendor=value");
        _stringTags = Enumerable.Range(0, 20)
            .Select(static i => new KeyValuePair<string, object?>($"tag.{i}", $"value.{i}"))
            .ToArray();
        _objectTags = Enumerable.Range(0, 20)
            .Select(static i => new KeyValuePair<string, object?>($"tag.{i}", new TagValue(i)))
            .ToArray();
        _listener = new ActivityListener
        {
            ShouldListenTo = static source => source.Name == "EgorBot.Activity.Tags",
            Sample = static (ref ActivityCreationOptions<ActivityContext> _) =>
                ActivitySamplingResult.AllDataAndRecorded,
        };
        ActivitySource.AddActivityListener(_listener);
    }

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

    [Benchmark]
    public Activity? StartStopWithTags()
    {
        Activity? activity = _source.StartActivity("request", ActivityKind.Server, _parent);
        KeyValuePair<string, object?>[] tags = ObjectValues ? _objectTags : _stringTags;
        for (int i = 0; i < TagCount; i++)
        {
            KeyValuePair<string, object?> tag = tags[i];
            activity!.SetTag(tag.Key, tag.Value);
        }
        activity?.Stop();
        return activity;
    }

    private sealed class TagValue(int value)
    {
        public override string ToString() => $"object-{value}";
    }
}

[MemoryDiagnoser]
public class InactiveActivityBenchmarks
{
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;

    [Params(false, true)]
    public bool ListenerPresent { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _source = new ActivitySource(
            ListenerPresent
                ? "EgorBot.Activity.Unsampled"
                : "EgorBot.Activity.Disabled");
        _listener = new ActivityListener
        {
            ShouldListenTo = static source => source.Name == "EgorBot.Activity.Unsampled",
            Sample = static (ref ActivityCreationOptions<ActivityContext> _) =>
                ActivitySamplingResult.None,
        };
        ActivitySource.AddActivityListener(_listener);
    }

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

    [Benchmark]
    public Activity? StartActivity() => _source.StartActivity("request");
}

Note

This benchmark request was generated with GitHub Copilot.

[Copilot]

Copilot's findings

• Files reviewed: 3/3 changed files
• Comments generated: 4

[artl93]

This remains work in progress and draft. The exact-head implementation, local BDN, focused/full DiagnosticSource tests, and review-thread pass are complete. Build Analysis is green and classifies the two raw CI failures as known issues (#105124 and #130948); the raw failed jobs still need a rerun. EgorBot acknowledged the sole current-head request but has not posted results. I will not mark the PR ready while those external gates remain.

Note

This status update was prepared with GitHub Copilot assistance and reviewed by the author.