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.

Sign up for GitHub

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

Jump to bottom

Add triggerExecutionSample native #19917

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Add triggerExecutionSample native #19917

merged 1 commit into from
Jul 30, 2024

Conversation

thallium
Copy link
Contributor

  • Add JPP flag for JFR
  • Add com.ibm.oti.vm.VM::triggerExecutionSample that calls jfrExecutionSample on all Java threads
  • Add test for triggerExecutionSample

@thallium
Copy link
Contributor Author

@tajila Can you please take a look? I'm not quite familiar with adding tests so I'm expecting to miss something e.g. only enable the test when JFR is enabled.

@tajila tajila requested a review from JasonFengJ9 July 25, 2024 01:31
@tajila
Copy link
Contributor

tajila commented Jul 25, 2024

Please name this PR something like "Add triggerExecutionSample native"

@thallium thallium changed the title JFR Work Add triggerExecutionSample native Jul 25, 2024
thallium added a commit to thallium/openj9-openjdk-jdk21 that referenced this pull request Jul 25, 2024
@thallium
Add JFR JPP flag
416c52e 
Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@tajila
Copy link
Contributor

tajila commented Jul 25, 2024

@JasonFengJ9 Please review these changes

@thallium thallium mentioned this pull request Jul 25, 2024
Merged
thallium added a commit to thallium/openj9-openjdk-jdk that referenced this pull request Jul 25, 2024
@thallium
Add JFR JPP flag
83053a4 
Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@thallium thallium mentioned this pull request Jul 25, 2024
Merged
thallium added a commit to thallium/openj9-openjdk-jdk that referenced this pull request Jul 25, 2024
@thallium
Add JFR JPP flag
6336dd2 
Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
thallium added a commit to thallium/openj9-openjdk-jdk21 that referenced this pull request Jul 25, 2024
@thallium
Add JFR JPP flag
0de6e8b 
Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@thallium
Copy link
Contributor Author

Addressed all the feedbacks above.

@JasonFengJ9
Copy link
Member

15:17:42 test/functional/cmdLineTests/jfr/metadata.blob:164: trailing whitespace.
15:17:42 +��S����������

Is there a way to generate this file dynamically? otherwise, the line endings check needs to skip this type of file.

thallium added a commit to thallium/TKG that referenced this pull request Jul 26, 2024
@thallium
Add JFR check to test flags
18d0c67 
This will help JFR related tests in the future.

Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@tajila
Copy link
Contributor

tajila commented Jul 26, 2024 

15:17:42 test/functional/cmdLineTests/jfr/metadata.blob:164: trailing whitespace.
15:17:42 +��S����������

Is there a way to generate this file dynamically? otherwise, the line endings check needs to skip this type of file.

There is no way to do this currently. We need to skip file endings check for this file.

@JasonFengJ9 JasonFengJ9 mentioned this pull request Jul 26, 2024
Closed
@JasonFengJ9
Copy link
Member

Opened an issue

@thallium thallium mentioned this pull request Jul 26, 2024
Merged
@thallium
Copy link
Contributor Author

Opend PR in TKG:
adoptium/TKG#589

@thallium
Copy link
Contributor Author

Updated PR.

JasonFengJ9 pushed a commit to JasonFengJ9/openj9-openjdk-jdk that referenced this pull request Jul 28, 2024
@thallium @JasonFengJ9
Add JFR JPP flag
be2983d 
Related: eclipse-openj9/openj9#19917

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@JasonFengJ9
Copy link
Member

Opend PR in TKG:
adoptium/TKG#589

Merged

JasonFengJ9
JasonFengJ9 approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@JasonFengJ9 JasonFengJ9 left a comment

Choose a reason for hiding this comment

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

LGTM

@tajila
Copy link
Contributor

tajila commented Jul 29, 2024

jenkins test sanity xlinux jdk17

keithc-ca
keithc-ca requested changes Jul 29, 2024
View reviewed changes
runtime/jcl/common/com_ibm_oti_vm_VM.cpp Show resolved Hide resolved
runtime/jcl/exports.cmake Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/build.xml Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/build.xml Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/build.xml Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/jfr.xml Outdated Show resolved Hide resolved
@keithc-ca
Copy link
Contributor

.gitattributes should be updated to identify *.blob as binary files.

@thallium
Copy link
Contributor Author

thallium commented Jul 29, 2024 •
edited
Loading

@keithc-ca all your feedbacks have been addressed. Also can we get ibmruntimes/openj9-openjdk-jdk#817 merged?

keithc-ca
keithc-ca requested changes Jul 29, 2024
View reviewed changes
.gitattributes Outdated Show resolved Hide resolved
runtime/jcl/exports.cmake Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/build.xml Outdated Show resolved Hide resolved
test/functional/cmdLineTests/jfr/playlist.xml Outdated Show resolved Hide resolved
@keithc-ca
Copy link
Contributor

The commit message says this "Depend on" ibmruntimes/openj9-openjdk-jdk#817; the dependency is in the other direction. Please correct that.

@thallium
Add triggerExecutionSample native
3f48ea9 
- Add JPP flag for JFR
- Add com.ibm.oti.vm.VM::triggerExecutionSample that calls
  jfrExecutionSample on all Java threads
- Add test for triggerExecutionSample

Related:
- ibmruntimes/openj9-openjdk-jdk#817

Depend on:
- adoptium/TKG#589

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
@tajila
Copy link
Contributor

tajila commented Jul 30, 2024

@keithc-ca Any further comments?

@keithc-ca
Copy link
Contributor

I believe the previous PR build (https://openj9-jenkins.osuosl.org/job/Build_JDK17_x86-64_linux_Personal/907) is still valid.

@keithc-ca keithc-ca merged commit a3a0974 into eclipse-openj9:master Jul 30, 2024
2 checks passed
thallium added a commit to thallium/openj9-openjdk-jdk23 that referenced this pull request Jul 30, 2024
@thallium
Add JFR JPP flag
1fe9c11 
Depend on: eclipse-openj9/openj9#19917
Related: ibmruntimes/openj9-openjdk-jdk#817

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
thallium added a commit to thallium/openj9-openjdk-jdk22 that referenced this pull request Jul 30, 2024
@thallium
Add JFR JPP flag
fc41bc5 
Depend on: eclipse-openj9/openj9#19917
Related: ibmruntimes/openj9-openjdk-jdk#817

Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>
This was referenced Jul 30, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tajila tajila tajila left review comments

@keithc-ca keithc-ca keithc-ca approved these changes

@JasonFengJ9 JasonFengJ9 JasonFengJ9 approved these changes

Assignees
No one assigned
Labels
comp:vm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thallium @tajila @JasonFengJ9 @keithc-ca