On Tue, 20 Dec 2022 02:25:38 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:

>> There are a few nsk debugger tests that pin multiple virtual threads to 
>> carrier threads when synchronizing. Sometime the default number of carrier 
>> threads (which equals the number of CPUs) is not enough, and the test 
>> deadlocks because virtual threads start to wait forever for an available 
>> carrier thread. This PR fixes this problem by using the 
>> `jdk.virtualThreadScheduler.parallelism` property to change the default 
>> number of carrier threads. I believe the largest number of carrier threads 
>> any test needs is 11, so I chose 15 just to be safe.
>> 
>> I had initially tried to fix each individual test by using the test support 
>> in `VThreadRunner.setParallism()`. The advantage of this was limiting the 
>> scope of the change to just a few tests, and also being able to specify the 
>> exact number of needed carrier threads. The disadvantage was having to make 
>> quite a few changes to quite a few tests, plus I had one troublesome test 
>> that was still failing, I believe because I didn't fully understand how many 
>> carrier threads it needed. Just giving every test 15 carrier threads in the 
>> end was a lot easier.
>
> Chris Plummer has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Better comments.

In terms of the actual changes what you have seems fine. I wonder whether the 
tests themselves should be checking the parallelism value is large enough? At 
the moment there is a disconnect between the number of vthreads a test may 
create versus the setting of the parallelism level in the launcher/binder code.

-------------

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

Reply via email to