Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Chris Plummer]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

Marked as reviewed by cjplummer (Reviewer).

-

PR Review: https://git.openjdk.org/jdk/pull/18174#pullrequestreview-1952843139

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Serguei Spitsyn]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

Looks okay. Is not this fix trivial? :)

-

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18174#pullrequestreview-1947544867

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Tue, 19 Mar 2024 20:41:56 GMT, Chris Plummer  wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   reverted failing tests.
>
> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Binder.java line 757:
> 
>> 755: }
>> 756: 
>> 757: if (classPath != null && !vmArgs.contains("-cp") && 
>> !vmArgs.contains("-classpath")) {
> 
> How does this change relate to using driver instead of othervm?

The classpaths are different for tests executed in agentvm and othervm.
As I see from logs jtreg uses CLASSPATH in othervm mode so debugee also 
inherits it. While agentvm load classed from 'test.class.path'.
It is not harm to use classpath 'test.class.path' in othervm mode.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18174#discussion_r1531188332

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

They also updated just to throw RuntimeException as other tests.

On Tue, Mar 19, 2024 at 2:20 PM Chris Plummer ***@***.***>
wrote:

> So no changes were made to any of these tests?
>
> —
> Reply to this email directly, view it on GitHub
> , or
> unsubscribe
> 
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-2008214565

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Chris Plummer]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

So no changes were made to any of these tests?

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-2008154319

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Tue, 19 Mar 2024 20:42:34 GMT, Chris Plummer  wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   reverted failing tests.
>
> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/ClassExclusionFilterTest.java 
> line 43:
> 
>> 41: if (result != 0) {
>> 42: throw new RuntimeException("TEST FAILED with result " + 
>> result);
>> 43: }
> 
> Why do we have tests under jdi/share?

It is not a test, but helper class with not the best name.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/18174#discussion_r1531123676

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

There are othervm/native tests. I am not sure if they could be just converted.
MonitorContendedEnteredRequest/addClassFilter_ClassName/TestDescription.java:64:
 * @run main/othervm/native
MonitorContendedEnteredRequest/MonitorContendedEnteredRequest001/TestDescription.java:54:
 * @run main/othervm/native
MonitorContendedEnteredRequest/MonitorContendedEnteredRequest002/TestDescription.java:54:
 * @run main/othervm/native
MonitorContendedEnteredRequest/addClassFilter_ReferenceType/TestDescription.java:62:
 * @run main/othervm/native
MonitorContendedEnteredRequest/addClassExclusionFilter/TestDescription.java:63: 
* @run main/othervm/native
MonitorContendedEnteredRequest/addInstanceFilter/TestDescription.java:63: * 
@run main/othervm/native
MonitorContendedEnteredRequest/addThreadFilter/TestDescription.java:63: * @run 
main/othervm/native
MonitorContendedEnterRequest/addClassFilter_ClassName/TestDescription.java:64: 
* @run main/othervm/native
MonitorContendedEnterRequest/addClassFilter_ReferenceType/TestDescription.java:62:
 * @run main/othervm/native
MonitorContendedEnterRequest/MonitorContendedEnterRequest001/TestDescription.java:54:
 * @run main/othervm/native
MonitorContendedEnterRequest/addClassExclusionFilter/TestDescription.java:63: * 
@run main/othervm/native
MonitorContendedEnterRequest/addInstanceFilter/TestDescription.java:63: * @run 
main/othervm/native
MonitorContendedEnterRequest/addThreadFilter/TestDescription.java:65: * @run 
main/othervm/native
MonitorContendedEnterRequest/MonitorContendedEnterRequest002/TestDescription.java:54:
 * @run main/othervm/native
MethodExitEvent/returnValue/returnValue004/returnValue004.java:50: * @run 
main/othervm/native
ThreadReference/forceEarlyReturn/forceEarlyReturn005/forceEarlyReturn005.java:54:
 * @run main/othervm/native
ThreadReference/forceEarlyReturn/forceEarlyReturn004/forceEarlyReturn004.java:42:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames004/ownedMonitorsAndFrames004.java:48:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames009/ownedMonitorsAndFrames009.java:47:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames002/ownedMonitorsAndFrames002.java:52:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames007/ownedMonitorsAndFrames007.java:54:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames008/TestDescription.java:57:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames006/ownedMonitorsAndFrames006.java:51:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames003/ownedMonitorsAndFrames003.java:51:
 * @run main/othervm/native
ThreadReference/ownedMonitorsAndFrames/ownedMonitorsAndFrames005/ownedMonitorsAndFrames005.java:48:
 * @run main/othervm/native
ReferenceType/instances/instances001/instances001.java:62: * @run 
main/othervm/native
ReferenceType/instances/instances004/TestDescription.java:49: * @run 
main/othervm/native
ReferenceType/instances/instances002/instances002.java:44: * @run 
main/othervm/native
ReferenceType/instances/instances005/instances005.java:44: * @run 
main/othervm/native
ReferenceType/instances/instances003/instances003.java:48: * @run 
main/othervm/native
ObjectReference/referringObjects/referringObjects003/referringObjects003.java:60:
 * @run main/othervm/native
ObjectReference/referringObjects/referringObjects002/referringObjects002.java:61:
 * @run main/othervm/native
ObjectReference/referringObjects/referringObjects001/referringObjects001.java:60:
 * @run main/othervm/native
ObjectReference/referringObjects/referringObjects004/referringObjects004.java:44:
 * @run main/othervm/native
stress/MonitorEvents/MonitorEvents001/TestDescription.java:62: * @run 
main/othervm/native
stress/MonitorEvents/MonitorEvents002/TestDescription.java:62: * @run 
main/othervm/native
stress/serial/ownedMonitorsAndFrames002/TestDescription.java:61: * @run 
main/othervm/native

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Chris Plummer]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

>  There were few tests which still execute using othervm mode. They require 
> some specific classpath/settings.

Which ones? They are hard to find among the 2000+ files changed.

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Binder.java line 757:

> 755: }
> 756: 
> 757: if (classPath != null && !vmArgs.contains("-cp") && 
> !vmArgs.contains("-classpath")) {

How does this change relate to using driver instead of othervm?

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/ClassExclusionFilterTest.java line 
43:

> 41: if (result != 0) {
> 42: throw new RuntimeException("TEST FAILED with result " + 
> result);
> 43: }

Why do we have tests under jdi/share?

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-2008097455
PR Review Comment: https://git.openjdk.org/jdk/pull/18174#discussion_r1531083329
PR Review Comment: https://git.openjdk.org/jdk/pull/18174#discussion_r1531084477

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[David Holmes]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

I agree there is a  more general issue of test interference in anything other 
than `othervm` mode but that is beside the point. It seems to me that going 
from othervm to driver mode for the debugger has some not insignificant risk 
when it comes to interference.

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-2002783668

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

That's a more generic question. 
The driver and main (default mode) actions might be executed in 2 modes: 
agentvm or othervm. So each driver (or main)  action might reuse jvm or start a 
new one each time. This is controlled by -agetnvm flag in jtreg. While othervm 
mode in tests is used for tests that couldn't be executed in agentvm mode (use 
some specific VM options). We don't force /othrevm option in tests because 
something might be broken and affect other tests execution. (Really it means 
that every test that doesn't start new process should be othrevm).
Each time when we run jtreg in agentvm we allow tests to reuse the same JVM and 
taking risks of possible incosistency of the future tests execution. So if test 
manages to break some part of JVM/JDK (it might be thread state, compile, gc, 
debugger,classoader, java.util or javac) we have a risk that next tests have 
jdk in inconsistent state. 

I think that the debugger might be treated as any other component.  If we need 
more isolation then it is needed to use othervm mode for jtreg. 

And using of agentvm/othervm mode for VM testing might be separate topic.

I can do more testing in different modes to see if we have any new failures 
after update.

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-1998190351

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[David Holmes]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

But if one test has a problem and messes up the debugger won't that kill the 
rest of the tests?

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-1990703702

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

That's a good question. 
Each test creates a new connector, starts new vm process with debugee and 
launch com.sun.jdi.VirtualMachine. I expect that it should be fine to run all 
of them in same vm. We are already use the same mode for com/sun/jdi jdk 
regression tests.
Moreover, it should be a bug if we can't run all these tests in the same vm 
process.

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-1988761928

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[David Holmes]

On Sat, 9 Mar 2024 05:27:43 GMT, Leonid Mesnik  wrote:

>> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
>> There is not need to start  debugger in the separate process for each test. 
>> Also, no need to run it with "-Xcomp" because the main goal is to test 
>> debugee behavior with different VM flags.
>> This fix updates tests to run debugger as driver to optimize execution time.
>> The fix also eliminates System.exit() which is not compatible with 
>> driver/agentvm mode and update shared classes to correctly add classpath 
>> when running debugee.
>> There were few tests which still execute using othervm mode. They require 
>> some specific classpath/settings. 
>> Tested by running all tests, with '-Xcomp' and virtual thread test factory.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   reverted failing tests.

> There is not need to start debugger in the separate process for each test.

I don't think I agree with that. Don't we want to have the debugger in a well 
known initial state for each test?

-

PR Comment: https://git.openjdk.org/jdk/pull/18174#issuecomment-1987638497

Re: RFR: 8327704: Update nsk/jdi tests to use driver instead of othervm [v3]

[Leonid Mesnik]

> vmtestbase nsk/jdi tests run 2 processes: debugger and debugee.
> There is not need to start  debugger in the separate process for each test. 
> Also, no need to run it with "-Xcomp" because the main goal is to test 
> debugee behavior with different VM flags.
> This fix updates tests to run debugger as driver to optimize execution time.
> The fix also eliminates System.exit() which is not compatible with 
> driver/agentvm mode and update shared classes to correctly add classpath when 
> running debugee.
> There were few tests which still execute using othervm mode. They require 
> some specific classpath/settings. 
> Tested by running all tests, with '-Xcomp' and virtual thread test factory.

Leonid Mesnik has updated the pull request incrementally with one additional 
commit since the last revision:

  reverted failing tests.

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18174/files
  - new: https://git.openjdk.org/jdk/pull/18174/files/ce7bbb56..01258fb1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=18174=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=18174=01-02

  Stats: 9 lines in 5 files changed: 0 ins; 0 del; 9 mod
  Patch: https://git.openjdk.org/jdk/pull/18174.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18174/head:pull/18174

PR: https://git.openjdk.org/jdk/pull/18174