Skip to content

Add testProcess{Config} task to accept user options#581

Draft
zhengyu123 wants to merge 10 commits into
mainfrom
zgu/test_cmd_opts
Draft

Add testProcess{Config} task to accept user options#581
zhengyu123 wants to merge 10 commits into
mainfrom
zgu/test_cmd_opts

Conversation

@zhengyu123

@zhengyu123 zhengyu123 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:
This PR adds a new task for testing, that accepts user options.

Motivation:
The task is created for testing new feature that avoids preloading all jmethod ids. By default, the feature is disabled and has to be enabled via fjmethodid=false argument, which is not possible with current testdebug/release tasks.

Additional Notes:

How to test the change?:
Run ./gradlew Pprofiler.options=fjmethodid=false testProcessDebug, fjmethodid=false is merged into command line that print out by tests.

For example:

[TEST] Applied profiler.options: fjmethodid=false
===> keep_jfrs: false
com.datadoghq.profiler.wallclock.SmokeWallTest > test PASSED (789ms)
com.datadoghq.profiler.wallclock.SmokeWallTest > test STARTED
[TEST] Applied profiler.options: fjmethodid=false
===> command: start,wall=1ms,fjmethodid=false,cstack=vm,attributes=tag1;tag2;tag3,jfr,file=/tmp/recordings/fp/SmokeWallTest_test14542021396455448632.jfr

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 review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-14940

Unsure? Have a question? Request a review!

@dd-octo-sts

dd-octo-sts Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #29101239424 | Commit: 2427a27 | Duration: 14m 35s (longest job)

All 32 test jobs passed

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

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-07-10 15:06:43 UTC

@zhengyu123 zhengyu123 changed the title Zgu/test cmd opts Add testProcess{Config} task to accept user options Jun 5, 2026
@zhengyu123 zhengyu123 marked this pull request as ready for review June 5, 2026 20:24
@zhengyu123 zhengyu123 requested a review from a team as a code owner June 5, 2026 20:24

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f5c7bae36

ℹ️ 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".

@jbachorik jbachorik left a comment

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.

Sphinx automated review — PR #581 · zgu/test_cmd_opts

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a13c32a888

ℹ️ 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".

Comment thread ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java Outdated

@rkennke rkennke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good change. I found one small issue, and a bug, which I then found out that Codex already flagged it:
https://github.com/DataDog/java-profiler/pull/581/changes#r3375494440

}

// Remove LD_LIBRARY_PATH to let RPATH work correctly
execTask.doFirst {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it's cleaner do not add LD_LIBRARY_PATH in the loop just above?

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.

Could you check if it is still the case? I cannot find any loops.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a80e41432

ℹ️ 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".

@dd-octo-sts

dd-octo-sts Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 0a80e41)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/122585095 Commit: 0a80e4143289bc79d408bfa5ea2776baf377ce4b

⚠️ Significant outliers

  • 🔴 fj-kmeans (JDK 25): runtime +7.7% (2637→2841 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10238 ms (7 iters) ✅ 10306 ms (7 iters) ≈ +0.7% (±20.1%) — / —
akka-uct 25 ✅ 8873 ms (8 iters) ✅ 8959 ms (8 iters) ≈ +1% (±18.2%) — / —
finagle-chirper 21 ✅ 5991 ms (11 iters) ✅ 6043 ms (11 iters) ≈ +0.9% (±47.8%) ⚠️ W:1 / ⚠️ W:1
finagle-chirper 25 ✅ 5449 ms (12 iters) ✅ 5394 ms (12 iters) ≈ -1% (±43.6%) ⚠️ W:1 / ⚠️ W:1
fj-kmeans 21 ✅ 2876 ms (22 iters) ✅ 2821 ms (22 iters) ≈ -1.9% (±4.3%) — / —
fj-kmeans 25 ✅ 2637 ms (24 iters) ✅ 2841 ms (22 iters) 🔴 +7.7% — / —
future-genetic 21 ✅ 2044 ms (30 iters) ✅ 2070 ms (30 iters) ≈ +1.3% (±4.4%) — / —
future-genetic 25 ✅ 2057 ms (30 iters) ✅ 1967 ms (31 iters) ≈ -4.4% (±4.7%) — / —
naive-bayes 21 ✅ 1232 ms (46 iters) ✅ 1270 ms (45 iters) ≈ +3.1% (±58.3%) — / —
naive-bayes 25 ✅ 1022 ms (56 iters) ✅ 1024 ms (56 iters) ≈ +0.2% (±54.4%) — / —
reactors 21 ✅ 15424 ms (5 iters) ✅ 16820 ms (5 iters) ≈ +9.1% (±14.4%) — / —
reactors 25 ✅ 17672 ms (5 iters) ✅ 18532 ms (5 iters) ≈ +4.9% (±5.1%) — / —
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 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 1 2246 / 2167 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ ✅ / 2 8269 / 8286 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 1 / 1 1308 / 1280 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 3 / 1 2907 / 2790 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 4 / ✅ 3486 / ✅ ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 6 / 6 3503 / 3518 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / ✅ 1550 / 1704 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / 1 1815 / 1878 ✅ / ✅ ✅ / ✅

@datadog-datadog-prod-us1

This comment has been minimized.

@zhengyu123

zhengyu123 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Summary of what changed in ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java: the -Pprofiler.options override logic is now exposed as public static String applyProfilerOptionOverrides(String), reused by getAmendedProfilerCommand(). Then I wired it into every test that calls JavaProfiler.execute() with a hard-coded command instead of going through AbstractProfilerTest:

ShutdownTest — runTest() now builds "start," + applyProfilerOptionOverrides(options) + ",jfr,file=..."; call sites pass options only (no "start," prefix baked in).
OtelContextStorageModeTest — all 5 execute() calls now wrap their options portion.
NativeSocketRestartTest — the manual restart-session execute() call now wraps "natsock=100us".
StopWithChurningThreadsTest / StartWithChurningThreadsTest — PROFILER_CMD split into a PROFILER_OPTIONS constant plus an override-applied command built once before the loop.
GetLineNumberTableLeakTest, WriteStackTracesAfterClassUnloadTest, CleanupAfterClassUnloadTest (all extend AbstractProfilerTest via AbstractDynamicClassTest) — now call applyProfilerOptionOverrides(getProfilerCommand()) instead of using getProfilerCommand()/a hard-coded literal directly.
One correctness detail I had to work around: the merge logic parses comma-separated key=value pairs, so wrapping the whole command (including the literal "start" action keyword or the trailing "jfr,file=..." segment) would corrupt them into "start=" / spurious keys. Every fix above applies the override only to the pure-options substring, matching the pattern AbstractProfilerTest itself already uses.

CarrierContextStorageTest and IdleThreadLifecycleTest needed no change — neither calls execute() with a profiler command. :ddprof-test:compileTestJava compiles cleanly with these changes (I also removed a handful of stale .o build artifacts left over from an upstream file rename that were unrelated to this fix but were blocking the native link step).

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 710ac934b8

ℹ️ 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".

@zhengyu123 zhengyu123 marked this pull request as draft July 3, 2026 14:52
@dd-octo-sts

dd-octo-sts Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 710ac93)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/122608527 Commit: 710ac934b8df5ebdd662a95ddcf996e3b21875bc

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 25): runtime -9.7% (2074→1873 ms)
  • 🔴 reactors (JDK 25): runtime +7.8% (17345→18700 ms)
  • ⚠️ akka-uct (JDK 21): wallclock unwinds skipped — latest 0 → dev 1828 (new)
  • ⚠️ akka-uct (JDK 25): wallclock unwinds skipped — latest 0 → dev 2210 (new)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10387 ms (7 iters) ✅ 10226 ms (7 iters) ≈ -1.6% (±21.7%) — / —
akka-uct 25 ✅ 8956 ms (8 iters) ✅ 8826 ms (8 iters) ≈ -1.5% (±21%) — / —
finagle-chirper 21 ✅ 5933 ms (11 iters) ✅ 6034 ms (11 iters) ≈ +1.7% (±46.6%) ⚠️ W:1 / ⚠️ W:1
finagle-chirper 25 ✅ 5472 ms (12 iters) ✅ 5412 ms (12 iters) ≈ -1.1% (±42.8%) ⚠️ W:1 / ⚠️ W:1
fj-kmeans 21 ✅ 2847 ms (22 iters) ✅ 2753 ms (23 iters) ≈ -3.3% (±4.3%) — / —
fj-kmeans 25 ✅ 2801 ms (22 iters) ✅ 2805 ms (22 iters) ≈ +0.1% (±4.6%) — / —
future-genetic 21 ✅ 2041 ms (30 iters) ✅ 2042 ms (30 iters) ≈ +0% (±4.6%) — / —
future-genetic 25 ✅ 2074 ms (30 iters) ✅ 1873 ms (33 iters) 🟢 -9.7% — / —
naive-bayes 21 ✅ 1277 ms (45 iters) ✅ 1223 ms (47 iters) ≈ -4.2% (±55%) — / —
naive-bayes 25 ✅ 1015 ms (56 iters) ✅ 1019 ms (56 iters) ≈ +0.4% (±55.6%) — / —
reactors 21 ✅ 16615 ms (5 iters) ✅ 15814 ms (5 iters) ≈ -4.8% (±14.3%) — / —
reactors 25 ✅ 17345 ms (5 iters) ✅ 18700 ms (5 iters) 🔴 +7.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 ✅ / ✅ ✅ / ✅ ✅ / 2 ✅ / 1828 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ ✅ / 4 ✅ / 2210 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 3 / ✅ 8521 / 8422 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 6 / 2 1250 / 1277 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 2 / ✅ 2907 / 2835 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 2 / 3 3459 / 3498 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 3 / ✅ 1680 / 1603 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 1749 / 1876 ✅ / ✅ ✅ / ✅

@dd-octo-sts

dd-octo-sts Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

1 failure(s) detected Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/123877235

❌ chaos: profiler tracer tcmalloc amd64 21 0 3 temXchaos
Chaos harness crashed (RC=124)
siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr: 0x0000000000000001
# C  [libjavaProfiler-dd-tmp309169057945649117.so+0x14070]

@jbachorik

Copy link
Copy Markdown
Collaborator

@zhengyu123 ping - do you want to finish this or it's to be closed?

@zhengyu123

Copy link
Copy Markdown
Contributor Author

@zhengyu123 ping - do you want to finish this or it's to be closed?

@jbachorik On my list, let me finish PR 639.

@dd-octo-sts

dd-octo-sts Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 15dbbf7)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/124031962 Commit: 15dbbf72207a12d0c387dfea07154ae7a4e46ba0

⚠️ Significant outliers

  • 🟢 fj-kmeans (JDK 25): runtime -3.1% (2846→2759 ms)
  • 🟢 future-genetic (JDK 21): runtime -4.6% (2112→2014 ms)
  • 🔴 future-genetic (JDK 25): runtime +6.9% (1976→2113 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10295 ms (21 iters) ✅ 10329 ms (21 iters) ≈ +0.3% (±11.1%) — / —
akka-uct 25 ✅ 8954 ms (24 iters) ✅ 8911 ms (24 iters) ≈ -0.5% (±11.1%) — / —
finagle-chirper 21 ✅ 6000 ms (33 iters) ✅ 5947 ms (33 iters) ≈ -0.9% (±25.3%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5513 ms (36 iters) ✅ 5545 ms (36 iters) ≈ +0.6% (±23.9%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2748 ms (68 iters) ✅ 2703 ms (70 iters) ≈ -1.6% (±2.7%) — / —
fj-kmeans 25 ✅ 2846 ms (66 iters) ✅ 2759 ms (68 iters) 🟢 -3.1% — / —
future-genetic 21 ✅ 2112 ms (88 iters) ✅ 2014 ms (93 iters) 🟢 -4.6% — / —
future-genetic 25 ✅ 1976 ms (94 iters) ✅ 2113 ms (88 iters) 🔴 +6.9% — / —
naive-bayes 21 ✅ 1252 ms (136 iters) ✅ 1290 ms (133 iters) ≈ +3% (±33.4%) — / —
naive-bayes 25 ✅ 1024 ms (167 iters) ✅ 1014 ms (168 iters) ≈ -1% (±31.7%) — / —
reactors 21 ✅ 17302 ms (15 iters) ✅ 16391 ms (15 iters) ≈ -5.3% (±6.4%) — / —
reactors 25 ✅ 18436 ms (15 iters) ✅ 18510 ms (15 iters) ≈ +0.4% (±6.3%) — / —
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 ✅ / ✅ ✅ / ✅ 2 / 1 1903 / 1893 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ ✅ / 2 2159 / 2309 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 4 / 6 9111 / 8785 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 2 / ✅ 8496 / 8523 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ ✅ / 1 1250 / 1290 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 4 / ✅ 1278 / 1281 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 1 / 3 2942 / 2978 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ ✅ / ✅ 2820 / 2987 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 6 / 7 3474 / 3544 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 4 / 5 3489 / 3441 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ ✅ / 1 1735 / 1718 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / 2 1874 / 1939 ✅ / ✅ ✅ / ✅

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.

3 participants