Reduce virtual-thread context-propagation overhead on park/unpark

[amarziali]

What Does This Do

The virtual-thread instrumentation swapped the whole scope stack on every VirtualThread.mount()/unmount() (i.e. on every park/unpark).

The trace scope stack lives in a virtual-thread-aware ThreadLocal, so it follows the VT across park/unpark and carrier migration on its own. It only needs seeding once (first mount), never swapping again. The ddprof profiler context is different: it's keyed by the carrier OS thread, so it's re-bound on mount and cleared on unmount, but only when carrier-bound profiling is active.

I added few JMH bench

Results (per park/unpark, JDK 21)

Benchmark	Design · profiling	Throughput (ops/µs)	Alloc (B/op)
currentCycle_profilingOff	current · off	463.8 ± 251.7	176
proposedSteady_profilingOff	redesign · off	5010.4 ± 354.7	~0
currentCycle_profilingOn	current · on	272.3 ± 21.0	288
proposedRebindUnbind_profilingOn	redesign · on	1755.8 ± 473.9	~0

And the deltas:

Profiling	Throughput	Allocation
off	463.8 → 5010.4 ops/µs (~11×)	176 → ~0 B/op
on	272.3 → 1755.8 ops/µs (~6.5×)	288 → ~0 B/op

Motivation

Additional Notes

Contributor Checklist

• Format the title according to the contribution guidelines
• Assign the type: and (comp: or inst:) labels in addition to any other useful labels
• Avoid using close, fix, or any linking keywords when referencing an issue
  Use solves instead, and assign the PR milestone to the issue
• Update the CODEOWNERS file on source file addition, migration, or deletion
• Update public documentation with any new configuration flags or behaviors
• Add your completed PR to the merge queue by commenting /merge. You can also:
  • Customize the commit message associated with the merge with /merge --commit-message "..."
  • Remove your PR from the merge queue with /merge -c
  • Skip all merge queue checks with /merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)
  • Get more information in this doc

Jira ticket: [PROJ-IDENT]

[dd-octo-sts]

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite	Status
Startup	🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results

Scenario	Candidate	master	Δ (95% CI of mean)
startup:insecure-bank:iast:Agent	14.04 s	13.97 s	[-0.5%; +1.5%] (no difference)
startup:insecure-bank:tracing:Agent	12.90 s	13.05 s	[-1.7%; -0.4%] (maybe better)
startup:petclinic:appsec:Agent	16.87 s	16.71 s	[+0.0%; +2.0%] (maybe worse)
startup:petclinic:iast:Agent	16.35 s	16.82 s	[-7.2%; +1.6%] (no difference)
startup:petclinic:profiling:Agent	16.01 s	16.66 s	[-7.9%; +0.2%] (no difference)
startup:petclinic:sca:Agent	16.87 s	16.73 s	[-0.2%; +2.0%] (no difference)
startup:petclinic:tracing:Agent	16.01 s	16.03 s	[-1.1%; +0.8%] (no difference)

Commit: acd37e67 · CI Pipeline · Benchmarking Platform UI

---

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

[mcculls]

As discussed the inefficiency in ContinuableScopeManager.swap is an artefact of having to wrap the current ScopeStack in the returned response.

We have to do this because all we know is that the caller will eventually pass that same context back into swap  (that's the essential contract of this call.)

It's also why we need to create a new ScopeStack  for the incoming context - basically the caller is saying preserve the current state of the stack and restore it when I call swap again. This is essential for things like Kotlin Coroutines where the thread-binding is done above the level of JDK ThreadLocal's

The upcoming ThreadLocalContextManager does not have this overhead - it's basically a ThreadLocal holding the current context, so no allocations are required in swap. The benchmark when using ThreadLocalContextManager for the existing code is effectively the same as the proposed approach.

Given this I think it's valid to optimize the VirtualThread instrumentation in the short-term to avoid using swap when we know it's not needed and we can trust the ThreadLocal binding, with the understanding that this won't be an issue with ThreadLocalContextManager.

[PerfectSlayer]

Sphinx Review

(as we were discussing during the sync meeting, using this generated PR as exemple to demo the output)

MEDIUM (9)

2. Tests only exercise the no-op default (ProfilingContextIntegrationTest.java:35) — the tests would pass identically if the real carrier-bound setContext/clearContext implementation were deleted entirely.

3. DatadogProfilingIntegration.setContext untested (DatadogProfilingIntegration.java:83) — the actual ddprof rebind logic (AgentSpan.fromContext + contextManager.activate + null-span guard) never runs under test since instrumentation tests run with profiling off.

4. Unconditional Context.current() + dispatch on every mount (VirtualThreadState.java:49) — even with profiling disabled, every mount pays a ThreadLocal read + virtual dispatch, undercutting the PR's own hot-path goal. The plan's capability-gate would have skipped this.

5. Profiler-carrier-binding concern leaked into VT lifecycle helper (same line) — no explicit isCarrierThreadBound() gate; correctness relies on unrelated integrations inheriting no-op defaults.

6. Java-lang-21 forked test ordering is unenforced (VirtualThreadApiInstrumentationForkedTest.java:11) — relies on VirtualThreadState's static flag being captured after @WithConfig sets the system property, with nothing asserting that actually happened; a mis-capture would silently test the same branch twice.

7. Discarded context.swap() return value, no restore-on-unmount (VirtualThreadState.java:45) — legacy path never restores the carrier's prior context on unmount (pre-diff behavior always did). Needs an explicit justification comment or a restore.

8. Asymmetric clearContext() vs. guarded setContext() (VirtualThreadState.java:58) — onUnmount() clears unconditionally even when onMount() set nothing, doing extra native ddprof clears on the hot path.

9. Missing Javadoc on onMount()/onUnmount() (VirtualThreadState.java:41) — the new branch-dependent semantics (seed-once, no restore in legacy branch) aren't documented, unlike the prior version.

LOW (5)

10. Stale/broken field Javadocs (VirtualThreadState.java:30) — previousContext/context/seeded docs no longer match the reworked two-branch design; one is grammatically broken.
11. No Javadoc on new setContext(Context) override (DatadogProfilingIntegration.java:83).
12. clearContext() duplicates contextManager.close() body instead of delegating (DatadogProfilingIntegration.java:91) — asymmetric with setContext() → activate(); risks drift.
13. Carrier-migration test likely doesn't force migration (VirtualThreadLifeCycleTest.java:219) — sets jdk.virtualThreadScheduler.parallelism=2 at test time, but the JVM reads it only at scheduler init, which has likely already happened; test gives false confidence.
14. Two mutually-exclusive strategies conflated in one class (VirtualThreadState.java:31) — legacy vs. swap paths each leave one field permanently unused; worth documenting or splitting.

Bottom line: the core idea (seed-once instead of swap-every-park/unpark) is sound and the benchmarks back up the perf claim, but #7 (silently dropping restore-on-unmount) and #4/#8 (unconditional work even when profiling is off) are worth a maintainer's explicit sign-off before merge, and the test coverage gaps (#2, #3, #6, #13) mean the new carrier-rebind logic isn't actually exercised by CI yet.

[mcculls]

nit: the legacy context manager doesn't rebuild the whole scope stack, but it does wrap the stack and context together so the original stack can be restored when the context is swapped back.

[mcculls]

I assume it's ok to just clear the profiling context on unmount - but just want to be sure :)

[mcculls]

Thanks!