[crac] RFR: 8364156: [CRaC] Recompile methods decompiled during or shortly after C/R [v2]

[Radim Vansa]

On Mon, 28 Jul 2025 14:59:16 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> test/jdk/jdk/crac/recompiler/NaturalDecompilationTest.java line 120:
>> 
>>> 118:             }
>>> 119:             try {
>>> 120:                 Thread.sleep(500); // Time to compile
>> 
>> I think this has some potential for false positives in the testsuite. I guess there is no API in whitebox to wait until this is observed? Alternatively watching the output in `test()` and signaling the process somehow...
>
> I am not sure what kind of false positives can there be, as I see it, in worst case we will just wait longer then necessary.
> 
> WhiteBox API indeed does not provide a way to wait for a compilation. I could probably indeed make the parent process read `PrintCompilation` from stdout and print to stdin but I am not sure this is needed as it is more complicated.

My bad, I misread this for failing when the method is not compiled but this is just a busy loop (and it won't fail if the first 2000 executions won't trigger recompilation). All is good, then.

>> test/jdk/jdk/crac/recompiler/RecompilationDelayTest.java line 115:
>> 
>>> 113: 
>>> 114:         if (delayMs > 0) {
>>> 115:             assertEquals(
>> 
>> 10 seconds sounds like enough, but ideally I would have tests as independent to timing as possible. Are there any timestamps in the compilation log so rather than checking the current time we could just read the logs and run assertions on those?
>
> I don't think this particular assert can be removed even if we would read logs.
> 
> It asserts that the machine had time to deoptimize the method before the delay expired. Because if we do it after the delay expires it is expected that there will be no recompilation. Reading logs won't free us from this assert.
> 
> The only thing I can come up with is to implement some mechanism to relaunch the test (or just the restore part if I implement the request above to make recompilation delay restore-settable) with a higher delay when the test fails this way.

So this is actually triggering the deoptimization (yes, the method sounds like that, but the error message sounds like it's just checking some statistics)? If the purpose of the test is to check that recompilations won't happen too soon, why is there a difference in the time when you trigger the deoptimization? (for delayMs == 0 this must be in the afterRestore, but why wouldn't you trigger it there even for delayMs > 0?)

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

PR Review Comment: https://git.openjdk.org/crac/pull/251#discussion_r2237034394
PR Review Comment: https://git.openjdk.org/crac/pull/251#discussion_r2237818802