On Mon, 7 Oct 2024 22:03:36 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
> This fixes a problem in the VTMS (Virtual Thread Mount State) transition
> frames hiding mechanism.
> Please, see a fix description in the first comment.
>
> Testing:
> - Verified with new test `vthread/CheckHiddenFrames`
> - Mach5 tiers 1-6 are passed
Changes requested by lmesnik (Reviewer).
src/hotspot/share/prims/jvmtiEnvBase.cpp line 691:
> 689:
> 690: if (is_virtual || jt->is_in_VTMS_transition()) { // filter out pure
> continuations
> 691: jvf = check_and_skip_hidden_frames(jt->is_in_VTMS_transition(), jvf);
Wouldn't be easier to split method `check_and_skip_hidden_frames` to
skip_yeilds and skip_transition frames?
BTW Also it is unclear shouldn't the `@Hidden` methods be skipped from all
jvmti frame streams?
src/hotspot/share/prims/jvmtiEnvBase.cpp line 2009:
> 2007: bool is_virtual = java_lang_VirtualThread::is_instance(thread_oop);
> 2008:
> 2009: // Target can be virtual or platform thread.
Can you please explain in comment why is it needed to disable all threads for
platform target thread.
test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java
line 25:
> 23:
> 24: /*
> 25: * @test id=virtual
Having 'id=virtual' not needed and might confuse people. They expect to have
other test variations for platform.
test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java
line 32:
> 30:
> 31: public class CheckHiddenFrames {
> 32: private static final String AGENT_LIB = "CheckHiddenFrames";
It is not used?
test/hotspot/jtreg/serviceability/jvmti/vthread/CheckHiddenFrames/CheckHiddenFrames.java
line 43:
> 41:
> 42: public static void main(String[] args) throws Exception {
> 43: Thread thread =
> Thread.ofVirtual().unstarted(CheckHiddenFrames::test);
You can use
startVirtualThread
to save one line.
-------------
PR Review: https://git.openjdk.org/jdk/pull/21397#pullrequestreview-2353060666
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1791023030
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1791008060
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790967726
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790968113
PR Review Comment: https://git.openjdk.org/jdk/pull/21397#discussion_r1790966876
