On Tue, 2 Jan 2024 15:22:05 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> A lot of test changes have accumulated in the loom repo, this includes both >> new tests and updates to existing tests. Some of these updates can be >> brought to the main line. This update brings over: >> >> - The existing tests for pinning use synchronized blocks. In preparation for >> changes to allow carrier thread be released when a virtual thread parks >> holding a monitor or blocks on monitorenter, these tests are changed to pin >> by having a native frame on the stack. This part includes test >> infrastructure to make it easy to add more tests that do operations while >> pinned. The tests still test what they were originally created to test of >> course. >> >> - The test for the JFR jdk.VirtualThreadPinned event is refactored to allow >> for additional cases where the event may be reported. >> >> - ThreadAPI is expanded to cover test for uncaught exception handling. >> >> - GetStackTraceWhenRunnable is refactored to not use a Selector, otherwise >> this test will be invalidated when blocking selection operations release the >> carrier. >> >> - StressStackOverflow is dialed down to run for 1m instead of 2mins. >> >> - The use of CountDownLatch in a number of tests that poll thread state has >> been dropped to keep the tests as simple as possible. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains four additional > commits since the last revision: > > - Revert changes to TracePinnedThreads.java > - Sync up from loom repo > - Merge > - Initial commit Overall these changes look OK to me. I haven't paid much attention to the jvmti test change, but I've looked at others and they seem fine. The `StressStackOverflow` test has its duration reduced by half which appears intentional and then there's a new test library helper class to allow pinning tasks in tests. Just one question - doesn't the use of a new native code in the test library (the `libVThreadPinner.c`) require any build changes (I'm not too familiar with that part)? ------------- Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17136#pullrequestreview-1801861084