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

Added caching in actions #542

Closed
georgemoralis opened this issue Aug 23, 2024 · 8 comments · Fixed by #985
Closed

Added caching in actions #542

georgemoralis opened this issue Aug 23, 2024 · 8 comments · Fixed by #985

Comments

@georgemoralis
Copy link
Collaborator

georgemoralis commented Aug 23, 2024

Especially windows builds are slow maybe caching can be added in ci

@aidenzzz
Copy link
Contributor

Maybe something like this?

@georgemoralis
Copy link
Collaborator Author

That could probably work somehow yes

@LeDragoX
Copy link
Contributor

Maybe something like this?

I tried using your example and reading through the updated docs https://github.com/actions/cache and
I've only managed to improve the "Configure CMake" time, maybe I've done something wrong.

macOS-Qt Linux-Qt Windows-Qt
image image image

So, I'm thinking about switching it to https://github.com/hendrikmuhs/ccache-action and see if it helps with the build time (hoping the hash keys will match)
Is this a great option?

@Artalus
Copy link

Artalus commented Sep 16, 2024

#622 now speeds up Linux and Mac builds, but @LeDragoX mentioned that they could not set up ccache for Windows builds. Speaking from my experience maintaining my company's CI, ccache indeed does not behave when CMake is configured to use MSBuild*. The alternative for me was to build our CMake projects using Ninja** instead. Moreover, Ninja on its own is already faster than MSBuild even without compiler caching.

There is a caveat though: some directives in CMakeLists are handled differently by the two generators***. So if only Ninja were to be used in CI, a subtle bug in CMakeLists.txt could evade the automated checks and create issues for Visual Studio users relying on the default generator


I ran some very basic experiments with Github Actions in my fork of the shadPS4 repo. Using cmake -G Ninja alone reduces time of cmake --build from ~20min to ~5min . Throwing ccache into the mix boosts this to ~2min.
The shadPS4.exe binary seems to be functioning properly when built with Ninja, but I did notice at least two differences compared to the upstream Github Actions:

  • the shadps4-....zip bundle uploaded from my Actions contains more files (specifically: dxcompiler.dll, dxil.dll, vc_redist.x64.exe). Not sure where these came from, might be one of dependencies or just a quirk of winqtdeploy
  • I was getting a lot of warning: argument unused during compilation: '/Zc:preprocessor' (externals/toml11 does not fully support clang-cl) - at the same time, I did not see some of the warnings present in the "default" build logs. This might be due to incorrect compiler flags setup somewhere in CMakeLists, mentioned before

; but to be fair, I did not delve deeply into this beyond "check if Windows CI builds with -G Ninja and -DCMAKE_CXX_COMPILER_LAUNCHER=ccache".

If the maintainers are willing to give this a try (the changes should only affect CI, not the local developer experience), I am ready to investigate further and submit a proper PR to switch the CI to use Ninja+ccache.


Some details on buildsystems

* MSBuild is the buildsystem used by Visual Studio itself to build "regular" .vcxproj projects - and therefore by CMake when it generates files for VS (either by default or when provided with -G Visual Studio ### configuration flag).
** Ninja is a low-level buildsystem supposed to be used by CMake and other meta-buildsystems. Modern installations of Visual Studio bundle Ninja together with CMake, so it is available on CI with minimal tinkering.
*** MSBuild is what CMake calls a multi-config generator (allowing builds of both Debug and Release from a single somelib.vcxproj file), while Ninja is a single-config generator (somelib.ninja would only contain info for either Debug or Release builds, depending on your CMake configuration flags). CMake internally uses different mechanisms for Ninja and VS project generation, which can lead to issues if CMakeLists was accidentally written to work with one generator but not the other.
Most notable difference - using if(CMAKE_BUILD_TYPE STREQUAL "Debug") to change the compiler flags / definitions would only work with Ninja or Makefiles; for Visual Studio you'd have to use $<Generator:expressions> instead)

@viniciuslrangel
Copy link
Contributor

@Artalus I only code using ninja + clang-cl on Windows for a while, and it works fine. It's ok to change

@LeDragoX
Copy link
Contributor

LeDragoX commented Sep 16, 2024

@Artalus I saw your fork's actions and the difference is extraordinary, I knew MSBuild build didn't work, but I think my ccache installation before was missing with ninja, that's why I couldn't make it work before. Even with sccache MSBuild didn't worked.
A PR with your changes would be welcome 🔥
Edit: just need to remove vc_redist.x64.exe from the upload... maybe deleting manually after running windeployqt, or with some parameter, I'm not sure why that happens tho.

@LeDragoX
Copy link
Contributor

LeDragoX commented Sep 16, 2024

@Artalus, are you on discord? I'm willing to help or make a PR with your solution, you can find me on the shadPS4's server then tag me. Windows' CIs are really needing this upgrade.
Just telling because I may have found the answer to vc_redist.x64.exe bundling inside the artifacts.

@LeDragoX
Copy link
Contributor

LeDragoX commented Sep 17, 2024

@Artalus If you want to try there, I can confirm adding --no-compiler-runtime to windeployqt on windows-qt.yml will remove vc_redist.x64.exe from the artifact, so only the .dlls will remain, and should be good enough.
I saw that moving the files dxcompiler.dll and dxil.dll still allow shadPS4.exe to initialize normally, I may need to look another parameter on windeployqt to solve that.

Edit: Adding --no-system-d3d-compiler --no-system-dxc-compiler solves the "ignorable" DLLs.

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

Successfully merging a pull request may close this issue.

5 participants