dolphin
dolphin copied to clipboard
dolphin-emu
BlockingLoop: cleanup Run() logic using compare_exchange_strong
Rather than trying to manually handle state changes that might have occurred between a load and a store, use std::atomic's compare_exchange_strong. Also changes the state machine from Need Execution -> Last Execution -> Done -> Sleeping to Need Execution -> Executing -> Done -> Sleeping.
BlockingLoopTest takes the same amount of time to run before and after this change (~260ms on my computer).
LGTM, but I'm bad at atomics, so maybe someone else should review it as well.
If you were testing on x86, it makes sense that you saw no perf difference, because the generated code should be literally identical; x86 has no concept of weak atomic updates (which accurately indicate success/failure). Changes that deal with atomic semantics should be tested, but they only really need to be tested on arm.
@shuffle2 Can you explain that a bit more? As written, this PR only uses strong atomic updates.
@ligfx in a previous comment you say you tested weak/strong and didn't see a perf difference. That's because there is literally no difference on x86. If you want to tweak the semantics for perf and maintain proper behavior, you'd need to test on an architecture where it will actually make a difference in the first place.
Ah, I see what you mean. Does the current state of the PR need to be tested on ARM, though, or is just x86 okay? I'm not planning on introducing weak atomic semantics at this point.
This wasn't intended to change any logic, merely to clean up the code. If it is changing something that's most likely a bug (unless it inadvertently fixed a bug, which seems less likely). @nbohr1more do you have a bug report you can link to?
I couldn't find an existing bug report but I can create one.
This patch makes the loading behavior more correct.
Currently, if you try to load the Speeder Bike level in master you will be greeted by an infinite buzz sound and black screen after the cut scene.
With this change, you can get in-game and play if you let the cut scene play and if you cancel the cut-scene you have a black screen but in-game audio plays.
So it seems to be more correct than master.
@nbohr1more are you comparing master + these commits to master, or the auto-created build for this pull request against master?
Manual rebuild:
- In this PR, I expand "Show all checks"
- Scroll down to pr_win-x64 and click details
- Click the Rebuild button in the upper right
- Download the build from step 8 (upload)
It seems like the "details" link next to pr-win-x64 doesn't go to the related page (check the Build Properties tab on the buildbot page).
In any case, I don't think triggering a rebuild would rebase the PR commits onto master, anyways (you're just seeing a change in behavior between this PR's base revision and current master).
cc @delroth for the buildbot links...
