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

Timer improvements #10872

Merged
merged 8 commits into from Aug 3, 2022
Merged

Timer improvements #10872

merged 8 commits into from Aug 3, 2022

Conversation

shuffle2
Copy link
Contributor

  • refactors Common::Timer to be much simpler and (hopefully) clear
  • moves unrelated code out of Common::Timer
  • Windows: disables some recent power throttling OS features. I'm not sure how effective this is, someone should test on laptop or something.
  • internally uses std::chrono in most places

@Zopolis4
Copy link
Contributor

Is this supposed to contain the move to C++20?

@shuffle2
Copy link
Contributor Author

Is this supposed to contain the move to C++20?

I was testing to see if that would enable non-msvc to use the time zone components of chrono, but it did not. So that code is msvc-only for now.

@shuffle2 shuffle2 force-pushed the timer branch 3 times, most recently from b666c9d to 689bbb0 Compare July 18, 2022 18:01
@shuffle2
Copy link
Contributor Author

A further improvement would be going through the codebase and wrapping the raw usages of Common::Timer::NowUs() in a similar way to protect against rollover, but I'm going to stop adding on to this PR now :)

@shuffle2 shuffle2 force-pushed the timer branch 2 times, most recently from 63aefc1 to 29f5d8a Compare July 18, 2022 19:36
@iwubcode
Copy link
Contributor

iwubcode commented Jul 19, 2022

My generic comment here is that the timer class is still holding onto C++98 (?) concepts with names that embed the time type into them, ex: SYNC_BUTTON_HOLD_MS_TO_RESET, NowUs, etc. Part of the beauty of chrono is that time math and time comparisons are just so intuitive. Probably shouldn't be this PR but I'd like for us to move to using chrono fully.

Otherwise this looks pretty good. Thank you for doing this!!!! Yay chrono. Hopefully one day we can migrate more of our code to std::filesystem too..

@shuffle2 shuffle2 force-pushed the timer branch 3 times, most recently from 098c371 to f6846bf Compare July 22, 2022 17:21
@shuffle2
Copy link
Contributor Author

I only made the comment about PROCESS_POWER_THROTTLING_EXECUTION_SPEED a little more clear; i think the PR is ready :)

@shuffle2 shuffle2 force-pushed the timer branch 2 times, most recently from 4a569bd to 5c0039a Compare July 30, 2022 18:25
@lioncash lioncash merged commit a8b2174 into dolphin-emu:master Aug 3, 2022
11 checks passed
@shuffle2 shuffle2 deleted the timer branch August 5, 2022 00:28
@piau9000
Copy link

piau9000 commented Aug 5, 2022

This broke the emulator for a lot of people.
After this commit, the following message shows up when trying to open it:

2022-08-05

@shuffle2
Copy link
Contributor Author

shuffle2 commented Aug 5, 2022

update your vc++ redist https://aka.ms/vs/17/release/vc_redist.x64.exe
or https://aka.ms/vs/17/release/vc_redist.arm64.exe if you're on arm64

@piau9000
Copy link

piau9000 commented Aug 5, 2022

Thanks. I'm gonna post it in the forums to the others that are having it too.

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