On Thu, 11 Jan 2024 13:49:14 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> This test was recently dialled down via JDK-8323002 but it still makes slow
>> progress on some test machines, esp. macox-x64-debug builds. The issue is
>> that the sampling of the target thread is skewed towards the unmounted case
>> so the target thread is disabled from being scheduled and doesn't make
>> progress. The test is re-worked to use a barrier so that the main thread and
>> target virtual thread run in lock step. This allows the virtual thread to
>> make progress at each iteration and also increases the chances of sampling
>> the stack trace at around the time that the target thread transitions from
>> being unmounted (due to the Thread.yield) and parking while pinned.
>
> test/jdk/java/lang/Thread/virtual/stress/GetStackTraceALotWhenPinned.java
> line 65:
>
>> 63: for (int i = 0; i < iterations; i++) {
>> 64: // wait for main thread to arrive
>> 65: barrier.await();
>
> Given the goal of this test, which is "Stress test Thread.getStackTrace on a
> virtual thread that is pinned", shouldn't this `barrier.await()` be a few
> lines below, inside the `synchronized (GetStackTraceALotWhenPinned.class)`
> block, just before it parks itself? That way, the main thread when it too
> reaches the barrier and when it next issues a getStackTrace() call, the
> chances of the other thread being pinned would be high(er)?
It's in the right place. This is a stress test to bash on Thread::getStackTrace
where the target virtual thread is transitioning from being unmounted to
parked-while-pinned. There are intermittent states and the stack walk will
depend on whether the target is mounted or not. There are other tests that
exercise Thread::getStackTrace on pinned threads, specifically
ThreadAPI.testGetStackTrace6 and ThreadAPI.testGetStackTrace7, but different to
the stress testing that is done here.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17353#discussion_r1448924715
