Skip to content

fix(profiler): recover frozen virtual-thread callers - #879

Open
kaahos wants to merge 8 commits into
paul.fournillon/fix-frozen-virtual-thread-truncationfrom
paul.fournillon/recover-frozen-virtual-thread-callers
Open

kaahos wants to merge 8 commits into
paul.fournillon/fix-frozen-virtual-thread-truncationfrom
paul.fournillon/recover-frozen-virtual-thread-callers

Conversation

@kaahos

@kaahos kaahos commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?:

Recover older Java callers from HotSpot heap stack chunks when a mounted virtual thread reaches a continuation return barrier. Append interpreted and compiled frames, including inlined callers and parent chunks, to the existing trace buffer before stopping at the continuation root (cstack=vm) or walking the carrier (cstack=vmx). For deoptimized compiled frames, recover the saved original return PC before decoding inline scopes.

Extend regression coverage for both walking modes, CPU and wall sampling, parent chunks, mounted-frame deoptimization, depth limits, native calls, unsupported recovery, and platform-thread/fully thawed controls.

Motivation:

HotSpot can thaw only part of a virtual thread's stack after remounting it. The remaining Java callers stay in heap chunks and are absent from a walk of the carrier's native stack. #877 correctly marks that omission as truncated; this change restores those callers where recovery is supported.

Additional Notes:

  • Stacked on fix(profiler): mark skipped frozen virtual-thread callers as truncated #877 (paul.fournillon/fix-frozen-virtual-thread-truncation). Retarget to main after that dependency merges.
  • Recovery is gated to HotSpot JDK 21–27, x86_64/aarch64, G1/Serial/Parallel, available layout metadata, and samples interrupted in an interpreted or compiled Java body. Traversal and output depth are bounded.
  • Native/runtime/VM states, unsupported collectors or layouts, and incomplete recovery retain the truncation fallback. This draft does not restore every omitted caller in every sampling state.
  • Runtime and compilation validation covered arm64. x86_64, JDK 22–24/26, musl, and ASan/TSan were not tested. The complete Java integration suite was not run.
  • A separate order-sensitive carrier test failure remains: on Linux/JDK 27 with Fix continuation metadata lookup and frame alignment #870's metadata fixes applied, the existing carrier check passes alone but fails after the same workload prelude with both the old PR fix(profiler): mark skipped frozen virtual-thread callers as truncated #877 library and the recovery draft. Both failures report break_cont_carrier_sp without encountering a return barrier; this PR does not claim to fix it.

How to test the change?:

./.claude/commands/build-and-summarize spotlessApply :ddprof-test:testDebug \
  '-Ptests=*Frozen*' -Pskip-gtest -PkeepJFRs --max-workers=2
  • Eight restoration cases fail against the PR fix(profiler): mark skipped frozen virtual-thread callers as truncated #877 native library because outer callers are missing. The final clean macOS/JDK 25 run passes 15 cases with two conditional GC skips, without retries.
  • The selected class also passes on Linux/glibc arm64 JDK 21/27 and macOS arm64 JDK 27, and under interpreter-only, C1-only, uncompressed-oop, Serial, and Parallel configurations. Selected ZGC cases verify the truncated fallback.
  • Native debug suite: 884 passed, six skipped. Independent JDK JFR-consumer checks verify caller count/order, absence of duplicate callers, and completeness metadata.
  • The deoptimization regression first fails against the original recovery implementation in both vm and vmx: the physical caller is retained but its inline callers are omitted. It passes with original-PC recovery on Linux arm64 JDK 21/27 and macOS arm64 JDK 25/27.
  • Targeted native PC-recovery suite: 24 cases pass across the JDK 21–22, 23–25 and 26+ handler layouts, including method-handle handlers, offset zero and invalid saved-PC slots. Run ./.claude/commands/build-and-summarize :ddprof-lib:gtestDebug_frozenDeoptimization_ut.
  • spotlessApply and git diff --check pass. The combined frozen-stack Java regression run passes 18 tests with two conditional GC skips.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a security review.
  • This PR doesn't touch any of that.
  • Related JIRA: PROF-16269

Comment on lines +88 to +90
// G1/Serial/Parallel relocate objects at safepoints. A sampled Java thread
// cannot reach a safepoint while its signal handler is reading these oops.
// Concurrently relocating collectors require load barriers not provided here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the profiling signal land on a thread with frozen stack and already in safepoint?
Or is this structurally impossible?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it can, it is not structurally excluded. There is a protection in FrozenStackWalker::append: it rejects any thread state other than _thread_in_Java before reading heap objects. HotSpot switches to _thread_blocked before waiting at the safepoint, so those samples skip frozen-frame recovery and retain truncated=true. For accepted samples, the PC must also belong to interpreted or compiled Java code, excluding freeze/thaw runtime execution.

@dd-octo-sts

dd-octo-sts Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #39815407680 | Commit: 4e36fbe | Duration: 25m 51s (longest job)

❌ 4 of 32 test jobs failed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - ✅ - -
8-ibm - ✅ - -
8-j9 ✅ ✅ - -
8-librca - - ✅ ✅
8-orcl - ✅ - -
11 - ✅ - -
11-j9 ✅ ✅ - -
11-librca - - ✅ ✅
17 ✅ ✅ - -
17-graal ✅ ✅ - -
17-j9 ✅ ✅ - -
17-librca - - ✅ ✅
21 ❌ ✅ - -
21-graal ❌ ❌ - -
21-librca - - ✅ ✅
25 ❌ ✅ - -
25-graal ✅ ✅ - -
25-librca - - ✅ ✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

⚠️ Flaky tests — failed, then passed on retry

Test Cells Message
FrozenVirtualThreadStackTest.cstack_vm glibc-21-graal-debug-aarch64 org.opentest4j.AssertionFailedError: Samples omitted the JVM-confirmed outer caller; recovered=1318, failed=9 ==> expect

These fail the build. Passing on a second run makes a test flaky, not passing. Fix it, or quarantine it against a ticket so the debt is tracked rather than forgotten.

Consider quarantining these — click for ready-made entries

A quarantined test still runs and still reports; its failures just stop
turning CI red. To quarantine one:

  1. Open a PROF ticket for the test, linking the failing job.
  2. Append the line below to ddprof-test/quarantine.txt, replacing
    PROF-XXXXX with the ticket number.
  3. Check the cells and reason columns — the proposal only knows what
    failed in this run, and a narrower cells glob keeps the same test
    gating everywhere it has not misbehaved.

CI fails once review_by passes, so an entry expires instead of piling up.

# test | ticket | added | review_by | cells | reason
# WIDENED: observed `com.datadoghq.profiler.wallclock.FrozenVirtualThreadStackTest.cstack=vm`, which covers() cannot match exactly; this entry mutes the whole class
com.datadoghq.profiler.wallclock.FrozenVirtualThreadStackTest.* | PROF-XXXXX | 2026-10-22 | 2027-01-20 | glibc-21-graal-debug-aarch64 | org.opentest4j.AssertionFailedError: Samples omitted the JVM-confirmed outer caller; recovered=1318, failed=9 ==> expected: <0> but was: <1> (seen in: glibc-21-graal-debug-aarch64)

❌ Failing tests

Test Cells Message
VirtualThreadWallClockTest.cstack_vmx glibc-21-debug-aarch64, glibc-25-debug-aarch64, glibc-21-graal-debug-amd64 org.opentest4j.AssertionFailedError: No sample showed carrier-thread frames (ForkJoinWorkerThread) — continuation unwind

Retried 4 of 32 cells.

Failed Jobs

Summary: Total: 32 | Passed: 28 | Failed: 4


Updated: 2026-10-22 17:05:57 UTC

…nto paul.fournillon/recover-frozen-virtual-thread-callers
@dd-octo-sts

dd-octo-sts Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 746acbe1

@dd-octo-sts

dd-octo-sts Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/146813861

@kaahos
kaahos marked this pull request as ready for review October 22, 2026 16:51
@kaahos
kaahos requested a review from a team as a code owner October 22, 2026 16:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-22T16:58:52.586003Z 746acbe Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: FAIL

Nested continuation transitions can make frozen-stack recovery append suspended child callers instead of parent callers and incorrectly report the trace as complete.

Open Bits AI session

🤖 Bits Code Review · Commit 746acbe

// carrier Thread.cont, including when the current Java thread is virtual.
uintptr_t continuation = readOop(carrier, layout.carrier_cont);
for (unsigned int i = 0; i < continuation_depth && continuation != 0; i++) {
continuation = readOop(continuation, layout.cont_parent);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Match recovered chunks to the crossed continuation boundary

When a partially thawed parent resumes a nested continuation, Continuation.run publishes the child in Thread.cont before enterSpecial and restores the parent only after it returns. Samples in those Java intervals pass the recovery guards, but the first native return barrier belongs to the parent. Reading Thread.cont therefore selects the child's chunks, potentially appending suspended child frames, omitting parent callers, and clearing truncation. Match recovery to the crossed native continuation boundary, or reject recovery during these transitions. Correctly identifying that boundary requires broader changes, so no localized suggestion is provided.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants