Transplant jtreg test YieldQueuing.java from Loom

[quadhier]

This patch mainly transplants jtreg test test/jdk/java/lang/Thread/virtual/YieldQueuing.java of Loom to KonaFiber-8.

Code changes

Changes for successful compilation:

1. remove @requires vm.continuations in jtreg header since this is always satisfied in KonaFiber
2. use testng instead of junit in jtreg header and import statements
3. substitute var keywords with specific class names and List.of() call with Arrays.asList() call
4. remove Thread.onSpinWait() calls since it is just an optimization hint (please refer to JEP 285) that we have not implemented yet

Changes for successful test:

1. add public access modifiers to the test class and methods for unit test to work
2. remove -Djdk.virtualThreadScheduler.maxPoolSize=1 vm option since it doesn't take effect in KonaFiber-8
3. in testYieldWithEmptyLocalQueue(), use a scheduler created by Executors.newSingleThreadExecutor() instead of the default ForkJoinPool since this test is intended to execute with only one carrier thread
4. in testYieldWithNonEmptyLocalQueue(), use a scheduler created by Executors.newSingleThreadExecutor() as well and update test oracle according to ThreadPoolExecutor's (which is the scheduler here) implementation on KonaFiber-8

Reason behind major changes of testYieldWithEmptyLocalQueue()

Why use a scheduler created by Executors.newSingleThreadExecutor() instead of the default one?

According to the Java option -Djdk.virtualThreadScheduler.maxPoolSize=1 set in jtreg header, this test is indicated to be executed using a scheduler with only one carrier thread.

The default scheduler is ForkJoinPool but KonaFiber-8 doesn't have this option to set a fixed number of carrier threads for ForkJoinPool, Executors.newSingleThreadExecutor() is used instead to create such a scheduler.

Reason behind major changes of testYieldWithNonEmptyLocalQueue()

Why change the test oracle from "A", "B", "A", "B", "C" to "A", "B", "C", "A", "B"?

Original version of this test case uses ForkJoinPool as a scheduler but now ThreadPoolExecutor is used instead. These two schedulers have different orders of handling tasks submitted from their worker thread.

In this test, roughly speaking, 5 tasks are submitted to the scheduler. They are (in the order of submission time from earliest to latest):

1. 3 virtual thread start() calls to submit continuations of thread virtual threads
2. Thread.unpark() called in VirtualThread-B to submit the continuation of VirtualThreadd-A
3. Thread.yield() called in VirtualThread-B to submit the continuation of VirtualThread-B

For ForkJoinPool, since 2-nd and 3-rd submissions happen in its worker thread, and VirtualThread-C is not executed yet, they are executed before VirtualThread-C. (Please refer to ForkJoinPool's implementation.) And the last three output should be "A", "B", "C".

For ThreadPoolExecutor, the execution order is exactly same as the submission order, so the VirtualThread-C is executed before continuations of VirtualThread-A and VirtualThread-B. The last three output should be "C", "A", "B".

With different schedulers come different testing results.

Detailed diff output

The following diff output shows all the changes (you could also see it here in GitHub):

diff --git a/jdk/test/java/lang/VirtualThread/YieldQueuing.java b/jdk/test/java/lang/VirtualThread/YieldQueuing.java
index edb0b6e9f7..78134b2184 100644
--- a/jdk/test/java/lang/VirtualThread/YieldQueuing.java
+++ b/jdk/test/java/lang/VirtualThread/YieldQueuing.java
@@ -21,37 +21,43 @@
  * questions.
  */

-/**
+/*
  * @test
+ * @run testng/othervm YieldQueuing
  * @summary Test Thread.yield submits the virtual thread task to the expected queue
- * @requires vm.continuations
- * @run junit/othervm -Djdk.virtualThreadScheduler.maxPoolSize=1 YieldQueuing
  */

+import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.LockSupport;

-import org.junit.jupiter.api.Test;
-import static org.junit.jupiter.api.Assertions.*;
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;

-class YieldQueuing {
+public class YieldQueuing {

     /**
      * Test Thread.yield submits the task for the current virtual thread to a scheduler
      * submission queue when there are no tasks in the local queue.
      */
     @Test
-    void testYieldWithEmptyLocalQueue() throws Exception {
-        var list = new CopyOnWriteArrayList<String>();
+    public void testYieldWithEmptyLocalQueue() throws Exception {
+        List<String> list = new CopyOnWriteArrayList<>();
+
+        AtomicBoolean threadsStarted = new AtomicBoolean();

-        var threadsStarted = new AtomicBoolean();
+        Executor executor = Executors.newSingleThreadExecutor();

-        var threadA = Thread.ofVirtual().unstarted(() -> {
+        Thread threadA = Thread.ofVirtual().scheduler(executor).unstarted(() -> {
             // pin thread until task for B is in submission queue
             while (!threadsStarted.get()) {
-                Thread.onSpinWait();
+                // Just an optimization hint and could be safely
+                // removed till we implement it
+                // Thread.onSpinWait();
             }

             list.add("A");
@@ -59,7 +65,7 @@ class YieldQueuing {
             list.add("A");
         });

-        var threadB = Thread.ofVirtual().unstarted(() -> {
+        Thread threadB = Thread.ofVirtual().scheduler(executor).unstarted(() -> {
             list.add("B");
         });

@@ -73,7 +79,7 @@ class YieldQueuing {
         // wait for result
         threadA.join();
         threadB.join();
-        assertEquals(list, List.of("A", "B", "A"));
+        assertEquals(list, Arrays.asList("A", "B", "A"));
     }

     /**
@@ -81,15 +87,18 @@ class YieldQueuing {
      * queue when there are tasks in the local queue.
      */
     @Test
-    void testYieldWithNonEmptyLocalQueue() throws Exception {
-        var list = new CopyOnWriteArrayList<String>();
+    public void testYieldWithNonEmptyLocalQueue() throws Exception {
+        List<String> list = new CopyOnWriteArrayList<>();
+
+        AtomicBoolean threadsStarted = new AtomicBoolean();

-        var threadsStarted = new AtomicBoolean();
+        Executor executor = Executors.newSingleThreadExecutor();

-        var threadA = Thread.ofVirtual().unstarted(() -> {
+        Thread threadA = Thread.ofVirtual().scheduler(executor).unstarted(() -> {
             // pin thread until tasks for B and C are in submission queue
             while (!threadsStarted.get()) {
-                Thread.onSpinWait();
+                // An optimization hint, not implemented yet
+                // Thread.onSpinWait();
             }

             list.add("A");
@@ -97,14 +106,14 @@ class YieldQueuing {
             list.add("A");
         });

-        var threadB = Thread.ofVirtual().unstarted(() -> {
+        Thread threadB = Thread.ofVirtual().scheduler(executor).unstarted(() -> {
             list.add("B");
             LockSupport.unpark(threadA);  // push task for A to local queue
             Thread.yield();               // push task for B to local queue, A should run
             list.add("B");
         });

-        var threadC = Thread.ofVirtual().unstarted(() -> {
+        Thread threadC = Thread.ofVirtual().scheduler(executor).unstarted(() -> {
             list.add("C");
         });

@@ -120,6 +129,6 @@ class YieldQueuing {
         threadA.join();
         threadB.join();
         threadC.join();
-        assertEquals(list, List.of("A", "B", "A", "B", "C"));
+        assertEquals(list, Arrays.asList("A", "B", "C", "A", "B"));
     }
 }

Progress

• [x] successful compilation
• [x] adapt testYieldWithEmptyLocalQueue()
• [x] adapt testYieldWithNonEmptyLocalQueue()
• [x] detailed analysis

Path consideration

BTW, just a minor problem: shall I put the YieldQueuing.java file in the path jdk/test/java/lang/VirtualThread or jdk/test/java/lang/VirtualThread/loom? I believe the former is the corresponding path of what this test used to be in Loom but I see most of the transplanted tests in the latter. :P

[johnshajiang]

@quadhier Thanks for this effort!

Did this test pass on your testing box? It just failed with my macOS and Kona JDK (Fiber) 8u362.

test YieldQueuing.testYieldWithEmptyLocalQueue(): failure
java.lang.AssertionError: Lists differ at element [0]: A != B expected [A] but found [B]
...
test YieldQueuing.testYieldWithNonEmptyLocalQueue(): failure
java.lang.AssertionError: Lists differ at element [0]: A != C expected [A] but found [C]
...

[quadhier]

Did this test pass on your testing box? It just failed with my macOS and Kona JDK (Fiber) 8u362.

Thanks for your reviews @johnshajiang ! After adding public modifiers to the classes and methods under test, this test also fails on my environment. (It seems that the test methods didn’t get executed before that). I’ll look into it.

[miao-zheng]

可以列一下，合入Kona Fiber的测试用例和loom原始测试用例的差异（类似git diff就行，看起来更清楚一点）

[quadhier]

可以列一下，合入Kona Fiber的测试用例和loom原始测试用例的差异（类似git diff就行，看起来更清楚一点）

好的，谢谢你的建议 @miao-zheng ，已更新。鉴于当前还有测试用例没有调通，我已将PR状态改为Draft。

[quadhier]

Hi, @johnshajiang and @miao-zheng , I have updated the test case and comments. Now it could pass on my linux x64 version of KonaFiber-8 (which is a fresh build from the latest source code of KonaFiber-8) and my macos version of x64 KonaFiber-8 (which is the latest release 1.8.0_362_fiber). I would appreciate it if you could try this jtreg test again whenever you have time. And I will add detailed analysis of these changes later. Thanks in advance!

[johnshajiang]

@quadhier The updated test just passed on my macOS x64 with KonaFIber 8u362.

[quadhier]

Hi @johnshajiang and @miao-zheng , thanks for your reviews! I have addressed the review comments and added detailed explainations for the changes.