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 compile times by fixing the sccache mechanism #2208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

G-D-Petrov
Copy link
Collaborator

@G-D-Petrov G-D-Petrov commented Feb 27, 2025

Reference Issues/PRs

Monday ref: 8590725039

What does this implement or fix?

Currently, the sscache mechanism doesn't work properly because:

  1. there is not enough space in GHA to store all of the data
  2. for windows specifically, the build is using precompiled headers and sccache doesn't support them and so can't cache the objects for them

This PR changes the flows to:

  • use S3 as a backend for the cache for "internal" PRs (still falls back to GHA for PRs by external contributors)
  • removes the precompiled headers for windows builds

This results in 3-4x speed up for compilation (10-15 min now vs 30-45 min before) for all builds.
Now the Linux builds + tests take less than an hour ( vs ~90 min before)

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

Try to setup s3 sccache

Add SCCACHE environment variables to build workflow

Update SCCACHE configuration in build workflow to include region and SSL settings

Remove unnecessary GitHub default packages in build workflow to save space

Update condition for enabling Windows compiler commands in build workflow

Simplify conditions for installing MSVC and enabling Windows compiler commands in build workflow

Remove /Zm flags from compiler options and restore PCH usage in CMake presets

Remove /Zm flags from CMakeLists and disable PCH usage in CMake presets

Clarify SCCACHE_GHA_VERSION environment variable comment in build_steps.yml
@@ -101,16 +110,12 @@ jobs:
CMAKE_BUILD_PARALLEL_LEVEL: ${{vars.CMAKE_BUILD_PARALLEL_LEVEL}}

# ========================= Leader steps =========================
- name: Remove GitHub default packages (baseimage) # To save space
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is no longer needed as the cache is reducing the compilation load, i.e. no as much disk space is needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

May still need it if we ever tear down (or invalidate) the whole cache?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doesn't seem like it, the wheels build was able to run even when an empty cache (when I changed to S3)
The step below is still needed for C++ tests jobs.

@G-D-Petrov G-D-Petrov added bug Something isn't working patch Small change, should increase patch version labels Feb 27, 2025
@G-D-Petrov G-D-Petrov mentioned this pull request Mar 4, 2025
5 tasks
@@ -48,7 +48,15 @@ jobs:
runs-on: ${{ needs.start_ec2_runner.status != 'failure' && needs.start_ec2_runner.outputs.label || matrix.distro}}
container: ${{ (matrix.os == 'linux' && inputs.job_type != 'build-python-wheels') && matrix.container || null}}
env:
SCCACHE_GHA_VERSION: ${{vars.SCCACHE_GHA_VERSION || 1}} # Setting this env var enables the caching
# 0 - uses S3 Cache, 1 - does uses GHA cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo does uses should be uses

@@ -48,7 +48,15 @@ jobs:
runs-on: ${{ needs.start_ec2_runner.status != 'failure' && needs.start_ec2_runner.outputs.label || matrix.distro}}
container: ${{ (matrix.os == 'linux' && inputs.job_type != 'build-python-wheels') && matrix.container || null}}
env:
SCCACHE_GHA_VERSION: ${{vars.SCCACHE_GHA_VERSION || 1}} # Setting this env var enables the caching
# 0 - uses S3 Cache, 1 - does uses GHA cache
# this was extract PRs can use the GHA cache
Copy link
Collaborator

Choose a reason for hiding this comment

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

Think there's a typo in this sentence, I don't understand it

# this was extract PRs can use the GHA cache
SCCACHE_GHA_VERSION: ${{secrets.AWS_S3_ACCESS_KEY && 0 || 1}}
SCCACHE_BUCKET: arcticdb-ci-sccache-bucket
SCCACHE_ENDPOINT: http://s3.eu-west-1.amazonaws.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know what region our CI runners run in? Allegedly they're all in the US, so that might be a better location for the cache.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will test this next week

@@ -101,16 +110,12 @@ jobs:
CMAKE_BUILD_PARALLEL_LEVEL: ${{vars.CMAKE_BUILD_PARALLEL_LEVEL}}

# ========================= Leader steps =========================
- name: Remove GitHub default packages (baseimage) # To save space
Copy link
Collaborator

Choose a reason for hiding this comment

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

May still need it if we ever tear down (or invalidate) the whole cache?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch Small change, should increase patch version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants