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

Reply via email to