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

Improve CI build and test speeds #1571

Closed
ronaldtse opened this issue Aug 2, 2021 · 20 comments
Closed

Improve CI build and test speeds #1571

ronaldtse opened this issue Aug 2, 2021 · 20 comments
Assignees

Comments

@ronaldtse
Copy link
Contributor

Our CI builds have increased in build times since adding multiple platforms.

The Windows MSVC builds/tests are especially slow and is becoming a blocking issue for effectively merging PRs.

We need to improve our CI build/test speed.

Thanks to @maxirmx for the following analysis of CI build speeds, we know what to improve one by one.

Workflow Job Total time Build and test Build only Test only
Windows MSVC x64 1h 20m 6m 1h 13m
Windows MSVC Win32 2h 12m 6m 2h 5m
Windows MSYS2 25m 19m
Debian 9 1h 9m 56m
CentOS 7 48m 17m
-- 18m
-- 28m
-- 32m
-- 41m
-- 48m
CentOS 8 1h 6m 32m
-- 41m
-- 45m
-- 55m
-- 56m
macOS 10.15 24m 23m
macOS 11.0 25m
@ronaldtse
Copy link
Contributor Author

@maxirmx could you please help here? Thanks.

@maxirmx maxirmx self-assigned this Aug 5, 2021
@maxirmx
Copy link
Member

maxirmx commented Aug 5, 2021

It looks like MSVC jobs use Debug configuration
Here is an explanation:
https://stackoverflow.com/questions/27086145/what-is-the-default-build-configuration-of-cmake/55993599#:~:text=In%20this%20answer%2C%20it%20says,the%20default%20cmake%20build%20configuration.

I believe I reproduced this issue locally.

@ronaldtse
Copy link
Contributor Author

@maxirmx is the debug configuration causing tests to run much slower?

@ni4
Copy link
Contributor

ni4 commented Aug 5, 2021

@ronaldtse If there are no optimisations then tests would run slower indeed. Looks like we are hardcoding Debug configuration in the windows-msvc.yml workflow:

cmake -B ./build -G "Visual Studio 16 2019" -A ${{ matrix.arch }} -DCMAKE_TOOLCHAIN_FILE=${{ matrix.vcpkg_dir }}\\scripts\\buildsystems\\vcpkg.cmake -DCMAKE_BUILD_TYPE=Debug .

@maxirmx
Copy link
Member

maxirmx commented Aug 5, 2021

For -G "Visual Studio 16 2019" build type is ignored.
cmake --build --config is required

@maxirmx
Copy link
Member

maxirmx commented Aug 5, 2021

@maxirmx is the debug configuration causing tests to run much slower?

Well, it looks like debug configuration doubles test time for MSVC
Specifically all "..dsa.." tests are ~10x slower

I changed it, we will see if it helps.

@maxirmx
Copy link
Member

maxirmx commented Aug 5, 2021

debian9 job rebuilds some or all dependencies without cashing
I believe python build takes extra 20 minutes or so

maxirmx added a commit that referenced this issue Aug 5, 2021
@ronaldtse
Copy link
Contributor Author

Indeed we would want to have some aggressive caching to speed up the CI time.

@maxirmx
Copy link
Member

maxirmx commented Aug 5, 2021

I will do a fork. Otherwise my experiments with GHA will block everybody else

@maxirmx
Copy link
Member

maxirmx commented Aug 7, 2021

Indeed we would want to have some aggressive caching to speed up the CI time.

Debian9 job uses i386 (32 bit) container and is affected by the following bug:
actions/checkout#334
both for actions/checkout@v2 and actions/cache@v1 (v2) if run at GHA

There are two possible solutions that can resolve the issue and allow run optimized checkout and aggressive caching:

  1. Rub Debian9 in amd64 container.
  2. Locate and resolve missing dynamic library [== patch action, or container or both]

@ronaldtse
Copy link
Contributor Author

I'm not even sure why we're only testing Debian 9 but not Debian 10/11.

  1. I think we should run both Debian 9 in i386 and amd64 to ensure tests work on 32-bit systems.
  2. We should try to fix the i386 issue in this case.

@ni4
Copy link
Contributor

ni4 commented Aug 8, 2021

@ronaldtse Debian 9 CI was added to be able to check system with 32-bit time_t

@ronaldtse
Copy link
Contributor Author

Thanks for the reminder! We should probably test against Debian 10 and 11 as well.

ronaldtse pushed a commit that referenced this issue Aug 8, 2021
ronaldtse pushed a commit that referenced this issue Aug 8, 2021
@ronaldtse
Copy link
Contributor Author

@maxirmx now that #1582 is merged, do you have any further suggestions on how we can improve/streamline the build times?

@maxirmx
Copy link
Member

maxirmx commented Aug 10, 2021

Some thoughts how to reduce debian9 build time by 10 min and bring overall time down to 55-56 min:

  • Debian9 run each time compiles/builds automake and cmake tools. It is required because there is no binary distributiions available for 32-bit automake and cmake.
  • We could use caching but actions/cache does not work for 32-bit containers because it is based on Node.js and 32-bit Node.js is not supported by runner configuration
  • We could patch runner confguration but there is no binary distribution for 32-bit Node.js available for required version

So we can do one of the following:

  1. Use custom Debian container with cmake and automake preinstalled. We will have to host it at Docker hub (simplest)
  2. Host 32-bit cmake and automake distributions somewhere
  3. Host 32-bit Node.js distribution somewhere (community friendly since it will help all other developers that are struggling with missing 32-bit actions\cache)

@ni4
Copy link
Contributor

ni4 commented Aug 12, 2021

@maxirmx I was also thinking about docker containers with all dependencies preinstalled (please see #1508) . Those may be published on Github Container Registry for faster download/access speed.

@maxirmx
Copy link
Member

maxirmx commented Aug 12, 2021

@maxirmx I was also thinking about docker containers with all dependencies preinstalled (please see #1508) . Those may be published on Github Container Registry for faster download/access speed.

It looks like deepndencies' setup takes 2 minutes for all platforms except debian.

@ronaldtse
Copy link
Contributor Author

It looks like deepndencies' setup takes 2 minutes for all platforms except debian.

In this case the additional maintenance burden of docker containers is probably not worthwhile. The caching mechanisms implement are already good enough I suppose.

@ni4
Copy link
Contributor

ni4 commented Aug 12, 2021

It looks like deepndencies' setup takes 2 minutes for all platforms except debian.

Having Docker container has another benefit - we would be safe from periodical failures due to runner or dependencies updates.

@ronaldtse ronaldtse added this to RNP Dec 17, 2021
@ronaldtse ronaldtse moved this to Medium priority in RNP Dec 17, 2021
ni4 pushed a commit that referenced this issue Jan 20, 2022
ni4 pushed a commit that referenced this issue Jan 20, 2022
ni4 pushed a commit that referenced this issue Jan 24, 2022
ni4 pushed a commit that referenced this issue Jan 24, 2022
@maxirmx
Copy link
Member

maxirmx commented Oct 30, 2023

I believe this issue can be closed now.
With a combination of caching and pre-build docker containers we do not do any excessive steps in setup/configure/build tasks

@maxirmx maxirmx closed this as completed Oct 30, 2023
@github-project-automation github-project-automation bot moved this from Medium priority to Done in RNP Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

3 participants