Add sanity checks for profiler - #483
Draft
jbachorik wants to merge 8 commits into
Draft
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jbachorik
force-pushed
the
muse/impl-20260415-211352
branch
from
April 16, 2026 09:58
e7113c2 to
d981e16
Compare
Contributor
CI Test ResultsRun: #30927324461 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-08-04 16:30:16 UTC |
Resolve conflicts in arguments.{h,cpp} and profiler.cpp (both branches
added new Arguments fields/CLI cases independently). Fix sanity_checks_run_once
to actually exercise the cached-check path, and add the missing
vmStructs.inline.h include that sanityCheck.cpp needed for VMFlag::addr()
to link in release builds.
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a native “sanity check” gate to profiler startup to prevent enabling profiling under clearly under-provisioned CPU/memory configurations (especially in containers), with an opt-out nosanity argument and basic Java integration tests.
Changes:
- Introduces
SanityChecker(new C++ module) to estimate effective CPU capacity and JVM+profiler memory footprint and return a structured[sanity] ...error on failure. - Adds Linux cgroup v1/v2 CPU quota and memory limit detection APIs to
OS, with macOS returning “unconstrained” stubs. - Wires the sanity gate into
Profiler::start()and addsnosanityparsing plus a new JUnit test class covering the override and start/stop behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java | Adds JUnit tests validating nosanity bypass and intended “run once” behavior across stop/start. |
| ddprof-lib/src/main/cpp/sanityCheck.h | Declares the SanityChecker entrypoint. |
| ddprof-lib/src/main/cpp/sanityCheck.cpp | Implements CPU/memory sanity checks and structured telemetry error formatting. |
| ddprof-lib/src/main/cpp/profiler.cpp | Gates Profiler::start() on the sanity-check result with caching. |
| ddprof-lib/src/main/cpp/os.h | Adds OS APIs for cgroup CPU and container memory limits. |
| ddprof-lib/src/main/cpp/os_macos.cpp | Implements macOS stubs for the new OS APIs (returns unconstrained). |
| ddprof-lib/src/main/cpp/os_linux.cpp | Implements Linux cgroup v1/v2 parsing for CPU quota and memory limits. |
| ddprof-lib/src/main/cpp/arguments.h | Adds _skip_sanity_checks argument state. |
| ddprof-lib/src/main/cpp/arguments.cpp | Parses the new nosanity argument (bare and boolean-like forms). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
|
@codex review |
![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: 8bc3963756
ℹ️ 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".
Contributor
Benchmark Results (commit 8bc3963)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/128810310 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10175 ms (21 iters) | ✅ 10306 ms (21 iters) | ≈ +1.3% (±11.3%) | — / — |
| akka-uct | 25 | ✅ 8989 ms (24 iters) | ✅ 8894 ms (24 iters) | ≈ -1.1% (±10.5%) | — / — |
| finagle-chirper | 21 | ✅ 6003 ms (33 iters) | ✅ 5957 ms (33 iters) | ≈ -0.8% (±25.1%) | |
| finagle-chirper | 25 | ✅ 5525 ms (36 iters) | ✅ 5457 ms (36 iters) | ≈ -1.2% (±25%) | |
| fj-kmeans | 21 | ✅ 2691 ms (70 iters) | ✅ 2631 ms (72 iters) | ≈ -2.2% (±2.5%) | — / — |
| fj-kmeans | 25 | ✅ 2799 ms (66 iters) | ✅ 2806 ms (66 iters) | ≈ +0.3% (±2.7%) | — / — |
| future-genetic | 21 | ✅ 2116 ms (88 iters) | ✅ 2090 ms (89 iters) | ≈ -1.2% (±2.7%) | — / — |
| future-genetic | 25 | ✅ 2045 ms (91 iters) | ✅ 2114 ms (87 iters) | 🔴 +3.4% | — / — |
| naive-bayes | 21 | ✅ 1277 ms (134 iters) | ✅ 1230 ms (138 iters) | ≈ -3.7% (±32.1%) | — / — |
| naive-bayes | 25 | ✅ 1019 ms (168 iters) | ✅ 993 ms (172 iters) | ≈ -2.6% (±31%) | — / — |
| reactors | 21 | ✅ 15909 ms (15 iters) | ✅ 16112 ms (15 iters) | ≈ +1.3% (±8.2%) | — / — |
| reactors | 25 | ✅ 18714 ms (15 iters) | ✅ 18348 ms (15 iters) | ≈ -2% (±3.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 / 2 | 2061 / 2000 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 2340 / 2123 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 5 / 2 | 8464 / 8352 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 4 | 8655 / 8151 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 7 / ✅ | 1267 / 1291 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 2 | 1281 / 1251 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 4 | 2955 / 2995 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 2871 / 2908 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 3 | 3493 / 3518 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 3475 / 3461 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 1539 / 1572 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 2 | 1929 / 1756 | ✅ / ✅ | ✅ / ✅ |
Fix CPU-count and memory sanity-check edge cases: treat OS::getCpuCount() failure as unknown rather than a hard fail, align the core threshold with issue #480 (2 cores), skip HotSpot-only memory estimates on OpenJ9/Zing, normalize the unbounded MaxMetaspaceSize sentinel, accept intx flags for ThreadStackSize, clamp overflow in the memory total, resolve cgroup limits from the process's actual (possibly nested) cgroup, and mark the first profiler start as sanity-checked even when nosanity skips execution.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
ddprof-lib/src/main/cpp/sanityCheck.cpp:64
- The available-memory upper bound is derived from
OS::getRamSize() - OS_RESERVE. IfOS::getRamSize()returns 0 (e.g. macOS currently stubs it to 0, or/proc/meminfois unavailable),upperbecomes 0 and the memory sanity check can never fail—even ifgetContainerMemoryLimit()provides a strict container limit. This undermines the purpose of the check on such hosts.
u64 ram = OS::getRamSize();
u64 upper = (ram > OS_RESERVE) ? (ram - OS_RESERVE) : 0;
if (container_limit > 0 && (u64)container_limit < upper) {
upper = (u64)container_limit;
}
ddprof-lib/src/main/cpp/sanityCheck.cpp:110
- The PR description says the core check verifies
effective CPU count >= 1, but the implementation enforces>= 2cores (effective_cores < 2). Please reconcile the intended threshold (update the check or update the PR description/spec) so behavior matches the documented requirement.
// Per DataDog/java-profiler#480, the profiler refuses to run with fewer
// than 2 cores. An unknown core count (-1) never fails this check — see
// the effective_cores computation above.
bool cpu_fail = (effective_cores >= 0 && effective_cores < 2);
bool mem_fail = (hotspot && upper > 0 && lower > upper);
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
ddprof-lib/src/main/cpp/os_macos.cpp:377
OS::getCpuCount()returns1whensysctlbyname("hw.logicalcpu", ...)fails. With the new sanity check requiring >= 2 cores, this turns an OS query failure into a deterministic startup failure on macOS. Returning-1on failure aligns with the new “unknown core count must not fail” logic in sanityCheck.cpp.
int OS::getCpuCount() {
int cpu_count;
size_t size = sizeof(cpu_count);
return sysctlbyname("hw.logicalcpu", &cpu_count, &size, NULL, 0) == 0 ? cpu_count : 1;
}
ddprof-lib/src/main/cpp/sanityCheck.cpp:89
- HotSpot’s
ThreadStackSizeflag commonly has value0to mean “use platform default”. In that case, the current calculation makesstack_sizebecome 0 and underestimates the JVM footprint (threads * stack_size), potentially letting the profiler start when it should fail the memory sanity check.
size_t heap_max = hotspot ? getVMSizeFlag("MaxHeapSize", 0) : 0;
size_t metaspace_max = hotspot ? getVMSizeFlag("MaxMetaspaceSize", DEFAULT_METASPACE) : 0;
size_t codecache = hotspot ? getVMSizeFlag("ReservedCodeCacheSize", DEFAULT_CODECACHE) : 0;
size_t stack_size = hotspot ? getVMSizeFlag("ThreadStackSize", DEFAULT_STACK_SIZE / 1024) * 1024 : 0;
ddprof-lib/src/main/cpp/sanityCheck.cpp:110
- The PR description says the core check verifies effective CPU count >= 1, but the implementation fails when
effective_cores < 2. Please align the PR description (and any downstream expectations in dd-trace-java telemetry parsing) with the implemented 2-core threshold, or adjust the threshold if >=1 is actually intended.
// Per DataDog/java-profiler#480, the profiler refuses to run with fewer
// than 2 cores. An unknown core count (-1) never fails this check — see
// the effective_cores computation above.
bool cpu_fail = (effective_cores >= 0 && effective_cores < 2);
bool mem_fail = (hotspot && upper > 0 && lower > upper);
2 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
ddprof-lib/src/main/cpp/sanityCheck.cpp:103
- The memory estimate can overflow before it reaches addClamped():
(u64)thread_count * (u64)stack_sizeis evaluated with normal unsigned arithmetic and may wrap, causinglowerto be under-estimated and the sanity check to incorrectly pass on extremeThreadStackSize/ thread-count combinations. Compute the product with an explicit overflow check and clamp the product before adding.
lower = addClamped(lower, (u64)metaspace_max);
lower = addClamped(lower, (u64)codecache);
lower = addClamped(lower, gc_overhead);
lower = addClamped(lower, (u64)thread_count * (u64)stack_size);
lower = addClamped(lower, PROFILER_OVERHEAD);
ddprof-lib/src/main/cpp/os_linux.cpp:891
- The cgroup v1 fallback assumes fixed mount points (
/sys/fs/cgroup/cpuhere and in walkCgroupV1CpuMillicores). On many distros the CPU controller is mounted as/sys/fs/cgroup/cpu,cpuacct(or similar), in which case this code will fail to find quota files and incorrectly report "unconstrained" (-1), weakening the sanity gate in exactly the container environments it targets. Consider resolving the actual mount point from/proc/self/mountinfo(preferred), or at least trying common alternate mount paths and ensuring the ancestor-walk stop condition (base_len) matches the chosen mount prefix.
if (getOwnCgroupPath("cpu", subpath, sizeof(subpath))) {
const char* base = "/sys/fs/cgroup/cpu";
size_t base_len = strlen(base);
size_t sub_len = strlen(subpath);
if (base_len + sub_len < sizeof(path)) {
ddprof-lib/src/main/cpp/sanityCheck.cpp:110
- PR description says the core sanity check enforces effective CPU count >= 1, but the implementation fails when
effective_cores < 2(matching issue #480). Please align the PR description (and any external docs/telemetry expectations) with the implemented threshold, or adjust the threshold here if >=1 is actually intended.
// Per DataDog/java-profiler#480, the profiler refuses to run with fewer
// than 2 cores. An unknown core count (-1) never fails this check — see
// the effective_cores computation above.
bool cpu_fail = (effective_cores >= 0 && effective_cores < 2);
bool mem_fail = (hotspot && upper > 0 && lower > upper);
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
ddprof-lib/src/main/cpp/sanityCheck.cpp:55
OS::getCgroupCpuMillicores()can legitimately return0for extremely small CPU quotas (since the quota/period math truncates). The currentcgroup_mc > 0checks treat0as “unconstrained/unavailable”, which can incorrectly ignore a real (near-zero) container CPU limit and markcontainerized=falsein telemetry.
int logical_cpus = OS::getCpuCount();
int cgroup_mc = OS::getCgroupCpuMillicores();
long container_limit = OS::getContainerMemoryLimit();
bool containerized = (cgroup_mc > 0 || container_limit > 0);
ddprof-lib/src/main/cpp/sanityCheck.cpp:110
- The PR description says the core sanity check enforces
effective CPU count >= 1, but the implementation enforces the Issue #480 requirement of “fewer than 2 cores” viaeffective_cores < 2. Please align the PR description (or the implementation) so downstream consumers/telemetry expectations match the actual behavior.
// --- Run checks ---
// Per DataDog/java-profiler#480, the profiler refuses to run with fewer
// than 2 cores. An unknown core count (-1) never fails this check — see
// the effective_cores computation above.
bool cpu_fail = (effective_cores >= 0 && effective_cores < 2);
bool mem_fail = (hotspot && upper > 0 && lower > upper);
ddprof-lib/src/main/cpp/os_linux.cpp:904
- The cgroup v1 fallback assumes the CPU controller is mounted at
/sys/fs/cgroup/cpu. On many cgroup v1 systems it is mounted as/sys/fs/cgroup/cpu,cpuacct, so the leafopen()fails and CPU quotas are silently treated as “unavailable”, reducing the effectiveness of the sanity gate and producing misleadingcontainerized=falsetelemetry.
// Fall back to cgroup v1, likewise resolved from the process's own path.
if (getOwnCgroupPath("cpu", subpath, sizeof(subpath))) {
const char* base = "/sys/fs/cgroup/cpu";
size_t base_len = strlen(base);
size_t sub_len = strlen(subpath);
Contributor
Benchmark Results (commit 1853124)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/128867390 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):
|
…E_MAX match Debug HotSpot builds align the unbounded sentinel down during ergonomics, so it no longer bit-matches SIZE_MAX but still dwarfs available memory, causing the sanity check to fail on debug builds.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
ddprof-lib/src/main/cpp/os_linux.cpp:1004
- The cgroup v1 fallback assumes the memory controller is mounted at
/sys/fs/cgroup/memory. This is not guaranteed across distros/container runtimes, so the code can incorrectly treat a constrained container as unconstrained and skip the memory gating. Prefer discovering the correct mountpoint via/proc/self/mountinfo(or attempting common v1 mount directory variants) rather than hard-coding the path.
// Fall back to cgroup v1, likewise resolved from the process's own path.
if (getOwnCgroupPath("memory", subpath, sizeof(subpath))) {
const char* base = "/sys/fs/cgroup/memory";
size_t base_len = strlen(base);
size_t sub_len = strlen(subpath);
if (base_len + sub_len < sizeof(path)) {
memcpy(path, base, base_len);
memcpy(path + base_len, subpath, sub_len + 1);
char leaf[PATH_MAX];
if ((size_t)snprintf(leaf, sizeof(leaf), "%s/memory.limit_in_bytes", path) < sizeof(leaf)) {
int fd = open(leaf, O_RDONLY);
if (fd != -1) {
close(fd);
return walkCgroupV1MemoryLimit(path);
}
ddprof-lib/src/main/cpp/sanityCheck.cpp:112
- PR description says the core sanity check gates on effective CPU count >= 1, but the implementation (and Issue #480) gate on < 2 cores. Please align the documented behavior and the implemented threshold (including telemetry expectations) so users and downstream ingestion don’t see inconsistent semantics.
// --- Run checks ---
// Per DataDog/java-profiler#480, the profiler refuses to run with fewer
// than 2 cores. An unknown core count (-1) never fails this check — see
// the effective_cores computation above.
bool cpu_fail = (effective_cores >= 0 && effective_cores < 2);
bool mem_fail = (hotspot && upper > 0 && lower > upper);
ddprof-test/src/test/java/com/datadoghq/profiler/sanity/SanityCheckTest.java:77
sanity_checks_run_once()is not a deterministic regression test for the “checks run once” behavior: on sufficiently provisioned CI hosts, the second start would succeed even if the sanity checks were accidentally re-run. To make this test meaningful, it needs an observable signal that the native sanity checks executed (e.g., an explicit native counter exposed via JavaProfiler/JVMAccess, or a test-only argument that forces the sanity check path to record a marker).
/**
* Sanity checks run at most once across start/stop cycles.
* After a successful start with checks enabled, subsequent starts do not re-run checks.
*/
@Test
void sanity_checks_run_once() throws Exception {
profiler = JavaProfiler.getInstance();
// First start with nosanity to guarantee success regardless of host resources.
profiler.execute(startCommand("nosanity"));
profiler.stop();
// Second start (without nosanity) must not fail due to re-running checks — the
// static guard in the native layer ensures they only fire on the first invocation.
assertDoesNotThrow(() -> profiler.execute(startCommand(null)));
}
ddprof-lib/src/main/cpp/os_linux.cpp:900
- The cgroup v1 fallback assumes the CPU controller is mounted at
/sys/fs/cgroup/cpu. On many cgroup v1 setups the mount directory is different (e.g.cpu,cpuacct), which would make this return-1(unconstrained) even when a quota is set, weakening the intended gating behavior in containers. Consider resolving the actual controller mountpoint via/proc/self/mountinfo(or trying common alternatives) before constructingpath.
This issue also appears on line 989 of the same file.
// Fall back to cgroup v1, likewise resolved from the process's own path.
if (getOwnCgroupPath("cpu", subpath, sizeof(subpath))) {
const char* base = "/sys/fs/cgroup/cpu";
size_t base_len = strlen(base);
size_t sub_len = strlen(subpath);
if (base_len + sub_len < sizeof(path)) {
memcpy(path, base, base_len);
memcpy(path + base_len, subpath, sub_len + 1);
char leaf[PATH_MAX];
if ((size_t)snprintf(leaf, sizeof(leaf), "%s/cpu.cfs_quota_us", path) < sizeof(leaf)) {
int fd = open(leaf, O_RDONLY);
if (fd != -1) {
close(fd);
return walkCgroupV1CpuMillicores(path);
Contributor
Benchmark Results (commit d17bcd0)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/128894969 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):
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
ddprof-lib/src/main/cpp/sanityCheck.cpp:82
ThreadStackSizecan legitimately be 0 (which makesstack_sizebecome 0 here), causing the memory estimate to ignore per-thread stack memory and potentially under-estimate the required memory.
size_t heap_max = hotspot ? getVMSizeFlag("MaxHeapSize", 0) : 0;
size_t metaspace_max = hotspot ? getVMSizeFlag("MaxMetaspaceSize", DEFAULT_METASPACE) : 0;
size_t codecache = hotspot ? getVMSizeFlag("ReservedCodeCacheSize", DEFAULT_CODECACHE) : 0;
size_t stack_size = hotspot ? getVMSizeFlag("ThreadStackSize", DEFAULT_STACK_SIZE / 1024) * 1024 : 0;
ddprof-lib/src/main/cpp/sanityCheck.cpp:64
- If OS::getRamSize() returns 0 (e.g., /proc/meminfo unavailable),
upperbecomes 0 and the memory sanity check is effectively disabled even when a cgroup/container memory limit is available. This can let the profiler start in memory-constrained containers without running the intended check.
This issue also appears on line 78 of the same file.
u64 ram = OS::getRamSize();
u64 upper = (ram > OS_RESERVE) ? (ram - OS_RESERVE) : 0;
if (container_limit > 0 && (u64)container_limit < upper) {
upper = (u64)container_limit;
}
Contributor
Benchmark Results (commit f68a10d)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/128906026 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10255 ms (21 iters) | ✅ 10362 ms (21 iters) | ≈ +1% (±11.5%) | — / — |
| akka-uct | 25 | ✅ 8851 ms (24 iters) | ✅ 8813 ms (24 iters) | ≈ -0.4% (±10.4%) | — / — |
| finagle-chirper | 21 | ✅ 5941 ms (33 iters) | ✅ 5952 ms (33 iters) | ≈ +0.2% (±25%) | |
| finagle-chirper | 25 | ✅ 5453 ms (36 iters) | ✅ 5462 ms (36 iters) | ≈ +0.2% (±24.5%) | |
| fj-kmeans | 21 | ✅ 2717 ms (68 iters) | ✅ 2727 ms (68 iters) | ≈ +0.4% (±2.7%) | — / — |
| fj-kmeans | 25 | ✅ 2814 ms (66 iters) | ✅ 2826 ms (66 iters) | ≈ +0.4% (±2.6%) | — / — |
| future-genetic | 21 | ✅ 2043 ms (91 iters) | ✅ 2071 ms (90 iters) | ≈ +1.4% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 1973 ms (93 iters) | ✅ 2069 ms (90 iters) | 🔴 +4.9% | — / — |
| naive-bayes | 21 | ✅ 1241 ms (137 iters) | ✅ 1330 ms (129 iters) | ≈ +7.2% (±33.5%) | — / — |
| naive-bayes | 25 | ✅ 950 ms (179 iters) | ✅ 1019 ms (167 iters) | ≈ +7.3% (±33%) | — / — |
| reactors | 21 | ✅ 17088 ms (15 iters) | ✅ 15405 ms (15 iters) | 🟢 -9.8% | — / — |
| reactors | 25 | ✅ 18811 ms (15 iters) | ✅ 18608 ms (15 iters) | ≈ -1.1% (±3.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 | ✅ / ✅ | ✅ / ✅ | 5 / 5 | 1926 / 2060 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 2 | 2184 / 2214 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 8523 / 8353 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 4 | 8319 / 8163 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 1242 / 1285 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 2 / 5 | 1271 / 1287 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 21 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 2969 / 2943 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 2874 / 2887 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 8 / 9 | 3487 / 3529 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 4 | 3499 / 3490 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1711 / 1525 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | 1 / ✅ | 1939 / 1835 | ✅ / ✅ | ✅ / ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
Adds two pre-start sanity checks that gate profiler initialization (cpu, wallclock, allocation, live heap):
min(logical_cores, cgroup_cpu_quota_in_millicores / 1000). An unknown core count (OS query failure with no cgroup limit) never fails the check. Reads cgroup v2cpu.maxwith v1cpu.cfs_quota_us/periodfallback, resolved from the process's own cgroup path (via/proc/self/cgroup) and walked up through nested ancestor groups — not just the hierarchy mount root — so a limit set on a parent slice (e.g./user.slice/...) is still honored.min(physical_RAM - 128MB, container_memory_limit)), using the same per-process, ancestor-walked cgroup resolution as the core check for the memory limit. JVM flags (MaxHeapSize,MaxMetaspaceSize,ReservedCodeCacheSize,ThreadStackSize) are read viaVMFlag::find(), acceptingUintx/Size_t/Uint64_t/Intxflag types sinceThreadStackSizeis declaredintxon standard HotSpot builds. This lookup is skipped entirely on OpenJ9/Zing, where the HotSpot VMStructs flag table doesn't exist, rather than fail (or silently pass) the check on numbers that don't reflect the actual JVM. An unboundedMaxMetaspaceSize(HotSpot's default) is normalized to a 256MB estimate whenever it exceeds available memory, since debug builds don't reliably expose the raw sentinel as bit-identical toSIZE_MAX.When either check fails, the profiler logs a WARN to stderr and returns an Error (surfaces as
IllegalStateExceptionon the Java side) containing a structured[sanity]message with full system details (cpu=ok/fail, memory=ok/fail, all metrics) for telemetry ingestion by dd-trace-java.A single
nosanityargument disables both checks. Checks run on first initialization only; the result — and the fact that the first start was handled at all, even undernosanity— is cached across start/stop cycles, so a later start withoutnosanitydoesn't unexpectedly re-run and fail the checks.Resolves #480
Motivation:
JIRA: PROF-13027 — Prevent the profiler from running on systems with insufficient resources, which can cause OOM kills, excessive CPU contention, or degraded application performance.
Additional Notes:
sanityCheck.h,sanityCheck.cpp— encapsulates both checksos.h,os_linux.cpp,os_macos.cpp— cgroup CPU/memory limit APIs, including per-process cgroup path resolution and ancestor-walking for nested cgroupsarguments.h,arguments.cpp—nosanityflag parsingprofiler.cpp— gate inProfiler::start()with cached resultOS::getBasicProcessInfo()with fallback to 200[sanity] cpu=fail,memory=ok,logical_cores=4,cgroup_millicores=500,...,containerized=trueHow to test the change?:
SanityCheckTest.java: verifynosanitybypasses checks,nosanity=trueworks, and checks run only once across stop/start cycles--cpus=0.5or--memory=256mand observe WARN +IllegalStateExceptionwith structured telemetrynosanityto the profiler arguments to confirm override worksFor Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.🤖 Generated with Claude Code via muse implement