Run Ahead stopped working for some cores

[Tatsuya79]

@Dwedit

Description

At least FBA and PCE CD can't use the run ahead feature any more. They both show "Failed to Load Sate. Run Ahead has been disabled.".

Steps to reproduce the bug

1. Have Run Ahead enabled, also "use 2nd core instance".
2. Start a game in current FBA or a CD game in Beetle SGX.
3. Run ahead should be disabled with a message.

Bisect Results

Started with https://github.com/libretro/RetroArch/commit/6154a843e0b8c575e594df2cfd9fa4b7ae6bcab1.

Observations

FBA can still use run ahead if you start it in 1 core mode. Starting with 2 instances won't allow you to change this (it stays force disabled). Defaulting back to 1 core was perhaps happening silently before and not any more on the new code. (Any way to get FBA to work in 2 instances?)

This doesn't work for PCE CD, one way I could get it to work on current RA build was by commenting out the content of runahead_error(void).

Version/Commit

Any RetroArch version built after the mentioned commit, current FBA and Beetle SGX cores.

Environment information

• OS: Win7 x64
• Compiler: MSYS2/Mingw64

[inactive123]

@Tatsuya79 Has this issue been fixed since?

[Tatsuya79]

Nope, it's still there.

It could be that for PCE CD a load state fails to load during the booting process and runahead disables itself.

For Fba I'm not sure why only 2nd instance mode fails like described above. I noticed that in QT UI, FBA has issues with paths and doesn't correct / to \ on windows for operations in the file browser mode (loading a rom from there fails, scanning with database checking lpl playlists paths are wrong).

[inactive123]

@Dwedit is this something you are aware of?

[FrozenFish24]

Hello.

Not sure if this is the best way to go about it, but adding a call to secondary_core.retro_serialize_size() in secondary_core_create() seems to fix this issue and get second instance mode working with FBAlpha.

Can confirm it avoids the audio problems in Armed Police Batrider (also Battle Garegga) as discussed here.

[Dwedit]

That is very strange. Adding in a retro_serialize_size call wouldn't hurt, but it's still a bug that needs fixing without weird workarounds.

[Tatsuya79]

Indeed, I added secondary_core.retro_serialize_size(); after line 274 (not sure if that's the best spot) and now fba works fine in 2 instances mode. CD-ROM games work too in Beetle-SGX with that.

I'll have to test later if fba slows down after a while or not, it's doing that in single instance mode. edit: finished some games without noticing that bug happening.

Nice find!

[Tatsuya79]

I noticed the performance loss over time playing Battle Garegga. Near the end of that (long) game using an auto-fire will make the sound skip. At the second loop the sound crackles and the game slows down just moving the dpad a bit too much.

So it's not as fast as in single instance but it can still happen.

[Tatsuya79]

Should we add the retro_serialize_size call for now?

Something else I noticed about run_ahead disabling itself (with retro_serialize_size call added):

-I tried to run a game in MAME with 2 instances (there's serialization for it now, but it's not working well for run ahead, crawling to an halt in 1 instance mode) -> got a message "Failed to Load Sate. Run Ahead has been disabled.", then I can not try 1 instance or anything again -I close MAME -I launch FBA -> run ahead is still forced disabled, I couldn't find a way to start it again without completely closing and restarting RA

[FrozenFish24]

I looked into this some more. The issue seems to be that the FBA core stores an internal variable state_size that it checks is equal to the size argument it receives from the frontend when it calls retro_serialize() and retro_unserialize().

The only places state_size is set are retro_load_game_common() where it is initialized to 0, and retro_serialize_size(), which calculates the proper value (via setting a callback to burn_dummy_state_cb()), but this is only ever done for the primary core, meaning state_size in the secondary core is always 0 and always fails the comparison against size.

Comparing against Snes9x it appears the reason other cores don't suffer the same problems is they re-compute this value within retro_serialize()/retro_unserialize() or simply trust the size argument they are passed.

So the possible fixes seem to be, modify the affected cores so they trust size. Or call retro_serialize_size() on the secondary core when it is created.

[Dwedit]

From libretro.h:

/* Returns the amount of data the implementation requires to serialize
 * internal state (save states).
 * Between calls to retro_load_game() and retro_unload_game(), the
 * returned size is never allowed to be larger than a previous returned
 * value, to ensure that the frontend can allocate a save state buffer once.
 */
RETRO_API size_t retro_serialize_size(void);

/* Serializes internal state. If failed, or size is lower than
 * retro_serialize_size(), it should return false, true otherwise. */
RETRO_API bool retro_serialize(void *data, size_t size);
RETRO_API bool retro_unserialize(const void *data, size_t size);

I don't see anything in the API documentation that says you must call retro_serialize_size before calling retro_unserialize, but in practice, RetroArch is calling core_serialize_size before doing anything with savestates. I'd say fix both. FBA should not depend on retro_serialize_size being called first, and RetroArch runahead should be made consistent with the other ways it saves state.

[FrozenFish24]

@Dwedit Sounds good, @barbudreadmon has fixed the issue in the FBAlpha core now. Save states function correctly even if retro_serialize_size has not been called.

@Tatsuya79 I can't replicate the issues you were having with Beetle-SuperGrafx, testing with Super Darius II both single and secondary core modes function properly. Perhaps this was already fixed at some point?

[Tatsuya79]

Indeed I'm just testing that again too and it seems to work fine now with beetle-sgx. I'm not sure what happened...

[Tatsuya79]

Then there's MAME. I tried the most recent one and mame2003-plus, they both disable 2nd instance. If you start them in 1 instance (you need to do it before run ahead gets disabled in a RA session) it's working but slows down really fast.

PR that added the serialization in main MAME.

[Tatsuya79]

Ah it's failing in Beetle-SGX with Ginga Fukei Densetsu Sapphire (J) so that's perhaps about arcade card games. Yes, same thing with Kabuki Itouryodan.

[FrozenFish24]

@Tatsuya79 I've been able to replicate your issue with Beetle-SGX. It seems to be the transition from BIOS to game that's breaking runahead. If you wait until you're in-game before enabling it everything works correctly.

Also interestingly, if you then restart (without closing content) runahead survives going BIOS to in-game.

[Tatsuya79]

Mame 2003 Plus is working somewhat with run ahead 2nd instance now, most probably thanks to Dwedit fix in RA. But testing it in Thunder Force AC you can see hiccups when you push a button that doesn't seem to be perf limitation related.

You need to remove 1 additional frame of lag with 2nd instance too, that's strange.

[Tatsuya79]

Something strange I'm experiencing with FBA: testing Batrider, sometimes I have 4 frames of lag, sometimes 3. It's different if I start with run_ahead_enabled = "true" already set, or if I activate it later in-game. Changing context from window/fullscreen can have a 1 frame difference too, or enabling/disabling run ahead.

[Dwedit]

I had made a "Debug Runahead" fork of Retroarch. The purpose of the fork is to catch save state inconsistencies. It's available from /dwedit/retroarch/tree/debug-runahead-2, but pretty much needs to be run in a debugger to be of any use.

It has four tests it can perform once Runahead Debug Mode is turned on (enable advanced options to see this setting)

• (Secondary core off, Runahead 0) See if Run, Save State, Load State, Save State produces the same two savestates or not
• (Secondary core off, Runahead 1) See if Run, Save State, Run, Load State, Save State produces the same two savestates or not
• (Secondary core on, Runahead 0) Compare Run, Save State, Load State against another instance which never loads state (this catches a lot of issues)
• (Secondary core on, Runahead 1) Compare Run, Save State, Run, Load State against another instance which never loads state

Then after creating two save states, it compares them byte-by-byte for any differences. Then once it finds a difference, it triggers a debug breakpoint.

Most emulators fail the tests, but a few pass successfully. QuickNES passes without any hitches at all, and Snes9x passes most of the time if you modify the emulator to never skip frames.

It would be a huge mountain of work to get cores to pass all the tests. But if a core can pass them, then it will have very consistent save states. I had contributed a few fixes for Snes9x that fixed a few inconsistencies that I caught.

Edit: Just thought of another idea, validate the graphics and sounds as well.

[FrozenFish24]

@Tatsuya79 I beleive I've found the cause of runahead failing in Beetle-SGX and have opened an issue. In regards to Batrider (and other Raising titles), they have variable latency on vanilla FBA and vanilla MAME too, so I think that's just how they work.

[Tatsuya79]

Ah ok, in fact I thought about that and forgot to test then... :sweat_smile:

[ghost]

Trick.

size_t retro_serialize_size()
{
  size_t size = stella.getStateSize();

  int runahead = -1;
  if(environ_cb(RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE, &runahead))
  {
    // future expanding size
    if(runahead & 4)
      size += 0x10000;
  }

  return size;
}

Use largest future size possible. PCECD Arcade needs 2.4+ MB. This is memory only.

Actual save / load to disk uses correct size.