beetle-psx-libretro icon indicating copy to clipboard operation
beetle-psx-libretro copied to clipboard

Lightning+Lightrec updates and other optimizations

Open ZachCook opened this issue 2 years ago • 15 comments

Split out cpu_lightrec.cpp from cpu.cpp instead of being mashed together.

Added hugepage, codebuffer, mmap at 0x0, and SP GP hit RAM optimizations and fallbacks, see commits for more details.

Use lightrec's GTE registers directly when not using beetle interpreter (while maintaining backwards compatibility with savestates).

Removed the Cycle Timing Check dynarec option.

ZachCook avatar Sep 25 '22 02:09 ZachCook

This pull request introduces 1 alert when merging 8ae372aeee9e246dcf1fc8f0e22678716c2f0676 into bd6b9ef3049fe3f70a18ee6f752a935ae83c2f2b - view on LGTM.com

new alerts:

  • 1 for Missing return statement

lgtm-com[bot] avatar Sep 25 '22 02:09 lgtm-com[bot]

After testing this with a few games on my Windows 11 system, I have noticed a few Runahead-related issues:

  • As reported on Discord, enabling Runahead in RetroArch with either single instance or secondary instance mode and having the dynarec active seems to be triggering unpredictable crashes with this PR, at several spots. This is with the same values/settings used with current master. Also, the amount of dynarec cycles do not appear to affect the likelihood of the crashes.
  • Based on my testings so far, the crashes can occur either at startup, especially if one fastforwards right after launching the content, or when loading a save. The latter in particular is being triggered here consistently whenever I try to load my "Akumajou Dracula X: Gekka no Yasoukyoku" save from the memory card screen with Runahead active, which worked normally before this PR. Again, fastforwarding seems to greatly increase the chance of RetroArch and the core suddenly crashing with this setup.
  • If Dynarec SPU Samples is set to any value above 1 and Runahead is enabled with Single Instance mode, the audio will get noticeably choppy and distorted. Disabling Runahead or setting it to Second Instance Mode makes it sound normal again.

Ryunam avatar Sep 29 '22 14:09 Ryunam

This pull request introduces 1 alert when merging 1e715316728a5d0c1edbf901152b3cbde9f05536 into bd6b9ef3049fe3f70a18ee6f752a935ae83c2f2b - view on LGTM.com

new alerts:

  • 1 for Missing return statement

lgtm-com[bot] avatar Oct 01 '22 02:10 lgtm-com[bot]

@Ryunam Hi, are your issues addressed now?

LibretroAdmin avatar Oct 01 '22 03:10 LibretroAdmin

Okay, I have performed a few more tests after the last commit:

  • The audio issue caused by the combination of SPU Samples set to a value higher than 1 and Runhead Single Instance mode seems to be fixed. I tried switching the values back and forth during gameplay and everything appears to sound normally.
  • However, the Dynarec SPU Samples by itself (with or without Runahead) seems not to play nicely with a few games. "Final Fantasy" (the PS1 remake of FF1) stood out during my testing, as any value higher than 1 triggers missing and sustained notes in the music of that game. A clear example can be seen by starting the game, then immediately entering the first city, Corneria. Set the samples to 16 and the music will start having missing parts or oddly prolonged notes. Setting the Samples value back to 1 makes the music play normally again. I assume this is partly expected behavior, as the feature in question might not work well with certain games depending on how the music files are streamed, but i figure it's worth pointing it out.
  • ~Interestingly, the crashes do not seem to happen anymore either! I have tried to trigger them multiple times and so far so good.~

Ryunam avatar Oct 01 '22 11:10 Ryunam

@Ryunam I'm surprised the crashes disappeared, though I couldn't reproduce so just let me know if they come back.

The SPU Samples option was expected to cause that, I'll add a note in the description about it, it's basically just running the sampling at a lower frequency and then running multiple times to catch up, it's a performance hack, I'm surprised it works as well as it does. Though now that you found a game I can test I might be able to improve it further by doing some run time detection or tweaking the timing code, but that may be a bigger project for the future.

ZachCook avatar Oct 01 '22 14:10 ZachCook

Sorry @ZachCook, I stand corrected. The crashes appeared once again while I was testing other games, so they seem to be unrelated to the SPU changes. They are also very unpredictable: sometimes the affected games will launch normally and then crash when loading a save, sometimes the crash will occur almost immediately at startup, some other times they won't happen at all.

One consistent situation where I can reproduce the crash right now is with the Japanese PS1 port of Street Fighter Zero 2:

  • Under Core Options -> Emulation Hacks, set Skip BIOS to ON.
  • Have Runahead enabled with Second Instance ON in a per-core override, to make it activate automatically upon launching the game.
  • The amount of Runahead frames does not seem to matter. I'm using the default value of 1 in this case.
  • This was tested with both the glcore and Vulkan video drivers in RA.
  • Launch Street Fighter Zero 2 and hold the Fastforward button right after starting the game. One thing that may also contribute to the crash being triggered is pressing/mashing the Circle button while it's fastforwarding, but this was not verified.
  • On my system the game and RA will crash either after the Capcom logo animation, or later on when selecting an option from the main menu (for example, "Versus Mode").

The same core as compiled from current master, before this PR, never triggers any crash with these same steps and/or settings.

Ryunam avatar Oct 01 '22 16:10 Ryunam

Hi there, what is the state of this PR as it stands? Are there any roadblocks?

LibretroAdmin avatar Nov 14 '22 21:11 LibretroAdmin

Hi there, what is the state of this PR as it stands? Are there any roadblocks?

I'll update it again to latest lightrec+lightning, and see if I can reproduce the above mentioned crash, or at least narrow down what triggers it, I wouldn't really want to merge this until that is dealt with.

ZachCook avatar Nov 14 '22 23:11 ZachCook

Updated to recently released Lightning 2.2.0 and Lightrec 0.6

Still working on crashes with runahead and loading, the hack I had there (restart entire dynarec and switch to interpreter for a while) looks awfully suspect and I'm working on a proper fix to at least rule that out as a cause if it doesn't just fix it.

ZachCook avatar Nov 17 '22 21:11 ZachCook

This pull request introduces 1 alert and fixes 1 when merging c41b8a7f88afcc8fb81fd50378746765bbd518c1 into f8a904ef5be34ba0b28fabf1b90325d37b321603 - view on LGTM.com

new alerts:

  • 1 for FIXME comment

fixed alerts:

  • 1 for FIXME comment

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] avatar Nov 17 '22 21:11 lgtm-com[bot]

@ZachCook Is this ready for merging now? Will this have any known regressions vs. the current state of things?

LibretroAdmin avatar Nov 19 '22 14:11 LibretroAdmin

@ZachCook Is this ready for merging now? Will this have any known regressions vs. the current state of things?

@LibretroAdmin It is not ready yet, I am seeing lots of crashing now than can be triggered by any kind of run-ahead or loading a save state.

You could apply just 8814bcbc9c8fdc552287097e87312ded80bd412a for now, it is quite a useful improvement by itself.

ZachCook avatar Nov 19 '22 22:11 ZachCook

OK, I cherry-picked that commit for now and pushed it to master

LibretroAdmin avatar Nov 20 '22 21:11 LibretroAdmin