[Experiment] Reduce Activity Links/Events first-node allocation (depends on #130610)

[artl93]

Note

Draft/experimental. Depends on #130610 (see below). Not ready for review.

What this does

Co-locates the first ActivityLink/ActivityEvent with its list-container object on Activity, eliminating one heap allocation the first time a link or event is added to (or passed at creation of) an Activity. Same pattern as the tag co-location in #130610.

Dependency on #130610

This branch is stacked on #130610 ("Reduce Activity tag storage allocations") and is not rebased/flattened onto plain main:

• Current base: Reduce Activity tag storage allocations #130610's branch tip 6e0474c0bca (includes its review fixes 2a3ea726497b/ca8c39548fde, a merge of upstream main, a test-portability tweak, a lock-free-read write-ordering fix in TagsLinkedList.Remove, and an invariant-culture test fix).
• This PR's own commits on top: 2932592c7ac (Links/Events co-location), 14ab396b529 (added tests), 6ce0eed1498 (review fixes) — current head.

Update: #130610 fixed all 4 TagsLinkedList issues the reviewer bot flagged on this PR's diff (premature newNode allocation in Add/Set, stale Value retention in Remove and the empty-then-no-op SetTag(key, null) path) and merged current upstream main. This branch has been rebased three times (git rebase --onto) to track #130610's tip as it evolved (09b7f6b27332 → e8b8847af12 → 6e0474c0bca; the latest additions are a 1-line write-ordering fix in TagsLinkedList.Remove for lock-free readers, plus test-only tweaks — no impact on this PR's own Links/Events code), so the diff shown here is now just this PR's own Links/Events layer versus #130610's current state — re-verify via gh pr diff since GitHub still computes the PR diff against dotnet/runtime:main (GitHub can't retarget a PR's base to a fork-only branch), so it will keep including #130610's full commit history until #130610 itself merges into dotnet/runtime:main. All 6 reviewer-bot threads on this PR (4 pointing at #130610's code, 2 on this PR's own tests) are now resolved.

Merge-order note: do not merge this PR before #130610 — the tag co-location and the link/event co-location are independent, additive savings, and this PR should not be treated as validation of #130610. Correct order: merge #130610 first, then retarget/flatten this PR onto resulting main and drop the duplicated tag-layer commits so it contains only link/event-specific changes.

Full test suite re-verified after the rebase: 462/462 passed (-c Debug -f net11.0).

Design

Activity._links/_events were each a DiagLinkedList<T>, where every node (DiagNode<T>) is a separately heap-allocated wrapper around one T plus a Next pointer. A list with exactly one entry — the common case: most OTel spans have 0-1 parent link and 0-1 event — allocates two objects (container + first node). ActivityLinksLinkedList/ActivityEventsLinkedList embed the first value as a field on the container itself, so the container is the first node; 2nd+ entries still allocate a DiagNode<T> as before. Enumeration order and ToString() output are unchanged (verified by regression tests; formatting shared via extracted ActivityLinkedListFormatting, used by both the new types and the legacy DiagLinkedList<T>.ToString() still used by System.Diagnostics.Metrics).

The disabled/unsampled path (ActivitySamplingResult.None) is untouched — Activity.Create isn't called at all in that case, so _links/_events storage is never allocated either way.

Canonical component-level allocation evidence

Deterministic per-operation allocation delta via GC.GetAllocatedBytesForCurrentThread(), 20,000 iterations after 2,000-iteration warmup, forced GC before measurement. Same worktree/build across three exact SHAs (only Activity.cs/DiagLinkedList.cs swapped + rebuilt), net11.0, Debug config.

Note: the SHAs in this table (7aa830a0359/ca551806834/ff2ab8a0664) predate the rebase onto #130610's updated tip above and no longer exist on this branch (superseded by 09b7f6b27332/6ea1697b4a7). The measured deltas are unaffected — the rebase only changed which commit each layer sits on top of, not the layers' own code — but a fresh re-run against the current SHAs is still outstanding.

Scenario	Upstream (7aa830a0359)	Round-1-only (ca551806834)	Stacked/this PR (ff2ab8a0664)	Δ Round-1→Stacked	Δ Upstream→Stacked
NoActivity (control)	0	0	0	0	0
Zero links, zero events	416	416	416	0	0
Single link	608	608	584	-24	-24
Many links (3)	672	672	648	-24	-24
Single event	504	504	480	-24	-24
Many events (3)	616	616	592	-24	-24
OTel shape (5 tags + 1 link + 1 event)	984	952	904	-48	-80
Disabled/unsampled listener + 1 link	64	64	64	0	0

-24 B per non-empty links list and -24 B per non-empty events list, regardless of how many entries follow the first (only the first node is affected). Disabled/unsampled path is allocation-neutral (64 B/op, all three SHAs). Combined with Round-1's tag savings: -80 B/op on a typical OTel-shaped span vs. upstream.

Caveats: Debug config (Release testhost layout was incomplete in this worktree — allocation site count shouldn't differ by JIT tier, but a Release re-run is still pending); no BenchmarkDotNet run yet (standalone BDN console project fails outside full-repo context due to source generators — in-repo deterministic probe used instead, consistent with the prior round's approach); no E2E/Kestrel run (net11.0-local vs. net10.0-installed-runtime TFM mismatch, optional per scope); single-threaded only (concurrency correctness covered separately by xunit regression tests below, not by this allocation harness). Harness source is archived (available on request) but was not committed (temporary probe, not a permanent test).

BenchmarkDotNet run requested below via @EgorBot; will post results here once available.

Tests

8 regression tests added to ActivityTests.cs: ToString() equivalence (links-at-creation vs. AddLink; empty/single/multi-event formatting), an 8-thread concurrent AddLink+AddEvent stress test (no lost entries, Barrier-synchronized and disposed), Activity.Create's multi-link enumerator path, links/events/tags independence, tag-data-type round-tripping on the co-located first event/link ([Theory], null/string/int), and the no-Activity-created path for ActivitySamplingResult.None.

Full System.Diagnostics.DiagnosticSource.Tests suite: 462/462 passed post-rebase (-c Debug -f net11.0; count is higher than the previously reported 457 because #130610 also added its own new tests, now included via the rebase).

Alternatives / out of scope

A small fixed-capacity inline array (e.g. first 2-3 elements) was considered instead of just 1 — not pursued to keep this minimal and parallel to the already-reviewed tags pattern. Activity.Baggage, ActivityContext/ActivityTraceId/ActivitySpanId formatting/parsing, and DiagnosticListener.Write dispatch overhead were investigated during the broader profiling pass and found either already efficient or not clearly justified as a follow-on at this time — out of scope here.

Note

This PR description and the linked evidence/analysis were prepared with AI assistance (GitHub Copilot). The author (@artl93) is coordinating this experiment and has reviewed the content.

[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 changes Activity’s internal storage for ActivityLink and ActivityEvent (and its already-stacked tag work) to co-locate the first node with the list container, aiming to avoid the extra heap allocation normally incurred by allocating a first DiagNode<T>.

Changes:

• Replace Activity’s _links / _events storage from DiagLinkedList<T> to co-located first-node list containers (ActivityLinksLinkedList / ActivityEventsLinkedList).
• Refactor DiagLinkedList<T>.ToString() formatting to use shared helpers (ActivityLinkedListFormatting) reused by the new co-located list types.
• Add new tests covering ToString equivalence and basic concurrent add stress for tags/links/events.

Show a summary per file

File	Description
src/libraries/System.Diagnostics.DiagnosticSource/tests/ActivityTests.cs	Adds regression/stress tests for tag/link/event behavior and formatting.
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagLinkedList.cs	Makes DiagNode<T> inheritable and factors link/event formatting into shared helpers used by multiple list implementations.
src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs	Introduces co-located first-node list containers for links/events and updates tag list to inherit from DiagNode<T>.

Copilot's findings

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

[Copilot]

Copilot's findings

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

[Copilot]

Copilot's findings

Comments suppressed due to low confidence (2)

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs:1694

• TagsLinkedList can represent an empty state via _last is null (e.g., when SetTag(key, null) is the first operation). With the new DiagNode<> inheritance, the constructor always stores firstValue into Value, which retains the tag key/value even when the logical list is empty. This is a memory-retention regression vs the previous _first = null representation. Consider initializing the base node with default and only assigning Value when the list is actually non-empty.

            public TagsLinkedList(KeyValuePair<string, object?> firstValue, bool set = false) : base(firstValue)
            {
                if (!set || firstValue.Value is not null)
                {
                    _last = this;
                }
            }

src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs:1790

• TagsLinkedList now uses the container object as the first node; when removing the last remaining tag (next is null), the list becomes logically empty (_last = null) but Value still holds the removed key/value, which keeps those objects alive as long as the Activity is alive. Clear the embedded node value when transitioning to empty to avoid retaining the last removed tag.

                        DiagNode<KeyValuePair<string, object?>>? next = Next;
                        if (next is null)
                        {
                            _last = null;
                        }

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

[artl93]

Note

Benchmark code below was written with AI assistance (GitHub Copilot).

@EgorBot -linux_amd -osx_arm64

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

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

[MemoryDiagnoser]
public class Bench
{
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private ActivityLink _link;
    private ActivityEvent _event;

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

        _link = new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded));
        _event = new ActivityEvent("cache-miss");
    }

    [Benchmark(Baseline = true)]
    public Activity? NoLinksNoEvents()
    {
        Activity? a = _source.StartActivity("op");
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAtCreation()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneEventAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddLink(_link);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? LinkPlusEvent()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }
}

[Copilot]

Copilot's findings

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

[artl93]

Note

Benchmark code below was written with AI assistance (GitHub Copilot). Re-posting after a rebase onto #130610's updated head (previous request was against the pre-rebase commit and never got a response).

@EgorBot -linux_amd -osx_arm64

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

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

[MemoryDiagnoser]
public class Bench
{
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private ActivityLink _link;
    private ActivityEvent _event;

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

        _link = new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded));
        _event = new ActivityEvent("cache-miss");
    }

    [Benchmark(Baseline = true)]
    public Activity? NoLinksNoEvents()
    {
        Activity? a = _source.StartActivity("op");
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAtCreation()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneEventAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddLink(_link);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? LinkPlusEvent()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }
}

[artl93]

Stack interaction update

PR #130610 is now the independent tag-node base PR and has been refreshed onto current main. This link/event PR still carries copied/older versions of the #130610 commits and overlaps the same three files.

Required merge order:

1. Merge Reduce Activity tag storage allocations #130610 first.
2. Rebase/retarget this PR onto the resulting main.
3. Drop the duplicated tag commits so this PR contains only link/event-specific changes.

Do not merge this PR first: that would implicitly ship the tag change before its independent PR is complete. The link/event savings are separate and additive; they should not be treated as validation of #130610.

Note

This stack clarification was generated with GitHub Copilot.

[artl93]

Note

Benchmark code below was written with AI assistance (GitHub Copilot). Superseding the previous two requests, which used the undocumented -linux_amd flag — correcting to -amd.

@EgorBot -amd -osx_arm64

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

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

[MemoryDiagnoser]
public class Bench
{
    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private ActivityLink _link;
    private ActivityEvent _event;

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

        _link = new ActivityLink(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded));
        _event = new ActivityEvent("cache-miss");
    }

    [Benchmark(Baseline = true)]
    public Activity? NoLinksNoEvents()
    {
        Activity? a = _source.StartActivity("op");
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAtCreation()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneEventAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? OneLinkAddedPostCreation()
    {
        Activity? a = _source.StartActivity("op");
        a?.AddLink(_link);
        a?.Stop();
        return a;
    }

    [Benchmark]
    public Activity? LinkPlusEvent()
    {
        Activity? a = _source.StartActivity("op", ActivityKind.Internal, default(ActivityContext), links: new[] { _link });
        a?.AddEvent(_event);
        a?.Stop();
        return a;
    }
}

[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:2862

• This test can deadlock/hang on runtimes where multithreading isn’t supported (e.g., single-threaded builds) because it uses Task.Run + Barrier synchronization. Similar multi-threaded tests in this file are guarded with ConditionalFact on PlatformDetection.IsMultithreadingSupported; this one should be too.

        [Fact]

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