Implement JVM_VirtualThreadHideFrames() #16654

thallium · 2023-02-02T17:19:07Z

Hide some JVM TI events after notifyJvmtiHideFrames(true) is called.

Added a helper to decide whether a event should be dispatched.

Signed-off-by: Gengchen Tuo gengchen.tuo@ibm.com

Fixes: #16012

thallium · 2023-02-02T17:30:50Z

@fengxue-IS FYI
Currently the following events are hidden:

Other things to notice:

In OpenJDK, in function with @JvmtiMountTransition annotation, the Exception Catch event is not sent but Exception event is still sent.
OpenJDK seems to be hiding VirtualThread.mount() and VirtualThread.unmount(), need to do further verification on that.
Events triggered by JNI method is not hidden yet, but should be easy to do?

thallium · 2023-02-15T15:34:22Z

@fengxue-IS can you please take a look

fengxue-IS

minor nit pick on formatting

runtime/bcutil/ClassFileOracle.hpp

runtime/oti/j9.h

thallium · 2023-03-29T19:52:35Z

With the change the events mentioned above are hidden which matches the behaviour to RI.

fengxue-IS · 2023-03-30T18:54:29Z

In OpenJDK, in function with @JvmtiMountTransition annotation, the Exception Catch event is not sent but Exception event is still sent.

ExceptionCatch have known issues in how it is triggered in RI, see #5785.
It does make sense that if RI is not hiding exception event, this maybe considered a error path so exception event shouldn't be hidden. I will double check this to confirm if we should match RI.

OpenJDK seems to be hiding VirtualThread.mount() and VirtualThread.unmount(), need to do further verification on that.

Have this been confirmed?

Events triggered by JNI method is not hidden yet, but should be easy to do?

JNI events that are equivalent to Java events should also be hidden (those that map the the same JVMTI_EVENT code in jvmtiHook.c.)

thallium · 2023-04-05T17:54:27Z

Have this been confirmed?

This is due to openjdk's implementation and is not part of the issue that this PR is trying to fix.

Also field access and modification event triggered by JNI is now hidden.

fengxue-IS

lgtm, the hiding of normal mount transition will be pursued as part of #16984

@babsingh please take a look

runtime/oti/j9.h

runtime/codert_vm/cnathelp.cpp

babsingh · 2023-04-10T18:19:55Z

What testing was done to verify these changes?
Do we plan to enable tests after this PR is merged?

++ @gacholio for review.

thallium · 2023-04-10T18:35:26Z

What testing was done to verify these changes?

I modified the VirtualThread.java to try to trigger some events after notifyJvmtiHideFrames() is called and see if any event is received by a JVMTI agent.

Do we plan to enable tests after this PR is merged?

No, I don't think there's a related test.

runtime/oti/j9.h

gacholio · 2023-04-12T16:53:12Z

If the goal here is to hide JVMTI events, the work should really be done in JVMTI, not at the hook dispatch points. Hooks can be used for more than just JVMTI.

runtime/j9vm/javanextvmi.cpp

runtime/oti/j9.h

runtime/oti/VMHelpers.hpp

keithc-ca · 2023-04-12T20:10:13Z

If the goal here is to hide JVMTI events, the work should really be done in JVMTI, not at the hook dispatch points. Hooks can be used for more than just JVMTI.

I agree.

thallium · 2023-04-12T20:43:03Z

If the goal here is to hide JVMTI events, the work should really be done in JVMTI, not at the hook dispatch points. Hooks can be used for more than just JVMTI.

Makes sense, I'll work on that.

runtime/jvmti/jvmtiHook.c

gacholio · 2023-05-02T06:50:18Z

The crash is likely due to a missing check that the vthread is mounted, which would result in NULL carrier thread.

Who actually calls this?

thallium · 2023-05-02T14:57:10Z

Who actually calls this?

It's called in VirtualThread.switchToCarrierThread and VirtualThread.switchToVirtualThread.

runtime/j9vm/javanextvmi.cpp

runtime/jvmti/jvmtiHook.c

thallium · 2023-05-02T17:54:49Z

Sorry I just realized I force pushed an old branch, should be fixed now.

runtime/jvmti/jvmtiHook.c

keithc-ca · 2023-05-02T18:53:42Z

Please squash the commits; this modifies and then reverts changes, e.g. jvmtiExtensionMechanism.c.

thallium · 2023-05-02T19:45:29Z

@keithc-ca Squashed and addressed the changes, sorry about the mess.

thallium · 2023-05-02T22:21:31Z

test/hotspot/jtreg/serviceability/jvmti/stress/StackTrace/Suspended/GetStackTraceSuspendedStressTest.java

The above test is currently disabled. I saw a crash in JVM_VirtualThreadHideFrames while running this test locally.

JVMDUMP039I Processing dump event "gpf", detail "" at 2023/04/28 13:51:50 - please wait.
Unhandled exception
Type=Segmentation error vmState=0x00040000
J9Generic_Signal_Number=00000018 Signal_Number=0000000b Error_Value=00000000 Signal_Code=00000001
Handler1=00007FA00843E930 Handler2=00007FA008239400 InaccessibleAddress=0000000000000008
RDI=0000000000000000 RSI=0000000000000002 RAX=0000000000000004 RBX=0000000000285300
RCX=0000000000000000 RDX=0000000000000000 R8=0000000000285300 R9=00007FA004018A20
R10=0000000000000000 R11=00007FA0086FDD70 R12=00000000FFEE4210 R13=0000000000000001
R14=00007FA0085F171C R15=00007F9FB090F578
RIP=00007FA00864781B GS=0000 FS=0000 RSP=00007F9FB090F2B0
EFlags=0000000000010202 CS=0033 RBP=0000000000000000 ERR=0000000000000004
TRAPNO=000000000000000E OLDMASK=0000000000000000 CR2=0000000000000008
xmm0 00007f9fb090f560 (f: 2962289920.000000, d: 6.932919e-310)
xmm1 00007fa0085f1690 (f: 140449424.000000, d: 6.932992e-310)
xmm2 00000000001d7a00 (f: 1931776.000000, d: 9.544242e-318)
xmm3 00000000003f8630 (f: 4163120.000000, d: 2.056855e-317)
xmm4 00000000003f8710 (f: 4163344.000000, d: 2.056965e-317)
xmm5 00000000003f8718 (f: 4163352.000000, d: 2.056969e-317)
xmm6 00000000003f8710 (f: 4163344.000000, d: 2.056965e-317)
xmm7 00000000003f8718 (f: 4163352.000000, d: 2.056969e-317)
xmm8 0000000000284d41 (f: 2641217.000000, d: 1.304935e-317)
xmm9 0000000000260968 (f: 2492776.000000, d: 1.231595e-317)
xmm10 0000000000000003 (f: 3.000000, d: 1.482197e-323)
xmm11 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm12 00000000001d7770 (f: 1931120.000000, d: 9.541000e-318)
xmm13 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm14 0000000000000000 (f: 0.000000, d: 0.000000e+00)
xmm15 00007fa00861ab50 (f: 140618576.000000, d: 6.932992e-310)
Module=/root/openj9-openjdk-jdk20/build/linux-x86_64-server-release/images/jdk/lib/default/libjvm.so
Module_base_address=00007FA008627000 Symbol=JVM_VirtualThreadHideFrames
Symbol_address=00007FA008647780
Target=2_90_20230329_000000 (Linux 5.4.0-147-generic)
CPU=amd64 (8 logical CPUs) (0x3e884e000 RAM)
----------- Stack Backtrace -----------
JVM_VirtualThreadHideFrames+0x9b (0x00007FA00864781B [libjvm.so+0x2081b])
ffi_call_unix64+0x56 (0x00007FA0085CC016 [libj9vm29.so+0x1cc016])
ffi_call_int+0x1bc (0x00007FA0085CAFCC [libj9vm29.so+0x1cafcc])
_ZN37VM_DebugBytecodeInterpreterCompressed3runEP10J9VMThread+0x1c0fd (0x00007FA008500CBD [libj9vm29.so+0x100cbd])
debugBytecodeLoopCompressed+0xd5 (0x00007FA0084E4BB5 [libj9vm29.so+0xe4bb5])
 (0x00007FA008559BB2 [libj9vm29.so+0x159bb2])

@babsingh changing notifyJvmtiHideFrames(false) to vthread.notifyJvmtiHideFrames(false) will make the crash gone and fail as a duplication of #17123. Looks like hotspot always does the operation on the current thread.

runtime/j9vm/javanextvmi.cpp

fengxue-IS

@thallium can you confirm that the updated code still passes tests you've wrote.

Also, should we consider add those test to the Java20andUp @babsingh ?

babsingh · 2023-05-03T19:02:05Z

Also, should we consider add those test to the Java20andUp @babsingh ?

Yes, that's a good idea. More testing is always preferred. Adding them to the sanity.functional suite will help us identify breakages early-on. To not delay the merge of this PR, the new tests can added in a separate PR.

Hide some JVM TI events after notifyJvmtiHideFrames(true) is called. Fixes: eclipse-openj9#16012 Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

thallium · 2023-05-03T20:21:39Z

can you confirm that the updated code still passes tests you've wrote.

@fengxue-IS Yes

fengxue-IS · 2023-05-08T17:46:08Z

@keithc-ca all previous review comments have been addressed, can you take another look

keithc-ca · 2023-05-08T19:21:24Z

Jenkins test sanity amac jdk17,jdk20

The shouldPostEvent check was missed for the JVMTI FramePop event in eclipse-openj9#16654. Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

thallium force-pushed the hideFramesPR branch 2 times, most recently from aded17b to 96014a5 Compare February 15, 2023 15:32

thallium marked this pull request as ready for review February 15, 2023 15:34

fengxue-IS reviewed Mar 2, 2023

View reviewed changes

runtime/bcutil/ClassFileOracle.hpp Outdated Show resolved Hide resolved

runtime/oti/j9.h Outdated Show resolved Hide resolved

thallium force-pushed the hideFramesPR branch from 96014a5 to 6b5afc0 Compare March 2, 2023 23:07

thallium force-pushed the hideFramesPR branch 2 times, most recently from 4f1bffe to dc7b5c3 Compare March 29, 2023 18:34

thallium force-pushed the hideFramesPR branch from dc7b5c3 to e6f75f6 Compare April 5, 2023 17:47

thallium requested a review from fengxue-IS April 5, 2023 20:53

thallium mentioned this pull request Apr 6, 2023

Implement JVM_VirtualThread[Mount|Unmount|Start|End] #17125

Closed

fengxue-IS approved these changes Apr 6, 2023

View reviewed changes

babsingh reviewed Apr 10, 2023

View reviewed changes

runtime/oti/j9.h Outdated Show resolved Hide resolved

runtime/codert_vm/cnathelp.cpp Outdated Show resolved Hide resolved

thallium force-pushed the hideFramesPR branch from e6f75f6 to 82ef67e Compare April 12, 2023 15:28

babsingh reviewed Apr 12, 2023

View reviewed changes

runtime/oti/j9.h Outdated Show resolved Hide resolved

thallium force-pushed the hideFramesPR branch from 82ef67e to f39b0cd Compare April 12, 2023 15:52

babsingh approved these changes Apr 12, 2023

View reviewed changes

thallium mentioned this pull request Apr 12, 2023

JDK20 requires implementation of JVM_VirtualThreadHideFrames() #16012

Closed

keithc-ca requested changes Apr 12, 2023

View reviewed changes

runtime/j9vm/javanextvmi.cpp Outdated Show resolved Hide resolved

runtime/oti/j9.h Outdated Show resolved Hide resolved

runtime/oti/VMHelpers.hpp Outdated Show resolved Hide resolved

thallium force-pushed the hideFramesPR branch from f39b0cd to c2549d9 Compare April 13, 2023 20:26

babsingh reviewed Apr 14, 2023

View reviewed changes

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

keithc-ca self-requested a review May 2, 2023 17:22

keithc-ca requested changes May 2, 2023

View reviewed changes

runtime/j9vm/javanextvmi.cpp Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

keithc-ca requested changes May 2, 2023

View reviewed changes

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

runtime/jvmti/jvmtiHook.c Show resolved Hide resolved

runtime/jvmti/jvmtiHook.c Outdated Show resolved Hide resolved

thallium force-pushed the hideFramesPR branch 2 times, most recently from bd63e6d to 906ca1e Compare May 2, 2023 19:35

fengxue-IS suggested changes May 3, 2023

View reviewed changes

runtime/j9vm/javanextvmi.cpp Outdated Show resolved Hide resolved

babsingh mentioned this pull request May 3, 2023

JVMTI framepop02.java#id1 checks outside a virtual thread's scope #16346

Closed

thallium force-pushed the hideFramesPR branch from 906ca1e to a0a2cae Compare May 3, 2023 17:19

fengxue-IS approved these changes May 3, 2023

View reviewed changes

gacholio approved these changes May 3, 2023

View reviewed changes

Implement JVM_VirtualThreadHideFrames()

7e91e03

Hide some JVM TI events after notifyJvmtiHideFrames(true) is called. Fixes: eclipse-openj9#16012 Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

thallium force-pushed the hideFramesPR branch from a0a2cae to 7e91e03 Compare May 3, 2023 19:59

keithc-ca approved these changes May 8, 2023

View reviewed changes

keithc-ca merged commit 36f6357 into eclipse-openj9:master May 8, 2023

thallium mentioned this pull request Jun 8, 2023

[JDK21] Support JvmtiMountTransition Annotation #17520

Closed

thallium mentioned this pull request Jun 15, 2023

Add shouldPostEvent check for JVMTI FramePop event #17604

Merged

thallium added a commit to thallium/openj9 that referenced this pull request Jun 16, 2023

Add shouldPostEvent check for JVMTI FramePop event

33051de

The shouldPostEvent check was missed for the JVMTI FramePop event in eclipse-openj9#16654. Signed-off-by: Gengchen Tuo <gengchen.tuo@ibm.com>

babsingh mentioned this pull request Jul 10, 2023

Add a walk state flag to skip methods with the JvmtiMountTransition annotation #17758

Closed

babsingh mentioned this pull request Nov 10, 2023

Spin during VirtualThread MountBegin and UnmountBegin #18439

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement JVM_VirtualThreadHideFrames() #16654

Implement JVM_VirtualThreadHideFrames() #16654

thallium commented Feb 2, 2023

thallium commented Feb 2, 2023

thallium commented Feb 15, 2023

fengxue-IS left a comment •

edited

Loading

thallium commented Mar 29, 2023

fengxue-IS commented Mar 30, 2023

thallium commented Apr 5, 2023 •

edited

Loading

fengxue-IS left a comment

babsingh commented Apr 10, 2023

thallium commented Apr 10, 2023

gacholio commented Apr 12, 2023

keithc-ca commented Apr 12, 2023

thallium commented Apr 12, 2023

gacholio commented May 2, 2023

thallium commented May 2, 2023

thallium commented May 2, 2023

keithc-ca commented May 2, 2023

thallium commented May 2, 2023

thallium commented May 2, 2023

fengxue-IS left a comment

babsingh commented May 3, 2023

thallium commented May 3, 2023

fengxue-IS commented May 8, 2023

keithc-ca commented May 8, 2023

Implement JVM_VirtualThreadHideFrames() #16654

Implement JVM_VirtualThreadHideFrames() #16654

Conversation

thallium commented Feb 2, 2023

thallium commented Feb 2, 2023

thallium commented Feb 15, 2023

fengxue-IS left a comment • edited Loading

Choose a reason for hiding this comment

thallium commented Mar 29, 2023

fengxue-IS commented Mar 30, 2023

thallium commented Apr 5, 2023 • edited Loading

fengxue-IS left a comment

Choose a reason for hiding this comment

babsingh commented Apr 10, 2023

thallium commented Apr 10, 2023

gacholio commented Apr 12, 2023

keithc-ca commented Apr 12, 2023

thallium commented Apr 12, 2023

gacholio commented May 2, 2023

thallium commented May 2, 2023

thallium commented May 2, 2023

keithc-ca commented May 2, 2023

thallium commented May 2, 2023

thallium commented May 2, 2023

fengxue-IS left a comment

Choose a reason for hiding this comment

babsingh commented May 3, 2023

thallium commented May 3, 2023

fengxue-IS commented May 8, 2023

keithc-ca commented May 8, 2023

fengxue-IS left a comment •

edited

Loading

thallium commented Apr 5, 2023 •

edited

Loading