Reduce virtual-thread context-propagation overhead on park/unpark - #11893
gh-worker-dd-mergequeue-cf854d[bot] merged 21 commits into
Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
As discussed the inefficiency in 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 The upcoming Given this I think it's valid to optimize the VirtualThread instrumentation in the short-term to avoid using |
d80872e to
b05f030
Compare
Sphinx Review(as we were discussing during the sync meeting, using this generated PR as exemple to demo the output) MEDIUM (9)
LOW (5)
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. |
PerfectSlayer
left a comment
PerfectSlayer
left a comment
There was a problem hiding this comment.
I left a bunch of suggestions 🙏
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 498ed03df5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
amarziali
added
type: feature
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b38147c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b38147c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
47c3097 to
76c8e26
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@DataDog review |
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The PR reduces virtual-thread park and unpark work.
JDK 22 and later seed trace context once and rebind ddprof on each mount.
JDK 21 now keeps the safe per-mount path.
The prior update fixes the benchmark model and states its native-call limit.
No open finding remains.
🤖 Datadog Autotest · Commit 76c8e26 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
jbachorik
left a comment
jbachorik
left a comment
There was a problem hiding this comment.
Looks reasonable. LGTM!
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #11893...
Detected 1 merge blocker(s) to address: 🟠 Pending
|
bric3
left a comment
bric3
left a comment
There was a problem hiding this comment.
LGTM, I suggest a few comment tweaks.
FYI here's the schema I used to understand the code:
flowchart TD
E[Virtual-thread lifecycle event] --> P{Per-mount context required?}
P -->|JDK 21, CWS enabled,<br/>or legacy manager disabled| OLD
OLD[Swap saved Context<br/>on every mount and unmount]
P -->|JDK 22+, legacy manager,<br/>CWS disabled| NEW
NEW[Initialize inherited trace context once<br/>around VirtualThread.run]
NEW --> Q{Profiler binding required?}
Q -->|Yes| PROF[Bind current context on mount<br/>and root on unmount]
Q -->|No| NONE[No context operation<br/>during park/unpark]
sequenceDiagram
participant VT as java.lang.VirtualThread
participant Store as ContextStore
participant State as VirtualThreadState
participant Ctx as Context
participant Prof as ProfilingContextIntegration
Note over VT,Prof: Calls from VirtualThread include the injected instrumentation advice
alt JDK 21 OR legacy context manager disabled OR CWS enabled
Note over VT,Ctx: Existing behavior retained: swap context on every mount and unmount
loop Each execution segment, from mount to unmount
VT->>VT: mount()
VT->>Store: get(virtualThread)
Store-->>VT: state
VT->>State: onMount()
State->>Ctx: context.swap()
Ctx-->>State: previousContext
Note over State,Prof: Normal context/scope activation callbacks run
VT->>VT: Execute or resume application code
Note over VT,State: Before unmount(), while the virtual thread is still current
VT->>Store: get(virtualThread)
Store-->>VT: state
VT->>State: onUnmount()
State->>Ctx: previousContext.swap()
Ctx-->>State: Save returned context for the next mount
VT->>VT: unmount()
end
else JDK 22+ AND legacy context manager enabled AND CWS disabled
Note over VT,Prof: New behavior: Initialize inherited trace context once, Update the carrier’s profiler binding on every mount.
VT->>VT: First mount()
VT->>State: onMountWithoutStore()
opt Carrier binding required
State->>Prof: setContext(Context.current())
Note right of Prof: Initially root:<br/>captured context is not installed yet
end
Note over VT,State: Enter private run(Runnable), after the first mount
VT->>Store: get(virtualThread)
Store-->>VT: state
Note over VT,State: Keep state in an advice local across suspension
VT->>State: onRun()
State->>Ctx: captured context.swap()
Ctx-->>State: previousContext
opt Profiling enabled
Ctx->>Prof: Scope activation binds the captured span
end
VT->>VT: Start application code
loop Each unmount/remount cycle (can happen repeatitively in the VT lifetime)
Note over VT,State: For example:<br/>doSomeWork()<br/>Thread.sleep(100)<br/>doSomeWork()
VT->>State: onUnmountWithoutStore()
opt Carrier binding required
State->>Prof: setContext(Context.root())
end
VT->>VT: unmount()
Note over VT,Ctx: Trace scope stack remains in the virtual thread's ThreadLocal
VT->>VT: mount(), possibly on another carrier
VT->>State: onMountWithoutStore()
opt Carrier binding required
State->>Ctx: Context.current()
Ctx-->>State: Current context, including any active child span
State->>Prof: setContext(current context)
end
VT->>VT: Resume application code
end
Note over VT,State: Exit run(Runnable), including exceptional exit
VT->>State: afterRun()
State->>Ctx: previousContext.swap()
VT->>State: onUnmountWithoutStore()
opt Carrier binding required
State->>Prof: setContext(Context.root())
end
VT->>VT: Final unmount()
end
Note over VT,State: Termination advice: afterDone / afterTerminate
VT->>Store: remove(virtualThread)
Store-->>VT: state
VT->>State: onTerminate()
State->>Ctx: continuation.release()
Co-authored-by: Brice Dutheil <[email protected]>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
8 participants
What Does This Do
Reduces virtual-thread context-propagation overhead on JDK 22+ by avoiding full trace-context swaps on every mount and unmount.
The trace scope stack is stored in a virtual-thread-aware
ThreadLocal, so it follows the virtual thread across parking and carrier migration. It is seeded once when the virtual thread starts.ddprof context remains carrier-thread-local. When carrier-bound profiling is active, it is rebound on mount and cleared on unmount.
JDK 21 retains the existing per-mount context-swap path because its virtual-thread lifecycle differs across update releases.
Motivation
The existing path restores the saved trace context on every mount and unmount. Although it reuses the saved scope stack, each cycle still performs two full context swaps and allocates context wrappers.
Benchmark
JMH on JDK 21. The current-path benchmark retains the context returned by each reverse swap, matching repeated production mount/unmount cycles.
Throughput uses 8 threads. Allocation uses a single-thread control run to avoid cross-thread effects.
In this Java-side microbenchmark, the proposed path provides approximately:
The profiling stub matches ddprof's shared integration lifetime, per-thread isolation, and set/clear pattern. It does not execute native ddprof calls, so profiling-on throughput is not an estimate of production ddprof performance.