On Tue, 11 Feb 2025 08:38:48 GMT, SendaoYan <s...@openjdk.org> wrote:
>> H all,
>>
>> This PR add `/native` keyword in the test header for virtual thread tests.
>> The `/native` keyword will make run the related tests by jtreg standalone
>> more friendly.
>>
>> I runed all the tests without -nativepath argument and find the fail tests.
>> This will find all the virtual thread tests which missing '/native' flag.
>>
>> 1. java/lang/management/ThreadMXBean/VirtualThreads.java#default
>>
>> VirtualThreads.java:223 call "invoker()" in
>> test/lib/jdk/test/lib/thread/VThreadPinner.java file, which will call the
>> native function "call".
>> java/lang/management/ThreadMXBean/VirtualThreads.java#no-vmcontinuations run
>> this test with VM option "-XX:-VMContinuations" and
>> `assumeTrue(VThreadScheduler.supportsCustomScheduler(), "No support for
>> custom schedulers");` will make junit skip the
>> 'testGetThreadInfoCarrierThread' unit test, so
>> 'VirtualThreads.java#no-vmcontinuations' do not need '/native' keywork.
>>
>> 2. java/lang/Thread/virtual/stress/PinALot.java
>>
>> PinALot.java:58 call "invoker()" in
>> test/lib/jdk/test/lib/thread/VThreadPinner.java file, which will call the
>> native function "call".
>>
>> 3. java/lang/Thread/virtual/SynchronizedNative.java
>>
>> Call System.loadLibrary("SynchronizedNative") at line 85.
>>
>> 4. Tests java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java,
>> java/lang/Thread/virtual/ThreadAPI.java,
>> java/lang/Thread/virtual/RetryMonitorEnterWhenPinned.java,
>> java/lang/Thread/virtual/MonitorWaitNotify.java,
>> java/lang/Thread/virtual/MonitorPinnedEvents.java,
>> java/lang/Thread/virtual/MonitorEnterExit.java and other tests are similar
>> to java/lang/Thread/virtual/stress/PinALot.java
>>
>>
>> - Why we need the '/native' keywork?
>>
>> I usually run the tests through jtreg directively, rather than run the tests
>> through make test. If I run the test which load the native shared library
>> files and the test missing '/native' keyword but I forgot to pass the
>> '-nativepath' argument to jtreg, or pass the incorrect '-nativepath'
>> argument to jtreg, sometimes it will report timeouted after a long time,
>> such as java/lang/Thread/virtual/JfrEvents.java. If we add the missing
>> '/native' keywork, jtreg will report 'Error. Use -nativepath to specify the
>> location of native code' right away. So add the missing '/native' keyword
>> will make run the tests more friendly.
>>
>> Change has been verified locally, test-fix, no risk.
>
> SendaoYan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add /native for test/jdk/java/lang/Thread/virtual/ThreadPollOnYield.java
> test/jdk/java/lang/Thread/virtual/Starvation.java
test/jdk/java/lang/Thread/virtual/Starvation.java line 28:
> 26: * @library /test/lib
> 27: * @bug 8345294
> 28: * @run main/othervm/timeout=200/native
> --enable-native-access=ALL-UNNAMED Starvation 100000
Did you mean to add /timeout=200 to this test?
test/jdk/java/lang/Thread/virtual/ThreadPollOnYield.java line 39:
> 37: * @requires vm.continuations & vm.compMode != "Xcomp"
> 38: * @library /test/lib
> 39: * @run junit/othervm/native --enable-native-access=ALL-UNNAMED -Xcomp
> -XX:-TieredCompilation -XX:CompileCommand=inline,*::yield*
> -XX:CompileCommand=inline,*::*Yield ThreadPollOnYield
Would you mind splitting this line (jtreg is okay with that) as it's nearly 190
chars now so getting too long.
test/jdk/java/lang/Thread/virtual/ThreadPollOnYield.java line 69:
> 67: if (flag != true) {
> 68: throw new RuntimeException("flag = " + flag);
> 69: }
What are these other changes about?
test/jdk/java/lang/management/ThreadMXBean/VirtualThreads.java line 30:
> 28: * @modules java.base/java.lang:+open java.management
> 29: * @library /test/lib
> 30: * @run junit/othervm/native --enable-native-access=ALL-UNNAMED
> VirtualThreads
Can you check if the end copyright date needs to be updated on this one.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23550#discussion_r1950436579
PR Review Comment: https://git.openjdk.org/jdk/pull/23550#discussion_r1950439893
PR Review Comment: https://git.openjdk.org/jdk/pull/23550#discussion_r1950438562
PR Review Comment: https://git.openjdk.org/jdk/pull/23550#discussion_r1950440627
