On Wed, 6 Nov 2024 17:38:59 GMT, Patricio Chilano Mateo 
<pchilanom...@openjdk.org> wrote:

>> Good work! I'll approve the GC related changes.
>> 
>> There are some simplifications I think can be done in the ObjectMonitor 
>> layer, but nothing that should go into this PR.
>> 
>> Similarly, (even if some of this is preexisting issues) I think that the way 
>> we describe the frames and the different frame transitions should be 
>> overhauled and made easier to understand. There are so many unnamed 
>> constants and adjustments which are spread out everywhere, which makes it 
>> hard to get an overview of exactly what happens and what interactions are 
>> related to what.  You and Dean did a good job at simplifying and adding 
>> comments in this PR. But I hope this can be improved in the fututre.
>> 
>> A small note on `_cont_fastpath`, as it is now also used for synchronised 
>> native method calls (native wrapper) maybe the comment should be updated to 
>> reflect this.
>> 
>> // the sp of the oldest known interpreted/call_stub frame inside the
>> // continuation that we know about
>
>> A small note on `_cont_fastpath`, as it is now also used for synchronised 
>> native method calls (native wrapper) maybe the comment should be updated to 
>> reflect this.
>> 
>> ```
>> // the sp of the oldest known interpreted/call_stub frame inside the
>> // continuation that we know about
>> ```
>>
> Updated comment.

> @pchilano `CancelTimerWithContention.java` test is failing on s390x with 
> Timeout Error. One weird thing is that it only fails when I run whole tier1 
> test suite. But when ran independently test passes.
> 
> One thing I would like to mention is that I ran test by **disabling** 
> VMContinuations, as Vthreads are not yet supported by s390x.

We added this test to provoke contention on the delay queues, lots of 
timed-Object.wait with notification before the timeout is reached. This code is 
not used when running with -XX:+UnlockExperimentalVMOptions 
-XX:-VMContinuation. In that execution mode, each virtual thread is backed by a 
platform/native thread and in this test it will ramp up 10_000 virtual threads. 
The output in your log suggests it gets to ~4700 threads before the jtreg 
timeout kicks in. It might be that when you run the test on its own that there 
is enough resources for the test to pass, but not enough resources (just too 
slow) when competing with other tests.

I think we can add `@requires vm.continuations` to this test. It's not useful 
to run with the alternative virtual thread implementation.

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

PR Comment: https://git.openjdk.org/jdk/pull/21565#issuecomment-2461756432

Reply via email to