[crac] RFR: 8328344: [CRaC] Avoid error when running with -XX:+PerfDisableSharedMem [v2]

[Radim Vansa]

On Tue, 19 Mar 2024 19:39:10 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> test/jdk/jdk/crac/PerfMemoryRestoreTest.java line 69:
>> 
>>> 67:         while (!perfdata.toFile().exists()) {
>>> 68:             if (System.nanoTime() - start > TimeUnit.SECONDS.toNanos(10)) {
>>> 69:                 if (perfDisableSharedMem) {
>> 
>> The file won't ever appear, so this would busy loop for the whole 10 seconds, extending the testsuite. Could you avoid the loop completely, or in case of `perfDisableSharedMem == true` reduce the delay to, say, 100 ms?
>
> If we expect that the file will appear we give it up to ten seconds to appear. If we want to be sure that it doesn't appear, we should then wait at least as long, right? Now for the positive case we expect that the file will appear quickly, so sleeping for a small amount of time like 10ms is fine. For the negative case, we already know that we will have to wait for the full ten seconds anyway, so we can just do that and not loop at all by sleeping 10 seconds.
> 
> I'll change the sleep time accordingly.

As you say, in normal case the file should appear very quickly. The 10 second duration is there only to avoid false positives. Yes, if you want to assert that the file does not appear you would have to wait much longer than what is the 'expected' time.

I hope you don't take this as too much nitpicking; I am trying to balance the duration of the testsuite (esp. for manual runs) - not only in this case but also setting a precedent in style - vs. the risk of false negatives. I hate false positives as these reduce the credibility of the testsuite, but in my view asserting the non-existence of the file is not important to justify stalling of tests - I expect that if the VM option is completely ignored the tests should blow somewhere else.

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

PR Review Comment: https://git.openjdk.org/crac/pull/153#discussion_r1531642454