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

Increase RemoteDebuggerPeerTCP poll to 6.9ms to reduce CPU usage #57526

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

tavurth
Copy link
Contributor

@tavurth tavurth commented Feb 1, 2022

Closes #57256

Screenshot 2022-02-01 at 22 17 32

And profile report:

Screenshot 2022-02-01 at 22 20 23

@tavurth tavurth requested a review from a team as a code owner February 1, 2022 19:17
@tavurth tavurth changed the title Try fix mac OS high CPU usage Fixes mac OS high CPU usage Feb 1, 2022
@Faless
Copy link
Collaborator

Faless commented Feb 1, 2022

Please use a descriptive commit name, e.g.:

Increase RemoteDebuggerPeerTCP poll interval to 10ms

Fix high CPU usage on MacOS.

Did you try with 1ms instead? Does it make much difference?

@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

@Faless I will check that now, and I will add a more descriptive name & force push

@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

1ms seems to result in an increase of around 6% CPU with sleep_usec set to 40,000.

Screenshot 2022-02-01 at 22 30 30

I will try @ 4.1ms to fit with the 144hz refresh rate

@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

@Faless 4.1ms seems to be a good compromise as it leads to only 1% increase in CPU while updating at 144hz.

Thoughts?

Screenshot 2022-02-01 at 22 35 09

@Faless
Copy link
Collaborator

Faless commented Feb 1, 2022

@Faless 4.1ms seems to be a good compromise as it leads to only 1% increase in CPU while updating at 144hz.

Thoughts?

@tavurth that sounds good. Thanks!

@tavurth tavurth force-pushed the bugfix/high-macos-cpu-usage branch from 4c486f9 to fc3a72a Compare February 1, 2022 19:37
@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

Updated commit message and force pushed

@Calinou
Copy link
Member

Calinou commented Feb 1, 2022

4.1 ms doesn't match a 144 Hz refresh rate, it matches (roughly) a 240 Hz refresh rate. Nonetheless, if it works well, you can keep this value 🙂

@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

6.9ms seems to give a slight cpu reduction (on average dipping to 9%) but not a huge amount.

Screenshot 2022-02-01 at 23 32 40

I will push that version so we can eak the maximum out of it as well as for correctness.

@tavurth tavurth force-pushed the bugfix/high-macos-cpu-usage branch from fc3a72a to 48be532 Compare February 1, 2022 20:35
Fix high CPU usage on MacOS by reverting the polling for Network
debugging to match 144hz refresh rate.
@tavurth tavurth force-pushed the bugfix/high-macos-cpu-usage branch from 48be532 to c37bd41 Compare February 1, 2022 20:36
@tavurth
Copy link
Contributor Author

tavurth commented Feb 1, 2022

Seems ready

@akien-mga akien-mga changed the title Fixes mac OS high CPU usage Increase RemoteDebuggerPeerTCP poll to 6.9ms to reduce CPU usage Feb 2, 2022
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

@akien-mga akien-mga merged commit 0509086 into godotengine:master Feb 2, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 2, 2022
@akien-mga
Copy link
Member

Let's give this value a try, and if some users find it still too high, or too low for real-time updates, we could also consider making it configurable.

@tavurth
Copy link
Contributor Author

tavurth commented Feb 2, 2022

@akien-mga configurable would be very nice!

I'm not sure if this needs to be cherry picked back to 3.x because as far as Calinou stated the system was rewritten for 4.x

@Faless Faless removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 2, 2022
@Faless
Copy link
Collaborator

Faless commented Feb 2, 2022

Yeah, we don't need the cherry pick, the new debugger code (which uses threads) was not ported to 3.x

@tavurth tavurth deleted the bugfix/high-macos-cpu-usage branch February 2, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Godot 4.x uses minimum 20% CPU on Mac while debugging (Intel)
4 participants