Page MenuHomeWildfire Games

AMD Ryzen fix
ClosedPublic

Authored by Stan on May 7 2020, 6:41 PM.

Details

Summary

Currently we use a wide variety of timers on windows. HPET nearly often fails to initialize because it requires admin rights. The fallback is usually TSC which is an usually reliable timer, that unfortunately seems to have some issues on the latest Ryzen CPUS. This patch force QPC usage. This way all platforms can run the game the same way. It should not have a big performance impact as computers are way faster than they used to 10 years ago. Ideally we'd find a better way to handle those cases but this is patch is in case we don't find a better solution for A24

All the other big engines also use QPC

According to this post on Reddit

Windows will use HPET if required, generally games use QueryPerformanceCounter (QPC) and Windows 10 will use the fastest timer available e.g Invariant TSC (ITSC) or Time Stamp Counter (TSC). Default settings for Windows 10 is HPET available ( not disabled ) and other timers available ( not disabled )

I will post the discussion with @janwas after cleaning it a bit.

See also the discussion here
I also made a post here no answers yet.

The mail exchange with AMD currently lead nowhere.

About clock monotonic https://stackoverflow.com/questions/3523442/difference-between-clock-realtime-and-clock-monotonic

Test Plan

Compile and run the game on windows, try to play in both single player and multiplayer, and report if there are any issues

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Stan updated this revision to Diff 13716.Nov 4 2020, 7:31 PM
Stan edited the summary of this revision. (Show Details)

Delete more stuff

Vulcan added a comment.Nov 4 2020, 7:31 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3369/display/redirect

Vulcan added a comment.Nov 4 2020, 7:32 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2816/display/redirect

Vulcan added a comment.Nov 4 2020, 7:33 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1717/display/redirect

Stan updated this revision to Diff 13717.Nov 4 2020, 7:34 PM

Fix header including itsefl

Vulcan added a comment.Nov 4 2020, 7:34 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3370/display/redirect

wraitii added a subscriber: wraitii.Nov 4 2020, 7:34 PM

IMO you can go much further, see inline

source/lib/timer.cpp
78

Replace with

QueryPerformanceFrequency(&freq); // new var
QueryPerformanceCounter(&start); // same as others
107–109

Replace with t = QueryPerformanceCounter() - start

Vulcan added a comment.Nov 4 2020, 7:35 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2817/display/redirect

Vulcan added a comment.Nov 4 2020, 7:35 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1718/display/redirect

Stan updated this revision to Diff 13722.Nov 4 2020, 10:44 PM

More removal

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3374/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2821/display/redirect

Stan marked 2 inline comments as done.Nov 4 2020, 10:46 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1722/display/redirect

This is much closer to what I had in mind, yes :)
It seems to compile on my mac, but phabricator has trouble applying the diff, obviously.

source/lib/timer.cpp
85

I think you can make start a LARGE_INTEGER, and cast in timer_Time() before the freq division.

wraitii added inline comments.Nov 5 2020, 8:57 AM
source/lib/timer.cpp
109

you need now - start here

Stan updated this revision to Diff 13724.Nov 5 2020, 9:15 AM

Remove more code

Vulcan added a comment.Nov 5 2020, 9:16 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3375/display/redirect

Vulcan added a comment.Nov 5 2020, 9:17 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2822/display/redirect

Vulcan added a comment.Nov 5 2020, 9:17 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1723/display/redirect

Stan updated this revision to Diff 13725.Nov 5 2020, 9:22 AM

Add start back

Vulcan added a comment.Nov 5 2020, 9:22 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3376/display/redirect

Vulcan added a comment.Nov 5 2020, 9:22 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1724/display/redirect

Vulcan added a comment.Nov 5 2020, 9:23 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2823/display/redirect

Stan updated this revision to Diff 13726.Nov 5 2020, 9:24 AM
Stan marked an inline comment as done.

Last comment

Vulcan added a comment.Nov 5 2020, 9:25 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3377/display/redirect

Vulcan added a comment.Nov 5 2020, 9:25 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1725/display/redirect

Vulcan added a comment.Nov 5 2020, 9:25 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2824/display/redirect

Stan updated this revision to Diff 13727.Nov 5 2020, 9:30 AM

make it shorter, use multiplication instead of division each frame

Vulcan added a comment.Nov 5 2020, 9:31 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3378/display/redirect

Vulcan added a comment.Nov 5 2020, 9:31 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1726/display/redirect

Vulcan added a comment.Nov 5 2020, 9:31 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2825/display/redirect

Vulcan added a comment.Nov 5 2020, 9:45 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3379/display/redirect

Vulcan added a comment.Nov 5 2020, 9:45 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1727/display/redirect

Vulcan added a comment.Nov 5 2020, 9:45 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2826/display/redirect

Stan updated this revision to Diff 13730.Nov 5 2020, 9:59 AM

Sanity checks cleanup

Freagarach added a subscriber: Freagarach.EditedNov 5 2020, 10:16 AM

I've tested this on (L)Ubuntu 18.04, doesn't break.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3381/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2828/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1729/display/redirect

The Game still runs on Windows AMD Ryzen 3700X.

vladislavbelov added inline comments.Nov 5 2020, 8:35 PM
binaries/system/readme.txt
89

Is it still true, that it's unsafe?

source/lib/sysdep/os/win/whrt/qpc.cpp
39

Can it be used? Since it uses QPC.

source/lib/sysdep/os/win/wposix/wpthread.cpp
520

There are usages (commented or not) of the sem_timedwait in our code.

source/lib/sysdep/os/win/wposix/wtime.cpp
27

Missed and not deleted occurrence of clock_gettime.

source/lib/timer.cpp
108

QueryPerformanceCounter also returns BOOL, I think we need to check that too.

Stan updated this revision to Diff 13752.Nov 5 2020, 8:51 PM
Stan marked 5 inline comments as done.

Add an ENSURE.

Vulcan added a comment.Nov 5 2020, 8:52 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3402/display/redirect

Vulcan added a comment.Nov 5 2020, 8:53 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2849/display/redirect

Stan marked an inline comment as done.Nov 5 2020, 8:53 PM
Stan added inline comments.
binaries/system/readme.txt
89

Yes on old hardware eg Pentium3. On new it will just use HPET.

Worth noting that all the engines use it and that Microsoft recommends it. Also worth noting that our code was more unsafe.

source/lib/sysdep/os/win/whrt/qpc.cpp
39

I did inially, but it's unecessary overhead. And @wraitii asked me to remove it to extract the core functionnality.

source/lib/sysdep/os/win/wposix/wpthread.cpp
520

Not on windows. And if on Linux, then it uses native functions not this weird emulation. I think this whole file will be removed when @wraitii finish his migration to std::thread.

I can delete wprofiler.h if you want as it's not used.

Vulcan added a comment.Nov 5 2020, 8:54 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1750/display/redirect

vladislavbelov added inline comments.Nov 6 2020, 7:25 PM
source/lib/timer.cpp
132–134

Incorrect name.

vladislavbelov added inline comments.Nov 6 2020, 7:26 PM
source/lib/sysdep/os/win/wposix/wpthread.cpp
520

Maybe, but at least you have to remove usages of the removed functions.

Stan updated this revision to Diff 13768.Nov 6 2020, 7:49 PM
Stan marked an inline comment as done.

lpfrequency to fequency

Vulcan added a comment.Nov 6 2020, 7:49 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3417/display/redirect

Vulcan added a comment.Nov 6 2020, 7:50 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2864/display/redirect

Vulcan added a comment.Nov 6 2020, 7:51 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1765/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Nov 7 2020, 12:18 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.