Found fix for save/load hangs in Jpcsp 64-bit

[Itaru]

In the jpcsp.HLE.modules150.sceUtility class, add a 1ms delay inside the while loop at line 788 as follows:

// Wait for user selection.
while (!saveListSelected) {
if (!mainDisplay.isVisible()) {
break;
}
try {
Thread.sleep(1);
} catch (InterruptedException ex) {}
}

For some reason, this delay is needed when using the 64-bit JRE. It is also needed for the 32-bit JRE in server mode using the -server switch. It's strange that the 32-bit JRE in client mode runs perfectly fine without the delay though. I guess when using the 32-bit Java Hotspot Server VM or the 64-bit Java VM, the while loop goes too fast that the JVM somehow misses the break check. Obviously this is not a bug in the Jpcsp code but a strange quirk in the JVM. The added 1ms delay inside the while loop eliminates the problem.

[Hykem]

Ah! So that's the problem, JVM was executing too fast. Never thought this could happen...
Thanks a lot for tracing this down! Going to commit a patch right away.

[Itaru]

I did some more research on this strange quirk, and I'm convinced that it's actually a bug in the JIT compiler used in the 32-bit Server JVM and the 64-bit JVM. The 32-bit client JVM uses a different JIT compiler which does not have this bug, so that's why the original while loop works just fine on the 32-bit client JVM. Due to this bug, the JIT Server Compiler mis-compiles the while loop and creates an infinite loop instead. Only by adding another statement inside the while loop will the buggy JIT Server Compiler compile the while loop properly. Any Java statement will work and doesn't have to be the 1ms sleep delay that I used above.

If Sun/Oracle ever fixes the JIT Server Compiler, this workaround can be removed in the future. However, a more elegant way is to refactor/replace the while-polling code with wait/notify calls which doesn't need the workaround above. But the workaround works well, and perhaps it may be a good idea to increase the delay to keep the CPU from polling too quickly than necessary. I think a delay of 250ms is good so that it only polls 4 times a second, and I'm sure people won't mind the quarter second delay after saving/loading.

[Orphis]

Is the variable volatile ? If not, it should be to fix the problem I guess. It's not a bug, just a compiler feature.

[Itaru]

Orphis is right, it should be declared volatile. The problem is actually due to optimizations done by the JIT Server Compiler such as variable caching which causes the infinite while loop, so it's not a bug but inappropriate optimization done by the JIT Server Compiler. This kind of optimization is not done in the JIT Client Compiler so that's why it works in the 32-bit Client JVM without being declared volatile.

So, the proper fix should be to change the declaration of saveListSelected on line 329 to:

protected volatile boolean saveListSelected;

and maybe clean up the while loop on line 788 as follows:

// Wait for user selection.
while (!saveListSelected && mainDisplay.isVisible()) {
// Do nothing.
}

However, it seems that there's no need to check the state of saveListSelected at all in the while loop. We need to leave the loop regardless of the state of saveListSelected since the user can just close the dialog or click the Cancel button. We only need to check whether mainDisplay is visible and exit the loop when it's no longer visible. So, the proper loop should be:

// Wait for user selection.
while (mainDisplay.isVisible()) {
// Do nothing.
}

The above works in the 32-bit Client JVM, but hangs with the JIT Server Compiler due to inappropriate optimization. Adding a sleep delay inside the while loop is still a valid workaround here, and also saves the CPU from doing unnecessary constant polling.

So, I propose the following while loop:

// Wait for user selection.
while (mainDisplay.isVisible()) {
try {
Thread.sleep(250);
} catch (InterruptedException ex) {}
}

Comments and corrections are very welcome. I learn something new everyday by playing with Jpcsp.

[Hykem]

Indeed. Just went through a couple of docs and reached the same conclusions.
The variable saveListSelected was more of a sanity check and either declaring it as volatile or removing it are viable options, but the delay is still needed.

[kouta01]

Where can I find "jpcsp.HLE.modules150.sceUtility class"?

[gid15]

The fix proposed by Itaru is now available in r1961.
Sorry for the delay...

[kouta01]

Great, thanks!