feat: emit TaskBlock events for native socket I/O#666
Conversation
This comment has been minimized.
This comment has been minimized.
CI Test ResultsRun: #29960610324 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-07-22 22:07:55 UTC |
5de84e2 to
c4a2496
Compare
Benchmark Results (commit 4077c10)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125241502 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10318 ms (21 iters) | ✅ 10297 ms (21 iters) | ≈ -0.2% (±11.5%) | — / — |
| akka-uct | 25 | ✅ 8877 ms (24 iters) | ✅ 8872 ms (24 iters) | ≈ -0.1% (±10%) | — / — |
| finagle-chirper | 21 | ✅ 5924 ms (33 iters) | ✅ 6023 ms (33 iters) | ≈ +1.7% (±25.7%) | |
| finagle-chirper | 25 | ✅ 5440 ms (36 iters) | ✅ 5447 ms (36 iters) | ≈ +0.1% (±24%) | |
| fj-kmeans | 25 | ✅ 2819 ms (66 iters) | ✅ 2768 ms (68 iters) | ≈ -1.8% (±2.7%) | — / — |
| future-genetic | 21 | ✅ 2100 ms (89 iters) | ✅ 2118 ms (87 iters) | ≈ +0.9% (±2.8%) | — / — |
| future-genetic | 25 | ✅ 2072 ms (89 iters) | ✅ 2005 ms (93 iters) | 🟢 -3.2% | — / — |
| naive-bayes | 25 | ✅ 990 ms (172 iters) | ✅ 1017 ms (169 iters) | ≈ +2.7% (±31.9%) | — / — |
| reactors | 21 | ✅ 16610 ms (15 iters) | ✅ 16761 ms (15 iters) | ≈ +0.9% (±8%) | — / — |
| reactors | 25 | ✅ 18536 ms (15 iters) | ✅ 18473 ms (15 iters) | ≈ -0.3% (±2.8%) | — / — |
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 | 2000 / 2002 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 2240 / 2285 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 4 | 8783 / 8895 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 8381 / 7985 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 4 / ✅ | 1321 / ✅ | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 1268 / 1276 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 2 | 2969 / 2966 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 1 | 2828 / 2888 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 3471 / 3500 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1675 / 1607 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1935 / 1843 | ✅ / ✅ | ✅ / ✅ |
8ad829e to
7a6627e
Compare
7a6627e to
0aadd79
Compare
There was a problem hiding this comment.
Pull request overview
Adds TaskBlock event production for native socket I/O waits (Linux) by introducing native interposition + fd classification, expanding TaskBlock recording infrastructure (native + Java API), and updating wall-clock precheck behavior/tests to reflect lifecycle-owned suppression semantics.
Changes:
- Introduces
datadog.TaskBlockJFR event end-to-end: metadata, recording, counters, and Java public paired API (beginTaskBlock/endTaskBlock). - Implements Linux native socket I/O interposition + fd classification and emits TaskBlock for owned native I/O waits (plus lifecycle-safe patch/unpatch handling).
- Expands/updates Java + C++ test coverage and adds a wall-clock precheck overhead benchmark + native test fixture build support.
Reviewed changes
Copilot reviewed 91 out of 91 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/WallclockMitigationsCombinedTest.java | Updates assertions to reflect context-scoped threads not being suppression candidates. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/VirtualNativeIoTaskBlockTest.java | Adds (currently disabled) virtual-thread native I/O attribution coverage. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnfilteredWallPrecheckTest.java | Adds unfiltered (filter=) precheck behavior tests for owned-block suppression. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/UnfilteredWallPrecheckRestartTest.java | Verifies owned-block registry activation doesn’t leak across restart sequences. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/TaskBlockAssertions.java | Adds shared assertions for the datadog.TaskBlock event contract. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckTest.java | Updates precheck tests/counters for lifecycle-owned suppression and unfiltered sampling. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/PrecheckEfficiencyTest.java | Adjusts benchmark-like tests to run under explicit filter= semantics. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/ParkTaskBlockTest.java | Adds TaskBlock coverage for Java-owned park hooks + suppression behavior. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NativeSocketTaskBlockLifecycleTest.java | Adds (disabled) lifecycle tests for native socket hooks across stop/restart/uninstall. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/NativeIoBlockHelper.java | Adds JNI helper entrypoints for blocking Linux syscalls used in tests. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/MonitorTaskBlockTest.java | Adds TaskBlock coverage for monitor wait/contended callbacks and restart recovery. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedUnfilteredWallPrecheckTest.java | Runs unfiltered precheck coverage with delegated JVMTI stack collection. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedPrecheckTest.java | Ensures delegated stacks path uses explicit unfiltered recording flags. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedParkTaskBlockTest.java | Verifies TaskBlock production with delegated wall-clock stacks enabled. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedNativeSocketTaskBlockTest.java | Adds (disabled) native socket TaskBlock coverage with delegated stacks. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JvmtiBasedMonitorTaskBlockTest.java | Verifies monitor TaskBlock production with delegated stacks enabled. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockPreExistingThreadTest.java | Ensures pre-existing threads can initialize TaskBlock TLS and record events. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockDisabledTest.java | Confirms paired API remains inactive under filter=0 (no all-thread scope). |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/JavaProfilerTaskBlockApiTest.java | End-to-end tests for the paired public TaskBlock API and eligibility rules. |
| ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/J9WallClockPrecheckCapabilityTest.java | Ensures unsupported wall engine does not activate unfiltered precheck tracking on J9. |
| ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerTest.java | Adds agentpath-based startup tests and pre-existing monitor callback coverage. |
| ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java | Locks public API surface for TaskBlock and monitor ownership/delegation. |
| ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java | Adds child-process scenarios for agent initialization + pre-existing monitor callbacks. |
| ddprof-test/src/test/java/com/datadoghq/profiler/context/OtelContextStorageModeTest.java | Ensures trace context is cleared during cleanup. |
| ddprof-test/src/test/java/com/datadoghq/profiler/context/AllNativeContextTest.java | Ensures trace context is cleared during cleanup. |
| ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java | Adds beforeProfilerStart() hook and clears trace context on stop. |
| ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/WallClockPrecheckBenchmarkHooks.java | Exposes package-scoped owned-block hooks to JMH benchmarks. |
| ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/WallClockPrecheckOverheadBenchmark.java | Adds throughput benchmark for precheck overhead under increasing owned-block populations. |
| ddprof-lib/src/test/resources/native-libs/unloadable-io-lib/unloadable_io.c | Adds native fixture library used by unload/unpatch safety tests. |
| ddprof-lib/src/test/resources/native-libs/unloadable-io-lib/Makefile | Build recipe for unloadable native fixture library. |
| ddprof-lib/src/test/resources/native-libs/reladyn-lib/reladyn.c | Expands relocation fixture to exercise multiple import slots for same symbol. |
| ddprof-lib/src/test/cpp/wallprecheck_args_ut.cpp | Adds capability + filter parsing tests for unfiltered wall precheck. |
| ddprof-lib/src/test/cpp/wallClockCounters_ut.cpp | Updates counter tests to new “suppressed owned block” naming. |
| ddprof-lib/src/test/cpp/wallClockCandidateSelector_ut.cpp | Adds tests for candidate selection/backfill logic used in wall-clock sampling. |
| ddprof-lib/src/test/cpp/park_state_ut.cpp | Extends tests for park + monitor block state snapshotting and slot transitions. |
| ddprof-lib/src/test/cpp/jvmSupport_ut.cpp | Adds tests for JNI-based platform vs virtual thread classification. |
| ddprof-lib/src/test/cpp/frame_ut.cpp | Adds test ensuring JVMTI→ASGCT frame copy preserves overlapping fields. |
| ddprof-lib/src/test/cpp/elfparser_ut.cpp | Adds import-location coverage + skips a deliberate race test under TSan. |
| ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java | Adds TaskBlock public API, monitor delegation flag, and thread-aware native hook signatures. |
| ddprof-lib/src/main/cpp/wallClockCounters.h | Renames/repurposes suppression counter for lifecycle-owned blocked intervals. |
| ddprof-lib/src/main/cpp/wallClockCandidateSelector.h | Adds bounded backfill selector for wall-clock sampling candidates. |
| ddprof-lib/src/main/cpp/wallClock.h | Integrates bounded backfill selection + adds unfiltered-precheck capability on wall engines. |
| ddprof-lib/src/main/cpp/vmEntry.h | Adds monitor ownership/delegation init-result plumbing to bridge initialization. |
| ddprof-lib/src/main/cpp/threadState.h | Adds IO_WAIT OS thread state. |
| ddprof-lib/src/main/cpp/threadLocalData.h | Adds per-thread TaskBlock, park, and monitor state tracking for event emission. |
| ddprof-lib/src/main/cpp/taskBlockRecorder.h | Declares TaskBlock record/finish helpers and eligibility logic. |
| ddprof-lib/src/main/cpp/taskBlockRecorder.cpp | Implements min-duration thresholding and “finish-at-exit” recording path. |
| ddprof-lib/src/main/cpp/symbols.h | Makes UnloadProtection movable and adds release() for retained handles. |
| ddprof-lib/src/main/cpp/symbols_macos.cpp | Implements move operations + release for UnloadProtection. |
| ddprof-lib/src/main/cpp/symbols_linux.cpp | Hardens ELF address computations, adds relocation parsing fixes, and sanitizer-specific VDSO behavior. |
| ddprof-lib/src/main/cpp/profiler.h | Adds TaskBlock recording entrypoints/rotation state and test hooks. |
| ddprof-lib/src/main/cpp/nativeSocketSampler.h | Refactors sampling to use shared fd classifier and adds active/disable helpers. |
| ddprof-lib/src/main/cpp/nativeSocketSampler.cpp | Integrates shared fd classifier, improves lifecycle handling around patch failures, and adds test observers. |
| ddprof-lib/src/main/cpp/nativeSocketInterposer.h | Adds Linux native I/O interposer API and hook specs for blocking syscalls. |
| ddprof-lib/src/main/cpp/nativeFdClassifier.h | Adds shared fd type classifier with epoch+incarnation tagging for reuse safety. |
| ddprof-lib/src/main/cpp/nativeBlock.h | Adds native owned-block scope for TaskBlock emission around blocking syscalls. |
| ddprof-lib/src/main/cpp/nativeBlock.cpp | Implements native blocking scope lifecycle + TaskBlock recording integration. |
| ddprof-lib/src/main/cpp/libraryPatcher.h | Extends patch tracking for native I/O hook sets and safe restoration across unload. |
| ddprof-lib/src/main/cpp/jvmThread.h | Adds capability query for resolving native thread IDs beyond the caller. |
| ddprof-lib/src/main/cpp/jvmThread.cpp | Implements native thread ID lookup capability helper. |
| ddprof-lib/src/main/cpp/jvmSupport.h | Adds platform-thread classification helper. |
| ddprof-lib/src/main/cpp/jvmSupport.cpp | Implements JNI IsVirtualThread-based platform-thread classification. |
| ddprof-lib/src/main/cpp/jfrMetadata.h | Adds JFR type ID for TaskBlock. |
| ddprof-lib/src/main/cpp/jfrMetadata.cpp | Defines datadog.TaskBlock event schema and updates wall epoch suppression field name. |
| ddprof-lib/src/main/cpp/frames.h | Adds safe JVMTI→ASGCT frame copy helper. |
| ddprof-lib/src/main/cpp/flightRecorder.h | Adds TaskBlock recording entrypoints. |
| ddprof-lib/src/main/cpp/flightRecorder.cpp | Emits TaskBlock events and adds IO_WAIT thread state to constant pool. |
| ddprof-lib/src/main/cpp/event.h | Adds TaskBlock event struct and renames wall epoch suppression field. |
| ddprof-lib/src/main/cpp/engine.h | Adds wall-engine capability hook for unfiltered precheck consumption. |
| ddprof-lib/src/main/cpp/counters.h | Adds counters for TaskBlock and precheck backfill/lookup budgeting. |
| ddprof-lib/src/main/cpp/codeCache.h | Redesigns import tracking to support multiple slots per import + patch preparation. |
| ddprof-lib/src/main/cpp/codeCache.cpp | Implements multi-location import storage, patchability prep, and broader import discovery. |
| ddprof-lib/build.gradle.kts | Wires native fixture build directories into gtest extension configuration. |
| build-logic/conventions/src/main/kotlin/com/datadoghq/native/gtest/GtestPlugin.kt | Adds native fixture build task and CI-friendly build task dependency behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int taskBlockInflightForTest() const { | ||
| return _task_block_inflight.load(std::memory_order_acquire); | ||
| } |
| if (!taskBlockEvents.hasItems()) { | ||
| assertTrue(false, missingTaskBlockDiagnostic()); | ||
| } |
18130c4 to
4698b06
Compare
Benchmark Results (commit 4698b06)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126018479 Commit: ✅ Within expected boundariesNo significant runtime deltas (all within run-to-run noise) and no internal-counter outliers. Runtime details (per benchmark × JDK)
Internal counter details (ddprof)ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
|
4698b06 to
cb952e8
Compare
cb952e8 to
cd67725
Compare
Benchmark Results (commit cd67725)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126291644 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10352 ms (21 iters) | ✅ 10315 ms (21 iters) | ≈ -0.4% (±11.2%) | — / — |
| akka-uct | 25 | ✅ 8989 ms (24 iters) | ✅ 8928 ms (24 iters) | ≈ -0.7% (±10.6%) | — / — |
| finagle-chirper | 21 | ✅ 5977 ms (33 iters) | ✅ 5873 ms (33 iters) | ≈ -1.7% (±24.6%) | |
| finagle-chirper | 25 | ✅ 5487 ms (36 iters) | ✅ 5512 ms (36 iters) | ≈ +0.5% (±24.7%) | |
| fj-kmeans | 21 | ✅ 2824 ms (66 iters) | ✅ 2737 ms (69 iters) | 🟢 -3.1% | — / — |
| fj-kmeans | 25 | ✅ 2810 ms (66 iters) | ✅ 2810 ms (66 iters) | ≈ 0% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2074 ms (89 iters) | ✅ 2074 ms (89 iters) | ≈ 0% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2073 ms (89 iters) | ✅ 2093 ms (88 iters) | ≈ +1% (±2.6%) | — / — |
| naive-bayes | 21 | ✅ 1285 ms (134 iters) | ✅ 1234 ms (138 iters) | ≈ -4% (±32.2%) | — / — |
| naive-bayes | 25 | ✅ 994 ms (172 iters) | ✅ 1022 ms (168 iters) | ≈ +2.8% (±32.1%) | — / — |
| reactors | 21 | ✅ 16106 ms (15 iters) | ✅ 16148 ms (15 iters) | ≈ +0.3% (±6.8%) | — / — |
| reactors | 25 | ✅ 18475 ms (15 iters) | ✅ 18665 ms (15 iters) | ≈ +1% (±3.7%) | — / — |
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 / 1 | 2055 / 2030 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 2174 / 2475 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 4 | 8918 / 8594 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 7992 / 8655 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1289 / 1309 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 1291 / 1252 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 2945 / 2931 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 2867 / 2908 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 3527 / 3516 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 2 | 3486 / 3473 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1682 / 1564 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1819 / 1948 | ✅ / ✅ | ✅ / ✅ |
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/126818299 |
cd67725 to
eca3d88
Compare
Benchmark Results (commit eca3d88)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126390430 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10305 ms (21 iters) | ✅ 10365 ms (21 iters) | ≈ +0.6% (±11.5%) | — / — |
| akka-uct | 25 | ✅ 8825 ms (24 iters) | ✅ 8930 ms (24 iters) | ≈ +1.2% (±10%) | — / — |
| finagle-chirper | 21 | ✅ 5940 ms (33 iters) | ✅ 5959 ms (33 iters) | ≈ +0.3% (±25.2%) | |
| fj-kmeans | 21 | ✅ 2707 ms (69 iters) | ✅ 2698 ms (70 iters) | ≈ -0.3% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2735 ms (69 iters) | ✅ 2811 ms (67 iters) | 🔴 +2.8% | — / — |
| future-genetic | 21 | ✅ 2078 ms (89 iters) | ✅ 2090 ms (88 iters) | ≈ +0.6% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2056 ms (91 iters) | ✅ 2102 ms (88 iters) | ≈ +2.2% (±2.6%) | — / — |
| naive-bayes | 21 | ✅ 1251 ms (137 iters) | ✅ 1225 ms (139 iters) | ≈ -2.1% (±32.2%) | — / — |
| naive-bayes | 25 | ✅ 1026 ms (167 iters) | ✅ 1005 ms (170 iters) | ≈ -2% (±31.1%) | — / — |
| reactors | 21 | ✅ 16519 ms (15 iters) | ✅ 16203 ms (15 iters) | ≈ -1.9% (±7.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 / 1 | 1985 / 1922 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 2211 / 2276 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 6 / 5 | 8445 / 8610 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 3 | 1308 / 1293 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 1276 / 1318 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 2911 / 2965 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 1 | 2948 / 2836 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 3 / 5 | 3479 / 3486 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 3474 / 3438 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1748 / 1664 | ✅ / ✅ | ✅ / ✅ |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eca3d88f9e
ℹ️ 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".
| _task_block_enabled.store( | ||
| (activated & EM_WALL) && args._wall_precheck && track_unfiltered_wall, | ||
| std::memory_order_release); |
There was a problem hiding this comment.
Disable TaskBlock suppression for lightweight wall profiles
When a recording is started with wall=...,filter=,wallprecheck=true,lightweight=yes, this still enables TaskBlock ownership, so the wall timer suppresses owned blocked intervals; however Profiler::recordTaskBlock immediately rejects every TaskBlock while _omit_stacktraces is true. Those waits then produce neither normal wall samples nor TaskBlock events, causing blocking wall time to disappear from lightweight recordings. Please either keep TaskBlock suppression disabled for lightweight mode or allow stackless TaskBlock records.
Useful? React with 👍 / 👎.
| if (instance != null) { | ||
| return instance; |
There was a problem hiding this comment.
Validate delegation requests when reusing the singleton
If the profiler singleton was already initialized with the default delegateMonitorWaitEvents=false, a later caller using this new overload with true silently gets the existing instance without invoking the native conflict check. In that scenario Java wait instrumentation can assume it owns Object.wait while the native bridge remains non-delegated and will enable JVMTI wait callbacks, leading to duplicate/conflicting TaskBlock ownership instead of the documented IllegalStateException. Please check delegateMonitorWaitEvents against monitorEventsDelegated0() before returning an existing instance.
Useful? React with 👍 / 👎.
* fix(codecache): make memoryUsage() accurate and live CodeCache::memoryUsage() drove the CODECACHE_NATIVE_SIZE_BYTES counter but both under-counted and went stale: - The blob array was counted as _capacity * sizeof(CodeBlob*) — a pointer's worth — while the array holds CodeBlob (three pointers), a ~3x undercount. - Symbol name strings were approximated as _count * sizeof(NativeFunc), a fixed 4 bytes each, ignoring the name length that dominates the actual allocation. - The DWARF unwind table and build-id string were not counted at all. - CodeCacheArray cached the sum at add() time, so it never reflected a library's later symbol growth. Recompute both accurately on demand instead. CodeCache::memoryUsage() now sums the full blob array, each symbol's real name-string allocation (via new NativeFunc::allocSize()), its own name, the DWARF table, and the build-id. CodeCacheArray::memoryUsage() sums the live per-library values (the array is append-only, so iterating the published prefix is safe alongside concurrent adds); the stale _used_memory cache is removed. memoryUsage() is only called at dump time, so the O(symbols) recompute is not on any hot path. Tests: new codeCache_ut asserts usage tracks per-symbol name length, that a longer name costs more than a shorter one, and that the blob array is counted at full CodeBlob size rather than a pointer's. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(codecache): describe current memoryUsage(), drop old-formula references Comments explained what the previous formula did wrong rather than what the code now counts. Reword to describe the current behavior only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(codecache): don't read build-id in memoryUsage() (data race); review nits Address PR review feedback: - Drop the build-id from memoryUsage(). The background library refresher (Libraries::updateBuildIds) calls setBuildId() — which frees and replaces _build_id — on already-published caches under _build_id_lock, which dump does not hold. The strlen(_build_id) added here could race that free/replace (use-after-free / torn read). Build-id is negligible (~tens of bytes per library) next to the symbol tables, so excluding it keeps the read lock-free at no meaningful accuracy cost. The blob-name loop and dwarf-length read are safe: blobs are fixed once a library is published (same read the symbolication fast path does) and the dwarf read touches only the length scalar. - Mark CodeCache::memoryUsage() const (it does not mutate state). - Test: use reinterpret_cast for the integer-to-pointer args and drop the unused <cstring> include. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * harden(codecache): enforce/scope the published-immutability invariant Address the Sphinx review on memoryUsage()'s lock-free read: - Make memoryUsage()'s lock-free safety an enforced precondition. Add a _published flag, set by CodeCacheArray::add() before the pointer is published, and assert !_published in add()/expand()/setDwarfTable() — the mutators that touch the _blobs array and _dwarf_table_length that memoryUsage() reads from the dump thread. Standalone caches that are never registered (e.g. Libraries::_runtime_stubs, which is continuously mutated under its own lock) stay unpublished and may be mutated freely. - Scope the safety comment accordingly: the lock-free read is valid only for array-registered caches, and must not be used on _runtime_stubs without that cache's lock. - NativeFunc::create() now calls allocSize() instead of recomputing the size formula, so memoryUsage() and the allocation can't drift. - Reword CodeCacheArray::memoryUsage() to drop the incorrect "reflects symbols added after registration" claim (adds happen pre-publish); it reflects each library's fully-populated state at dump. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(codecache): cover array aggregation, DWARF term, exact blob-array size Address the Sphinx review's test-coverage gaps: - ArrayMemoryUsageSumsLibraries: exercises CodeCacheArray::memoryUsage() (the aggregation production consumes via Profiler::dump()), which no test covered — guards the loop and accumulation. - MemoryUsageCountsDwarfTable: populates a DWARF table via setDwarfTable() so the length * sizeof(FrameDesc) term is non-zero and a regression in it is detectable. - MemoryUsageCountsFullBlobArray: tightened from EXPECT_GT to an exact EXPECT_EQ (blob array + own name) so over-counting would fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(codecache): reference JitCodeCache::_runtime_stubs, not Libraries:: The comments on the lock-free memoryUsage() read named Libraries::_runtime_stubs (a vestigial, unused member) as the example of a continuously-mutated unpublished cache. The actual one is JitCodeCache::_runtime_stubs, mutated under JitCodeCache::_stubs_lock (add() under lock(), findRuntimeStub() under lockShared()). Fix the references. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * harden(codecache): make _published atomic (release/acquire) Addressing review feedback on visibility: _published was a plain bool read/written across threads (set by CodeCacheArray::add(), read by the add()/expand()/setDwarfTable() asserts). Make it std::atomic<bool> with a release store in markPublished() and acquire loads in the asserts. The store is sequenced before add()'s RELEASE publish of the pointer, so any thread that reaches a published cache observes _published == true — now without a formal data race and TSan-clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(codecache): deep-copy blob name strings in copyFrom() copyFrom() memcpy'd the CodeBlob array, which copied each blob's _name *pointer* — so a copied CodeCache shared name allocations with the original and both destructors freed them (double-free / use-after-free). Give the copy its own name strings via NativeFunc::create(), matching how build-id and the DWARF table are already deep-copied. (CodeCache is only ever used by pointer today, so the copy path is currently unexercised — this hardens it rather than fixing an observed crash.) Adds CopyIsDeepAndDoesNotDoubleFree, which copies a populated cache and destroys both, aborting on the old shallow-copy behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… frame skipping (#648)
Benchmark Results (commit 3919bfb)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126818333 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10418 ms (21 iters) | ✅ 10214 ms (21 iters) | ≈ -2% (±10.7%) | — / — |
| akka-uct | 25 | ✅ 8888 ms (24 iters) | ✅ 8832 ms (24 iters) | ≈ -0.6% (±10.4%) | — / — |
| finagle-chirper | 21 | ✅ 6019 ms (33 iters) | ✅ 5980 ms (33 iters) | ≈ -0.6% (±25.1%) | |
| finagle-chirper | 25 | ✅ 5432 ms (36 iters) | ✅ 5446 ms (36 iters) | ≈ +0.3% (±24.4%) | |
| fj-kmeans | 21 | ✅ 2789 ms (67 iters) | ✅ 2662 ms (69 iters) | 🟢 -4.6% | — / — |
| fj-kmeans | 25 | ✅ 2809 ms (67 iters) | ✅ 2750 ms (68 iters) | ≈ -2.1% (±2.7%) | — / — |
| future-genetic | 21 | ✅ 2036 ms (91 iters) | ✅ 2039 ms (90 iters) | ≈ +0.1% (±2.5%) | — / — |
| future-genetic | 25 | ✅ 2062 ms (90 iters) | ✅ 2126 ms (87 iters) | 🔴 +3.1% | — / — |
| naive-bayes | 21 | ✅ 1297 ms (132 iters) | ✅ 1288 ms (133 iters) | ≈ -0.7% (±32.7%) | — / — |
| naive-bayes | 25 | ✅ 1021 ms (168 iters) | ✅ 1018 ms (168 iters) | ≈ -0.3% (±31.8%) | — / — |
| reactors | 21 | ✅ 16109 ms (15 iters) | ✅ 17100 ms (15 iters) | ≈ +6.2% (±8.5%) | — / — |
| reactors | 25 | ✅ 18270 ms (15 iters) | ✅ 18619 ms (15 iters) | ≈ +1.9% (±5.8%) | — / — |
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 | 2080 / 1895 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 5 / 1 | 2429 / 2266 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 8524 / 8648 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 8076 / 8724 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 1281 / 1249 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 1299 / 1279 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 3000 / 2916 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 3 | 2872 / 2874 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 7 / 8 | 3542 / 3554 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 3505 / 3516 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1881 / 1883 | ✅ / ✅ | ✅ / ✅ |
What does this PR do?:
Adds native socket I/O TaskBlock instrumentation on Linux by interposing blocking socket APIs, classifying socket file descriptors, and recording synchronous TaskBlock events for owned native I/O waits.
Motivation:
Blocking native I/O is a major source of wall-clock time. Recording these intervals directly avoids intentionally signaling threads while they are blocked in recognized socket operations.
Additional Notes:
This PR includes Linux native interposition, socket/Fd classification, lifecycle handling for unloadable libraries, integration tests, and a native socket I/O benchmark.
How to test the change?:
Automated coverage includes native unit tests for interposition, socket sampling, native block handling, thread-filter suppression, and Java integration tests for native socket TaskBlock behavior.
./.claude/commands/build-and-summarize testDebugNative socket integration tests are Linux-specific and skip on macOS.
For Datadog employees:
dd:platform-security-reviewskill, or file a request via the PSEC review form).bewairealso runs automatically on every PR.