On Thu, 11 Jan 2024 13:21:16 GMT, Jaikiran Pai <[email protected]> wrote:
>> Alan Bateman has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Run in othervm mode
>
> test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java
> line 52:
>
>> 50: // need at least two carrier threads when main thread is a
>> virtual thread
>> 51: if (Thread.currentThread().isVirtual()) {
>> 52: VThreadRunner.ensureParallelism(2);
>
> Given that this changes the JVM level scheduler's parallelism, should we now
> use `/othervm` for these tests to prevent this change interfering with other
> tests?
The ensureParallelism here is to allow the test run on a container configured
with a single CPU. I don't think it will impact any other tests that run in the
same agent VM, at least I couldn't find any tests where it would create an
issue. But you are right that it would be better to restore it or run the test
in /othervm mode. The equivalent test in the loom repo does run in /othervm
mode as it has to launch with some VM options so we can keep it consistent with
that. Thanks.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1449134168
