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

Retain Continuation.vthread until the J9VMContinuation is freed #18251

Merged
merged 1 commit into from Oct 13, 2023
Merged

Retain Continuation.vthread until the J9VMContinuation is freed #18251

merged 1 commit into from Oct 13, 2023

Conversation

babsingh
Copy link
Contributor

@babsingh babsingh commented Oct 6, 2023

This PR is needed to enable the Skynet benchmark.

The changes in #18180 pass threadObject to
walkContinuationStackFrames.

The GC can walk the continuations until the associated native
J9VMContinuation structure is freed.

Since we rely on Continuation.vthread in walkContinuationStackFrames,
Continuation.vthread can only be unset once the native continuation
structure is freed.

Now, we set Continuation.vthread to NULL after the continuation is
freed. This will prevent a segfault which can occur due to
#18180.

Continuation.vthread is also used in JVMTI to not suspend virtual
threads once they enter the last unmount phase. This won't work anymore
because Continuation.vthread is no longer set to NULL at the start of
the last unmount phase. To address this issue, another continuation
state has been added to indicate that a virtual thread's continuation
has entered the last unmount phase.

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

@babsingh
Retain Continuation.vthread until the J9VMContinuation is freed
e64ca00 
This PR is needed to enable the Skynet benchmark.

The changes in eclipse-openj9#18180 pass threadObject to
walkContinuationStackFrames.

The GC can walk the continuations until the associated native
J9VMContinuation structure is freed.

Since we rely on Continuation.vthread in walkContinuationStackFrames,
Continuation.vthread can only be unset once the native continuation
structure is freed.

Now, we set Continuation.vthread to NULL after the continuation is
freed. This will prevent a segfault which can occur due to
eclipse-openj9#18180.

Continuation.vthread is also used in JVMTI to not suspend virtual
threads once they enter the last unmount phase. This won't work anymore
because Continuation.vthread is no longer set to NULL at the start of
the last unmount phase. To address this issue, another continuation
state has been added to indicate that a virtual thread's continuation
has entered the last unmount phase.

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh
babsingh commented Oct 6, 2023
View reviewed changes
runtime/jvmti/jvmtiThread.c
Comment on lines +1329 to +1331
ContinuationState continuationState = J9VMJDKINTERNALVMCONTINUATION_STATE(vmThread, continuationObj);;

if ((NULL != vthread) && J9_ARE_NO_BITS_SET(continuationState, J9_GC_CONTINUATION_STATE_LAST_UNMOUNT)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fengxue-IS This source code is in C; we can't invoke C++ code from ContinuationHelpers.hpp here; so, I have employed a workaround.

@babsingh babsingh requested a review from tajila October 6, 2023 15:28
fengxue-IS
fengxue-IS approved these changes Oct 6, 2023
View reviewed changes
Copy link
Contributor

@fengxue-IS fengxue-IS left a comment

Choose a reason for hiding this comment

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

LGTM

@babsingh
Copy link
Contributor Author

babsingh commented Oct 6, 2023

jenkins test sanity zlinux jdk21

@babsingh
Copy link
Contributor Author

babsingh commented Oct 6, 2023

jenkins compile win jdk21

babsingh added a commit to babsingh/openj9-openjdk-jdk21 that referenced this pull request Oct 6, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
49276a4 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
This was referenced Oct 6, 2023
Merged
Merged
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this pull request Oct 6, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
f8f544a 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/openj9-openjdk-jdk that referenced this pull request Oct 6, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
091a0db 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/openj9-openjdk-jdk21 that referenced this pull request Oct 6, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
e835d1d 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this pull request Oct 6, 2023
@babsingh
[JDK21/JDK22] Enable virtual/stress/Skynet
52fc7dd 
Depends on eclipse-openj9/openj9#18251
Depends on ibmruntimes/openj9-openjdk-jdk21#53
Depends on ibmruntimes/openj9-openjdk-jdk#675

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this pull request Oct 6, 2023
@babsingh
[JDK21/JDK22] Enable virtual/stress/Skynet
30a8670 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to babsingh/aqa-tests that referenced this pull request Oct 6, 2023
@babsingh
[JDK21/JDK22] Enable virtual/stress/Skynet
40d1f7b 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675
- Related: eclipse-openj9/openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
This was referenced Oct 6, 2023
Merged
Closed
Closed
Closed
@tajila
Copy link
Contributor

tajila commented Oct 12, 2023

jenkins test sanity alinux64 jdk21

@tajila
Copy link
Contributor

tajila commented Oct 12, 2023

jenkins compile win jdk8

@tajila tajila merged commit 3ad3c3b into eclipse-openj9:master Oct 13, 2023
9 checks passed
babsingh added a commit to ibmruntimes/openj9-openjdk-jdk21 that referenced this pull request Oct 16, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
5b974ba 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
babsingh added a commit to ibmruntimes/openj9-openjdk-jdk that referenced this pull request Oct 16, 2023
@babsingh
Run virtual/stress/Skynet with -Xnocompressedrefs
8ccfe36 
Skynet is a stress benchmark, which exceeds the below 4G capacity
of -Xcompressedrefs. The current workaround is to use
-Xnocompressedrefs where memory can be allocated outside the 4G
boundary.

To run Skynet with -Xcompressedrefs, the following features will be
needed:
- Move unmounted continuation stacks out of the low memory area.
- Improve performance of the sub-4G allocator so that it doesn't
  regress under high memory usage.

Depends on eclipse-openj9/openj9#18251

Related: eclipse-openj9/openj9#16728

Related: eclipse-openj9/openj9#15781

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia pushed a commit to adoptium/aqa-tests that referenced this pull request Oct 17, 2023
@babsingh
[JDK21/JDK22] Enable virtual/stress/Skynet (#4801)
acb19ca 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675
- Related: eclipse-openj9/openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia pushed a commit to llxia/openjdk-tests that referenced this pull request Nov 22, 2023
@babsingh @llxia
[JDK21/JDK22] Enable virtual/stress/Skynet (adoptium#4801)
deefede 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675
- Related: eclipse-openj9/openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
llxia pushed a commit to llxia/openjdk-tests that referenced this pull request Nov 22, 2023
@babsingh @llxia
[JDK21/JDK22] Enable virtual/stress/Skynet (adoptium#4801)
d02a638 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675
- Related: eclipse-openj9/openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
pshipton pushed a commit to adoptium/aqa-tests that referenced this pull request Nov 22, 2023
@babsingh @pshipton
[JDK21/JDK22] Enable virtual/stress/Skynet (#4801)
63b59cc 
- Depends on eclipse-openj9/openj9#18251
- Depends on ibmruntimes/openj9-openjdk-jdk21#53
- Depends on ibmruntimes/openj9-openjdk-jdk#675
- Related: eclipse-openj9/openj9#16728

Signed-off-by: Babneet Singh <sbabneet@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fengxue-IS fengxue-IS fengxue-IS approved these changes

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

None yet
4 participants
@babsingh @tajila @fengxue-IS @pshipton