dolphin-emu
Savestates: Track dirty pages for the purposes of speeding up sequential savestates
An attempt to solve the dirty page tracking problem referenced here.
First PR in an attempt to realize rollback as an emulator level feature.
What kind of feedback are you seeking on this PR at this stage? Are things working correctly right now?
What kind of feedback are you seeking on this PR at this stage? Are things working correctly right now?
Mostly unsure if the direction is correct, or if I should be directing efforts elsewhere. Yes, it does seem to work (at least on Windows) but I'd also like to get this tested more thoroughly and on other platforms once I implement them.
The logic in the savestating code looks wrong to me. How are you recovering the position of the memory on load?
The logic in the savestating code looks wrong to me. How are you recovering the position of the memory on load?
Yes, that part of the logic is quite rough and definitely wouldn't work as is; effectively my approach to solve the memory latency issues presented by needing to take sequential savestates every frame is to treat each state as a delta, where if you need to load savestate 3 after taking savestate 5 you must load savestates in reverse order from 5 -> 4 -> 3. This reduces the amount of information you need to store between savestates, or, in the case of rollback, frames.
Have you measured what kind of performance improvement you get from this? Before thinking about making it robust, it would be useful to know how much there is to gain from this. For all I know, maybe something other than savestating the memory is taking most of the savestating time.
Have you measured what kind of performance improvement you get from this? Before thinking about making it robust, it would be useful to know how much there is to gain from this. For all I know, maybe something other than savestating the memory is taking most of the savestating time.
I'm seeing significant improvements in a personal project utilizing a slightly different dirty tracking method. This is a fair ask, and I will benchmark each "stating step" to make sure my theory is correct. If it's not, I will close this draft.
For reference: 1 ms is the goal for both saves and loads, with up to 2 ms being passable; project Slippi saves and loads its state in ~1 ms depending on the machine, with mine clocking in comfortably under 1 ms.
After some initial benchmarking:
On master
| Game | Save Time | Load Time |
|---|---|---|
| Super Smash Bros. Melee (On FoD, 1v1) | ~11.5 ms | ~67.5 ms |
| Super Smash Bros. Brawl (On New Pork City, 1v1) | ~25.5 ms | ~100.5 ms |
Here are my (relevant) specs: AMD Ryzen 9 5950X NVIDIA GeForce GTX 1060 6GB 32 GB DDR4-3603 / PC4-28800 DDR4 SDRAM UDIMM
What's interesting is that most expensive action for saving a savestate in both games was stating within the Hardware namespace, with about ~18 ms spent stating Hardware and ~8 ms of that being the memory state in Brawl, and ~8 ms in Melee with ~5-6 ms of it being just memory. Conversely, by far the most expensive action in loading state is the PowerPC namespace, with ~55 ms in Melee and ~60 ms in Brawl.
I'd therefore argue that the most gains for saving state would indeed be with the memory portion, but with loading state it would be by far more effective to focus on the PPC state functionality.
In the PR comments, you've described that you're going to make savestates be deltas rather than complete snapshots of the system. This kind of logic must not be the default, as it ruins normal use of savestates, as well as TAS use of savestates.
Do you have a preference for how I should lock the current logic out of regular use? My thought is to pull the logic out of the original PR to mark logic as rollback related, but I don't think we would want to give people the wrong idea that rollback is even close to functional yet.
At the current stage, the code doesn't look like it will even reliably function, so I don't think that's something we really have to think about at this stage. This isn't going to get merged in its current state anyway. Long-term, I guess it would make sense to have some kind of parameter to the savestating code to indicate whether you want a regular savestate or a delta savestate, maybe?