Skip to content

Add performance counters/metrics for samplers - #811

Merged
zhengyu123 merged 12 commits into
mainfrom
zgu/performance
Sep 24, 2026
Merged

zhengyu123 merged 12 commits into
mainfrom
zgu/performance

Conversation

@zhengyu123

@zhengyu123 zhengyu123 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?:
Adds an opt-in (-PenableSamplerPerf), zero-cost-when-disabled instrumentation layer (SamplerPerf/SAMPLER_PERF_PROBE) that measures elapsed wall time and sample count per sampler family (CPU, wallclock, alloc, liveness, nativemem, nativesocket). Each sampler's signal handler or hot path gets a probe call (ctimer_linux.cpp, itimer.cpp, perfEvents_linux.cpp, wallClock.cpp, j9/j9WallClock.cpp, objectSampler.cpp, livenessTracker.cpp, mallocTracer.cpp, nativeSocketSampler.cpp). Counters are exposed through the existing Counters/JFR/getDebugCounters() machinery (counters.h) and a summary table is printed at Profiler::stop() via SamplerPerf::report(). Also adds Agent_OnUnload in vmEntry.cpp so JVMTI-only (-agentpath) loads get the same shutdown path as JNI_OnUnload. New unit tests in samplerPerf_ut.cpp and a new doc page, doc/reference/SamplerPerfCounters.md, explain the caveats around reading the numbers (elapsed vs CPU time, cpu/wall subset relationships, cross-sampler nesting, bailed-out samples, and a bounded shutdown-time undercount race).

A sample output:

[java-profiler] sampler performance (elapsed wall time, not CPU time):
[java-profiler]   sampler            total (ms)        count     avg (ns)
[java-profiler]   cpu                  1273.333        59979        21229
[java-profiler]   note: cpu includes unwinding_ticks_async, and alloc includes liveness; do not sum them.

Motivation:
There was no way to measure each sampler's own per-sample overhead in isolation — existing counters track totals but not per-engine timing, making it hard to attribute latency regressions to a specific sampler.

Additional Notes:
Default builds are unaffected — without -PenableSamplerPerf the macro expands to ((void)0) and the counters occupy no enum slot or storage.

How to test the change?:
Build with -PenableSamplerPerf and run the new samplerPerf_ut.cpp gtest suite (wired into GtestTaskBuilder.kt/ConfigurationPresets.kt the same way as the existing faultInjection flag). For an end-to-end check, run any profiled JVM workload with the flag enabled and confirm the [java-profiler] sampler performance table prints at shutdown.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a security review (run the dd:platform-security-review
    skill, or file a request via the PSEC review form).
    bewaire also runs automatically on every PR.
  • This PR doesn't touch any of that.
  • JIRA: PROF-16025

Unsure? Have a question? Request a review!

@dd-octo-sts

dd-octo-sts Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #35935274258 | Commit: 7cf38f8 | Duration: 15m 16s (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-09-24 00:05:43 UTC

@dd-octo-sts

dd-octo-sts Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

39 passed, 1 failed out of 40 expected configurations

Test Matrix

Platform JDK 8 JDK 11 JDK 17 JDK 21 JDK 25
glibc-x64-hotspot
glibc-x64-openj9
glibc-arm64-hotspot
glibc-arm64-openj9
musl-x64-hotspot
musl-x64-openj9
musl-arm64-hotspot
musl-arm64-openj9

Failure Details

glibc-x64-openj9-jdk21

Profiler-only:

"  Skipped (tracer not expected in this configuration)"
""
"[7/8] Checking for unexpected events..."
"  ? No unexpected events found"
""
"[8/8] Scenario-specific validation (ddprof_only)..."
"  Validating profiler-only scenario..."
"  ? Profiler-only scenario checks passed"
""
"=== Validation Summary ==="
"ExecutionSample:            0.0 events (datadog.ExecutionSample)"
"Stack traces:               0.0 samples"
"Thread diversity:           0.0 threads"
"Allocation samples:         367 events (datadog.ObjectSample)"
"ThreadAllocationStatistics: 43 events"
""
"VALIDATION_FAILED: One or more checks did not pass"

Script executed successfully: 425 commands
VALIDATION_FAILED: Validation checks did not pass

Links

@zhengyu123
zhengyu123 marked this pull request as ready for review September 21, 2026 20:32
@zhengyu123
zhengyu123 requested a review from a team as a code owner September 21, 2026 20:32

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 Bot 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.

Bits Code Review: FAIL

Two of the most critical issues are a shutdown race where probe counter updates escape inflight tracking (potentially corrupting profiling reports) and a missing extern "C" on Agent_OnUnload that silently prevents the JVM from invoking the shutdown callback on agentpath-loaded libraries. Additionally, J9Execution mode records to the wrong performance counter, and unsigned TSC subtraction can wrap on core migrations, both corrupting measurement data.

Open Bits AI session

🤖 Bits Code Review · Commit eaf6b5b · @DataDog review to ask questions

Comment thread ddprof-lib/src/main/cpp/ctimer_linux.cpp
Comment thread ddprof-lib/src/main/cpp/j9/j9WallClock.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/vmEntry.cpp Outdated
Comment thread ddprof-lib/src/main/cpp/samplerPerf.h Outdated

@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: eaf6b5b904

ℹ️ 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-lib/src/main/cpp/nativeSocketSampler.cpp
zhengyu123 and others added 2 commits September 21, 2026 17:13
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>

@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: a1eb67754e

ℹ️ 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-lib/src/main/cpp/mallocTracer.cpp
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Pipelines

Unblock PR with BitsAI

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 2 Pipeline jobs failed

DataDog/java-profiler | integration-test-x64-glibc: [openj9, 21] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/java-profiler | post-pr-comment — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 97b79ee | Docs | View more details | Give us feedback!

@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: 9774278e51

ℹ️ 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-lib/src/main/cpp/j9/j9WallClock.cpp Outdated

@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: 23f9c7c3f0

ℹ️ 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-lib/src/main/cpp/samplerPerf.h
@dd-octo-sts

dd-octo-sts Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

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

❌ profiler gmalloc amd64Xjit
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler gmalloc amd64Xmemory
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler jemalloc aarch64Xjit
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler jemalloc amd64Xjit
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler tcmalloc amd64Xjit
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler tcmalloc amd64Xmemory
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ profiler tracer tcmalloc amd64Xjit
Could not fetch ddprof 1.51.0-zgu_performance-SNAPSHOT jar
❌ chaos: profiler gmalloc aarch64 21 0 3 temXchaos
JDK 21.0.3-tem download failed
❌ chaos: profiler gmalloc aarch64 25 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler gmalloc amd64 21 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler gmalloc amd64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler jemalloc aarch64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler jemalloc aarch64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler jemalloc amd64 25 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tcmalloc aarch64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tcmalloc aarch64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler tcmalloc amd64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tcmalloc amd64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler tracer gmalloc aarch64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer gmalloc aarch64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler tracer gmalloc amd64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer gmalloc amd64 25 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer jemalloc aarch64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer jemalloc aarch64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler tracer jemalloc amd64 21 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)
❌ chaos: profiler tracer jemalloc amd64 25 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer tcmalloc aarch64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer tcmalloc aarch64 25 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer tcmalloc amd64 21 0 3 temXchaos
chaos.jar unavailable
❌ chaos: profiler tracer tcmalloc amd64 25 0 3 temXchaos
ddprof jar unavailable (Maven snapshot download failed)

@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.

LGTM

@kaahos kaahos 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.

Looks good to me! Just 1 nit comment regarding the documentation which might not be up-to-date - nothing blocking though.

Comment thread doc/reference/SamplerPerfCounters.md
@zhengyu123
zhengyu123 merged commit c001b67 into main Sep 24, 2026
249 of 284 checks passed
@zhengyu123
zhengyu123 deleted the zgu/performance branch September 24, 2026 00:08
@github-actions github-actions Bot added this to the 1.51.0 milestone Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants