Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GETLLAR spin detection and restore default configuration as before #12523 #12544

Merged
merged 1 commit into from Aug 22, 2022

Conversation

elad335
Copy link
Contributor

@elad335 elad335 commented Aug 21, 2022

Revert GETLLAR polling detection to be off (100 by default now) and the setting has been renamed to "SPU GETLLAR Busy Waiting Percentage", added a new setting with the old one's name "SPU Reservation Busy Waiting Percentage" which controls busy-waiting in reading SPU_RdEventStat channel (0 by default). Demon Souls needs GETLLAR to be 0 to have the results of pr #12523.

@elad335 elad335 force-pushed the savestates branch 2 times, most recently from bc987bc to b3407b9 Compare August 21, 2022 15:13
@elad335 elad335 force-pushed the savestates branch 3 times, most recently from ca537c2 to ca653ae Compare August 21, 2022 16:38
@elad335 elad335 changed the title [Test Only] SPU modifications Fix GETLLAR spin detection Aug 21, 2022
@elad335 elad335 force-pushed the savestates branch 5 times, most recently from 454b908 to b5bcc70 Compare August 21, 2022 18:00
@elad335 elad335 marked this pull request as ready for review August 21, 2022 18:00
@elad335
Copy link
Contributor Author

elad335 commented Aug 21, 2022

Relaxed default configuartions.

@elad335 elad335 changed the title Fix GETLLAR spin detection Fix GETLLAR spin detection and restore default configuration as before #12523 Aug 21, 2022
@kd-11 kd-11 requested a review from Nekotekina August 21, 2022 21:47
@solarmystic
Copy link

solarmystic commented Aug 21, 2022

Almost completely restores performance back to pre #12523 levels, well done.

Testing with Persona 5

  1. SPU Reservation Busy Waiting Percentage to 0 - 79 FPS
    image

  2. SPU Reservation Busy Waiting Percentage to 50 - 79 FPS
    image

  3. SPU Reservation Busy Waiting Percentage to 100 - 77 FPS
    image

Previous master (14068) - 80 FPS
image

As a side note, overall CPU usage has been reduced in this PR for all 3 permutations of settings compared to the previous master, from 66% to 62-64% on average, depending on Busy Waiting Percentage set.

@Nekotekina Nekotekina merged commit ee87fdc into RPCS3:master Aug 22, 2022
@solarmystic
Copy link

Seems to break performance in Ratchet and Clank Quest for Booty, according to Ramiro from the rpcs3 discord

https://cdn.discordapp.com/attachments/272035812277878785/1011173235808796693/unknown.png

image

https://discord.com/channels/272035812277878785/272035812277878785/1011173017692426262

@elad335
Copy link
Contributor Author

elad335 commented Aug 22, 2022

Reset the settings!

@Danke-Boi
Copy link

Revert GETLLAR polling detection to be off (100 by default now) and the setting has been remaned to "SPU GETLLAR Busy Waiting Percentage", added a new setting with the old one's name "SPU Reservation Busy Waiting Percentage" which controls busy-waiting in reading SPU_RdEventStat channel (0 by default). Demon Souls needs GETLLAR to be 0 to have the results of pr #12523.

*renamed
feel free to delete this after it's fixed, just thought you might wanna know

@Nerboruto
Copy link

Nerboruto commented Aug 23, 2022

I tried the latest version of the optimization + fix and unfortunately I think that part of the performance increase has been lost .. the first versions were really smooth, in this build the improvement is less. however there is the bugfix.
UPDATE: sorry for the comment, I was misled by doing tests between the various versions. I had put getllar poll to 0 but I found it again at 100. my fault .. sorry

@kd-11
Copy link
Contributor

kd-11 commented Aug 23, 2022

Please avoid commenting on the PR, it just causes confusion since other changes already happened since this PR.
Create a fresh ticket showing the loss in performance so we can escalate the matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants