Repository navigation
Conversation
CI Test ResultsRun: #39636252372 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-10-21 16:58:14 UTC |
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/127205833 |
ad202a1 to
44a17e3
Compare
|
🔗 Commit SHA: 2a6d2d8 | Docs | View more details | Give us feedback! |
6d59cf5 to
73e3af6
Compare
73e3af6 to
ebab4c4
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class TaskBlock event recording to the Java profiler so known blocking intervals can be emitted directly (without relying on wall-clock sampling signals), including native/JFR plumbing and updates to wall-precheck behavior and tests.
Changes:
- Introduces public Java API
beginTaskBlock(int)/endTaskBlock(long, long, long)and native recording pipeline fordatadog.TaskBlock. - Reworks wall-precheck suppression around lifecycle-owned blocked runs, including unfiltered registry tracking and bounded “lazy backfill” candidate selection.
- Expands unit/integration/benchmark coverage for unfiltered precheck, restart safety, JVMTI delegation, and TaskBlock behavior.
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/WallclockMitigationsCombinedTest.java | Updates combined mitigation assertions to ensure context-scoped threads remain sampled and suppression counters don’t change. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnfilteredWallPrecheckTest.java | Adds integration coverage for owned-block prechecks when filter= samples every thread. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnfilteredWallPrecheckRestartTest.java | Adds restart/epoch safety coverage for unfiltered tracking across multiple recordings. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java | Adds JMC-based assertion helpers for datadog.TaskBlock contract validation. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java | Updates precheck tests to new suppression semantics/counters and explicit filter= behavior. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckEfficiencyTest.java | Adjusts workload tests to align with new lifecycle-owned suppression and explicit unfiltered sampling. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedUnfilteredWallPrecheckTest.java | Adds JVMTI-delegated stack path coverage for unfiltered owned-block precheck. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedPrecheckTest.java | Aligns JVMTI precheck tests with explicit filter= commands. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockPreExistingThreadTest.java | Verifies TaskBlock TLS init for threads created before profiler startup. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockDisabledTest.java | Verifies TaskBlock API stays inactive when not in all-thread scope. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockApiTest.java | End-to-end coverage for paired TaskBlock API, nesting/token rules, context admission, vthreads, and live dump behavior. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/J9WallClockPrecheckCapabilityTest.java | Ensures unsupported J9 wall engine doesn’t activate unfiltered precheck tracking. |
| ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java | Verifies TaskBlock API is public while internal hooks remain non-public. |
| ddprof-test/src/test/java/com/datadoghq/profiler/context/OtelContextStorageModeTest.java | Ensures trace context is cleared during cleanup (ordering with reset). |
| ddprof-test/src/test/java/com/datadoghq/profiler/context/AllNativeContextTest.java | Ensures trace context is cleared during cleanup (ordering with reset). |
| ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java | Adds beforeProfilerStart() hook and clears trace context on stopProfiler(). |
| ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java | Exposes package-scoped hooks for benchmarking owned-block precheck overhead. |
| ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/WallClockPrecheckOverheadBenchmark.java | Adds JMH benchmark for steady-state wall timer overhead vs. owned-block population size. |
| ddprof-lib/src/test/cpp/wallprecheck_args_ut.cpp | Adds engine capability and filter= parsing regression tests. |
| ddprof-lib/src/test/cpp/wallClockCounters_ut.cpp | Renames/updates suppression counter tests to owned-block suppression. |
| ddprof-lib/src/test/cpp/wallClockCandidateSelector_ut.cpp | Adds unit tests for bounded candidate selection/backfill logic. |
| ddprof-lib/src/test/cpp/threadFilter_ut.cpp | Expands ThreadFilter tests for registry, epoch, token encoding, and owned-block suppression eligibility. |
| ddprof-lib/src/test/cpp/taskBlockRecorder_ut.cpp | Adds unit tests for TaskBlock eligibility, rotation, and record/stack failure accounting. |
| ddprof-lib/src/test/cpp/park_state_ut.cpp | Updates tests to owned-block state semantics and slot-reset expectations. |
| ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | Adds unit tests for platform vs. virtual thread classification via JNI table probing. |
| ddprof-lib/src/test/cpp/frame_ut.cpp | Adds unit test for copyJvmtiFrames() union-overlap safety. |
| ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java | Adds public TaskBlock API and native bindings; clarifies internal hook comments. |
| ddprof-lib/src/main/cpp/wallClockCounters.h | Renames suppressed-sample counter to owned-block suppression counter. |
| ddprof-lib/src/main/cpp/wallClockCandidateSelector.h | Introduces bounded randomized candidate selection with precheck-rejection backfill. |
| ddprof-lib/src/main/cpp/wallClock.h | Integrates candidate selector + new suppression counters and visit budgeting. |
| ddprof-lib/src/main/cpp/wallClock.cpp | Reworks precheck suppression to use ThreadFilter-owned block eligibility and optional lazy backfill/lookup accounting. |
| ddprof-lib/src/main/cpp/threadLocalData.h | Adds per-thread TaskBlock lifecycle state in TLS. |
| ddprof-lib/src/main/cpp/threadFilter.h | Adds registry/unfiltered tracking, epoching, owned-block snapshots/tokens, and suppression eligibility checks. |
| ddprof-lib/src/main/cpp/threadFilter.cpp | Implements registry/TID index, recording epochs, retirement, and owned-block suppression candidate logic. |
| ddprof-lib/src/main/cpp/taskBlockRecorder.h | Adds TaskBlock eligibility checks and recording helper APIs. |
| ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | Implements duration thresholding and basic eligibility helpers. |
| ddprof-lib/src/main/cpp/profiler.h | Adds TaskBlock recording entry points and rotation/inflight coordination. |
| ddprof-lib/src/main/cpp/profiler.cpp | Adds TaskBlock recording implementation, rotation coordination, registry bootstrap for existing threads, and updated filter init semantics. |
| ddprof-lib/src/main/cpp/jvmThread.h | Adds capability query for native thread-id lookup support. |
| ddprof-lib/src/main/cpp/jvmThread.cpp | Implements native thread-id lookup capability helper. |
| ddprof-lib/src/main/cpp/jvmSupport.h | Declares isPlatformThread() helper for virtual thread rejection. |
| ddprof-lib/src/main/cpp/jvmSupport.cpp | Implements JNI table probing to detect virtual threads (JDK 19+). |
| ddprof-lib/src/main/cpp/jfrMetadata.h | Adds new JFR type id for TaskBlock. |
| ddprof-lib/src/main/cpp/jfrMetadata.cpp | Registers datadog.TaskBlock event metadata fields. |
| ddprof-lib/src/main/cpp/javaApi.cpp | Adds native implementations for TaskBlock API + registry-aware filter/add/remove and owned-block hooks. |
| ddprof-lib/src/main/cpp/frames.h | Adds copyJvmtiFrames() helper for overlapping JVMTI/ASGCT frame buffers. |
| ddprof-lib/src/main/cpp/flightRecorder.h | Adds TaskBlock recording entry points in recording/recorder. |
| ddprof-lib/src/main/cpp/flightRecorder.cpp | Implements TaskBlock event serialization and recorder entry. |
| ddprof-lib/src/main/cpp/event.h | Adds TaskBlock event struct; updates wall epoch suppressed counter field name/type usage. |
| ddprof-lib/src/main/cpp/engine.h | Adds wall-engine capability query for unfiltered precheck support. |
| ddprof-lib/src/main/cpp/counters.h | Adds counters for registry lookup budget/retirement and TaskBlock outcomes; renames suppression counter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Benchmark Results (commit ebab4c4)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125876908 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10309 ms (21 iters) | ✅ 10352 ms (21 iters) | ≈ +0.4% (±11.1%) | — / — |
| akka-uct | 25 | ✅ 8971 ms (24 iters) | ✅ 8823 ms (24 iters) | ≈ -1.6% (±10.5%) | — / — |
| finagle-chirper | 21 | ✅ 5997 ms (33 iters) | ✅ 5974 ms (33 iters) | ≈ -0.4% (±25.4%) | |
| finagle-chirper | 25 | ✅ 5455 ms (36 iters) | ✅ 5469 ms (36 iters) | ≈ +0.3% (±24.6%) | |
| fj-kmeans | 21 | ✅ 2671 ms (69 iters) | ✅ 2788 ms (67 iters) | 🔴 +4.4% | — / — |
| fj-kmeans | 25 | ✅ 2765 ms (68 iters) | ✅ 2778 ms (67 iters) | ≈ +0.5% (±2.8%) | — / — |
| future-genetic | 21 | ✅ 2054 ms (91 iters) | ✅ 2127 ms (88 iters) | 🔴 +3.6% | — / — |
| future-genetic | 25 | ✅ 2027 ms (91 iters) | ✅ 2069 ms (90 iters) | ≈ +2.1% (±2.8%) | — / — |
| naive-bayes | 21 | ✅ 1284 ms (133 iters) | ✅ 1244 ms (137 iters) | ≈ -3.1% (±32.2%) | — / — |
| naive-bayes | 25 | ✅ 1013 ms (169 iters) | ✅ 1017 ms (168 iters) | ≈ +0.4% (±31.6%) | — / — |
| reactors | 21 | ✅ 16291 ms (15 iters) | ✅ 15943 ms (15 iters) | ≈ -2.1% (±8.5%) | — / — |
| reactors | 25 | ✅ 18609 ms (15 iters) | ✅ 18693 ms (15 iters) | ≈ +0.5% (±4%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 1885 / 2016 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 6 / ✅ | 2056 / 2299 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 8787 / 8984 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 8287 / 8556 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1235 / 1277 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 1295 / 1292 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 3001 / 3006 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 2834 / 2887 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 5 / 6 | 3513 / 3477 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 3500 / 3460 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 1612 / 1583 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 2 | 1916 / 1954 | ✅ / ✅ | ✅ / ✅ |
ebab4c4 to
19cbe47
Compare
Benchmark Results (commit 19cbe47)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125884549 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10377 ms (21 iters) | ✅ 10184 ms (21 iters) | ≈ -1.9% (±11.2%) | — / — |
| akka-uct | 25 | ✅ 8790 ms (24 iters) | ✅ 8896 ms (24 iters) | ≈ +1.2% (±9.7%) | — / — |
| finagle-chirper | 21 | ✅ 5965 ms (33 iters) | ✅ 5956 ms (33 iters) | ≈ -0.2% (±25.2%) | |
| fj-kmeans | 21 | ✅ 2696 ms (70 iters) | ✅ 2843 ms (66 iters) | 🔴 +5.5% | — / — |
| fj-kmeans | 25 | ✅ 2796 ms (67 iters) | ✅ 2746 ms (68 iters) | ≈ -1.8% (±2.8%) | — / — |
| future-genetic | 21 | ✅ 2052 ms (90 iters) | ✅ 2099 ms (89 iters) | ≈ +2.3% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2035 ms (91 iters) | ✅ 2047 ms (91 iters) | ≈ +0.6% (±2.7%) | — / — |
| naive-bayes | 21 | ✅ 1224 ms (139 iters) | ✅ 1250 ms (136 iters) | ≈ +2.1% (±33.2%) | — / — |
| naive-bayes | 25 | ✅ 1016 ms (168 iters) | ✅ 1017 ms (168 iters) | ≈ +0.1% (±31.6%) | — / — |
| reactors | 21 | ✅ 16927 ms (15 iters) | ✅ 15621 ms (15 iters) | 🟢 -7.7% | — / — |
| reactors | 25 | ✅ 17980 ms (15 iters) | ✅ 17858 ms (15 iters) | ≈ -0.7% (±6.2%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1969 / 1961 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 2168 / 2325 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 1 | 8522 / 8623 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 1 | 8617 / 8387 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1273 / 1252 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1256 / 1264 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 1 | 2942 / 2959 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 2855 / 2852 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 3466 / 3494 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 5 / 6 | 3466 / 3473 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 2 | 1660 / 1623 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 1789 / 1796 | ✅ / ✅ | ✅ / ✅ |
19cbe47 to
25ba2a3
Compare
388e4bb to
eba56fc
Compare
eba56fc to
6a9b591
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a9b591d83
ℹ️ 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".
| if (!slot->unownedBlockedFallbackEnabled()) { | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Avoid dropping JVMTI unowned-block tail weight
When jvmtistacks=true, this guard now enables the weighted unowned-block fallback even though the delegated engine has no call_trace_id for replay. If an unowned blocked run ends after the first recorded signal but before the tenth, flushUnownedBlockedTail() consumes the suppressed weight on the runnable signal, while WallClockJvmti::signalHandler never calls emitUnownedBlockedTailForWallPrecheck; those suppressed intervals therefore disappear instead of contributing to the recorded weight. Keep ordinary per-signal sampling for this engine or provide a delegated tail-emission mechanism.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
true, but let's keep it that way for now.
jbachorik
left a comment
There was a problem hiding this comment.
🗿 🤖 🔴
Sphinx Review found 1 critical/high severity finding(s) that must be addressed.
jbachorik
left a comment
There was a problem hiding this comment.
🗿 🤖 🔴
Sphinx Review found 1 critical/high severity finding(s) that must be addressed.
c632b1f to
52e9497
Compare
c0732d4 to
712816d
Compare
669f541 to
37c62b8
Compare
6428b35 to
7e21c43
Compare
7e21c43 to
2a6d2d8
Compare
What does this PR do?:
Adds the core TaskBlock event emission machinery. Recognized blocked intervals are recorded synchronously at block exit, with native APIs, JFR metadata, counters, and wall precheck integration.
Motivation:
Blocked threads should not need to receive wall-clock signals just to represent blocking time. TaskBlock events let the profiler report known blocking intervals directly.
Additional Notes:
This PR adds the generic recording path only. Concrete JVM and native blocking producers are added in follow-up PRs.
How to test the change?:
Automated coverage includes TaskBlock recorder unit tests, Java API tests, wall precheck behavior, disabled-state behavior, and combined wall-clock mitigation tests.
./.claude/commands/build-and-summarize testDebugFor Datadog employees:
dd:platform-security-reviewskill, or file a request via the PSEC review form).bewairealso runs automatically on every PR.