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 support for VirtualThread getStackTrace #15846

Merged
merged 5 commits into from
Sep 21, 2022
Merged

Add support for VirtualThread getStackTrace #15846

merged 5 commits into from
Sep 21, 2022

Conversation

fengxue-IS
Copy link
Contributor

@fengxue-IS fengxue-IS commented Sep 8, 2022 •
edited

  • Add copyFieldsFromContinuation helper in VMHelpers.hpp
  • Add locking mechanism on Continuation execution
  • Fix Thread.getStackTraceImpl to support VirtualThread

Closes: #15758

Signed-off-by: Jack Lu Jack.S.Lu@ibm.com

@fengxue-IS fengxue-IS added the project:loom Used to track Project Loom related work label Sep 8, 2022
@fengxue-IS fengxue-IS force-pushed the cont_stackwalker branch 9 times, most recently from e932dd7 to 855d35d Compare September 9, 2022 21:11
@fengxue-IS fengxue-IS changed the title [WIP] Add StackWalker support for Continuation Add support for VirtualThread getStackTrace Sep 9, 2022
@fengxue-IS fengxue-IS marked this pull request as ready for review September 9, 2022 21:12
@fengxue-IS fengxue-IS force-pushed the cont_stackwalker branch 4 times, most recently from 83e2cb2 to 87d8736 Compare September 13, 2022 20:42
@babsingh
Copy link
Contributor

babsingh commented Sep 13, 2022 •
edited

Move copyFieldsFromContinuation API to internalVMFunctions and associated changes: are also supposed to support the remaining JVMTI functions. They will be merged quicker in a separate PR because reviewing all the commits in this PR will take more time. This way, others working on the JVMTI functions, who need copyFieldsFromContinuation, won't be blocked on this PR.

@fengxue-IS
Copy link
Contributor Author

fengxue-IS commented Sep 14, 2022 •
edited

Opened #15893 for the copyFieldsFromContinuation helper

Will modify this PR

@fengxue-IS fengxue-IS changed the title Add support for VirtualThread getStackTrace [WIP] Add support for VirtualThread getStackTrace Sep 14, 2022
@fengxue-IS fengxue-IS force-pushed the cont_stackwalker branch 5 times, most recently from 6c2d26a to 76f9790 Compare September 16, 2022 21:24
@fengxue-IS fengxue-IS changed the title [WIP] Add support for VirtualThread getStackTrace Add support for VirtualThread getStackTrace Sep 16, 2022
@fengxue-IS
Copy link
Contributor Author

@tajila @babsingh Can you please take a look.

@fengxue-IS fengxue-IS mentioned this pull request Sep 20, 2022
Closed
@babsingh
Copy link
Contributor

Functionally, LGTM. Formatting needs some improvement but not a priority. Launching PR builds to verify functional correctness.

jenkins test sanity win,xlinux jdk19

@babsingh
Copy link
Contributor

jenkins test sanity amac jdk11

@babsingh
Copy link
Contributor

There are compiler errors:

10:57:39  /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/openj9/runtime/jcl/common/java_lang_StackWalker.cpp:142:2: error: unknown type name 'J9VMContinuation'
10:57:39          J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, continuationObject);
10:57:39          ^
10:57:39  [ 51%] Building CXX object runtime/gc_structs/CMakeFiles/j9gcstructs.dir/JVMTIObjectTagTableIterator.cpp.o
10:57:39  [ 51%] Building C object runtime/vm/CMakeFiles/j9vm.dir/drophelp.c.o
10:57:39  /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/openj9/runtime/jcl/common/java_lang_StackWalker.cpp:142:35: error: use of undeclared identifier 'J9VMJDKINTERNALVMCONTINUATION_VMREF'
10:57:39          J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, continuationObject);
10:57:39                                           ^
10:57:39  /Users/jenkins/workspace/Build_JDK11_aarch64_mac_Personal/openj9/runtime/jcl/common/java_lang_StackWalker.cpp:143:27: error: no member named 'copyFieldsFromContinuation' in 'J9InternalVMFunctions'
10:57:39          vm->internalVMFunctions->copyFieldsFromContinuation(vmThread, &stackThread, &els, continuation);
10:57:39          ~~~~~~~~~~~~~~~~~~~~~~~  ^
10:57:39  3 errors generated.

@fengxue-IS fengxue-IS force-pushed the cont_stackwalker branch 2 times, most recently from 5572772 to f5f394b Compare September 20, 2022 21:10
babsingh
babsingh reviewed Sep 20, 2022
View reviewed changes
Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

just for consistency: since the other two comments have a full-stop in the same method

runtime/jcl/common/thread.cpp Outdated Show resolved Hide resolved
runtime/jcl/common/thread.cpp Outdated Show resolved Hide resolved
@fengxue-IS
Fix MID_java_lang_StackWalker_walkWrapperImpl naming
f5df055 
Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@fengxue-IS
Continuation StackWalker support
6795f6d 
- Add copyFieldsFromContinuation helper in VMHelpers.hpp
- Add locking mechanism on Continuation execution

Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@fengxue-IS
Fix Java_java_lang_Thread_getStackTraceImpl to support VirtualThread
43f7f29 
Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@fengxue-IS
Add testing for VirtualThread.getStackTrace
a97274d 
Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@fengxue-IS
Replace ReentrantLock with unsafe.compareAndSetBoolean
4890f04 
Signed-off-by: Jack Lu <Jack.S.Lu@ibm.com>
@babsingh
Copy link
Contributor

jenkins test sanity win,xlinux jdk19

@babsingh
Copy link
Contributor

jenkins test sanity amac jdk11

@tajila tajila merged commit 7258354 into eclipse-openj9:master Sep 21, 2022
babsingh
babsingh reviewed Sep 21, 2022
View reviewed changes
Copy link
Contributor

@babsingh babsingh left a comment

Choose a reason for hiding this comment

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

Approved. LGTM.

@pshipton pshipton mentioned this pull request Sep 22, 2022
Closed
@fengxue-IS fengxue-IS mentioned this pull request Sep 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@babsingh babsingh babsingh left review comments

@tajila tajila tajila left review comments

Assignees
No one assigned
Labels
project:loom Used to track Project Loom related work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Loom: Continuation StackWalker support
3 participants
@fengxue-IS @babsingh @tajila