RFR: 8342775: [Graal] java/util/concurrent/locks/Lock/OOMEInAQS.java fails OOME thrown from the UncaughtExceptionHandler

[Tom Rodriguez]

On Sat, 2 Nov 2024 11:02:13 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Deoptimization with escape analysis can fail when trying to rematerialize objects as described in JDK-8227309.  In this test this can happen in Xcomp mode in the framework of the test resulting in a test failure.  Making the number of threads non-final avoids scalar replacement and thus the OOM during deopt.
>
> test/jdk/java/util/concurrent/locks/Lock/OOMEInAQS.java line 48:
> 
>> 46:     // Intentionaly non-final to avoid EA of the threads array in main which can cause this test to
>> 47:     // fail in Xcomp mode.
>> 48:     static int NTHREADS = 2; // intentionally not a scalable test; > 2 is very slow
> 
> Hello Tom, I don't have the necessary knowledge of runtime compilers, so consider this as drive-by questions than a review. 
> 
> On its own, the `static final` construct appears to be the correct one for this field. Removing the `final` to address an escape analysis implementation detail appears odd.
> Do you know if the failure happens only in `-Xcomp` mode? Looking at JDK-8342775 it wasn't clear to me that was the case. If it's happening only in `-Xcomp` mode, perhaps due to additional work being done by the compiler threads (?) and the fact that this test intentional runs with a very low `-Xmx`, maybe we should just skip the test from `-Xcomp` mode instead of changing the field declaration? We have several such tests which we skip in `-Xcomp` mode by using:
> 
> @requires (vm.compMode != "Xcomp")

I think it's better to actually execute the test which is trying ensure that OOM is handled by library code in all configurations.  Using final is just a stylistic choice in the harness so removing it to allow to test to run in more configurations seems like better than avoiding those configurations.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21745#discussion_r1896086136