Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable ASGCT by default on fairly safe J9 JDK versions #181

Merged
merged 4 commits into from
Feb 18, 2025

Conversation

jbachorik
Copy link
Collaborator

@jbachorik jbachorik commented Feb 13, 2025

What does this PR do?:
It updates the 'safe' versions for J9 usage of ASGCT.
These versions have been tested and are behaving correctly.

Motivation:
The JVMTI based sampler is causing a noticeable overhead. Being able to use ASGCT where it is fairly safe will make the usage of profiler much nicer experience.

Additional Notes:
This change is adding support for forcing the 'jvmti' samplers by setting system property dd.profiling.ddprof.j9.sampler=jvmti

How to test the change?:
Added sanity tests which will be executed when running on J9 JVMs

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-11356

Unsure? Have a question? Request a review!

Copy link

github-actions bot commented Feb 13, 2025

🔧 Report generated by pr-comment-scanbuild

Scan-Build Report

User:runner@fv-az1671-17
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 clean all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Tue Feb 18 16:22:03 2025

Bug Summary

Bug TypeQuantityDisplay?
All Bugs7
Logic error
Dereference of null pointer3
Suspicious operation
Bitwise shift1
Unused code
Dead initialization2
Dead nested assignment1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Suspicious operationBitwise shiftvmStructs.cppfind89716
Unused codeDead initializationjavaApi.cppJava_com_datadoghq_profiler_JavaProfiler_getStatus01161
Unused codeDead initializationstackWalker.cppwalkDwarf1601
Unused codeDead nested assignmentvmStructs.cppcheckNativeBinding10021
Logic errorDereference of null pointersafeAccess.hload3318
Logic errorDereference of null pointersymbols_linux.hElfParser14428
Logic errorDereference of null pointerflightRecorder.cppflush15048

Copy link

github-actions bot commented Feb 13, 2025

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Warnings (7)

Style Violations (398)

@jbachorik jbachorik force-pushed the jb/j9_versions branch 3 times, most recently from 6880d41 to c8e0e50 Compare February 14, 2025 10:17
@jbachorik jbachorik merged commit 8d7f493 into main Feb 18, 2025
84 checks passed
@jbachorik jbachorik deleted the jb/j9_versions branch February 18, 2025 17:08
@github-actions github-actions bot added this to the 1.21.0 milestone Feb 18, 2025
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.

1 participant