Skip to content

Use ccache in GHA pipelines #45701

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Use ccache in GHA pipelines #45701

wants to merge 3 commits into from

Conversation

jonashaag
Copy link
Contributor

@jonashaag jonashaag commented Jan 29, 2022 •
edited
Loading

Saves around 5–10 minutes in each of the GHA builds.

Also cleans up some duplicated code.

  • closes #xxxx
  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

@jonashaag jonashaag force-pushed the ccache branch 6 times, most recently from c368923 to a160eca Compare February 4, 2022 21:36
@pep8speaks
Copy link

pep8speaks commented Feb 5, 2022 •
edited
Loading

Hello @jonashaag! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-02-28 08:52:23 UTC

@jonashaag jonashaag force-pushed the ccache branch 3 times, most recently from 2584771 to 55d8df1 Compare February 11, 2022 06:28
@jonashaag jonashaag changed the title WIP Use ccache in GH pipelines Use ccache in GH pipelines Feb 11, 2022
@jonashaag jonashaag changed the title Use ccache in GH pipelines Use ccache in GHA pipelines Feb 11, 2022
@jonashaag jonashaag mentioned this pull request Feb 11, 2022
Closed
4 tasks
@jbrockmendel jbrockmendel added the CI Continuous Integration label Feb 11, 2022
@jonashaag jonashaag marked this pull request as ready for review February 12, 2022 09:02
mroeschke
mroeschke reviewed Feb 12, 2022
View reviewed changes
.github/actions/setup-ccache/action.yml Outdated
shell: bash

- name: Setup ccache
uses: hendrikmuhs/ccache-action@2181be813387616fc2b8dca72d3ff8b912b25f73
Copy link
Member

Choose a reason for hiding this comment

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

Can you reference a version instead of a git hash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did this for security purposes but I'm happy to change it if you're not concerned about that.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't a v1.x be stable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was worried more about security than stability. Tags can be force pushed with malicious code.

Copy link
Member

Choose a reason for hiding this comment

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

Should we not trust this particular action author?

I understand the security concern, but since we also reference tags in other actions, to potentially pick up bug fixes/performance improvements), IMO that's a worthy tradeoff. Open to other core team opinion

@jbrockmendel
Copy link
Member

@jonashaag can you merge main

@jonashaag jonashaag force-pushed the ccache branch 3 times, most recently from 4c17529 to 5952ecf Compare February 20, 2022 13:28
@jonashaag
Copy link
Contributor Author

ping @mroeschke

@jonashaag
Copy link
Contributor Author

Trying Sccache right now because it has support for Windows/Visual Studio compiler

@jonashaag jonashaag marked this pull request as draft February 22, 2022 14:39
@jonashaag
Copy link
Contributor Author

I can't figure out how to tell build_ext to use a different compiler for Windows. It always seems to use cl.exe. @mroeschke @jbrockmendel any ideas?

@jonashaag
Copy link
Contributor Author

Awesome, this seems to work great.

@jonashaag jonashaag force-pushed the ccache branch 2 times, most recently from 72621b4 to 92840f8 Compare February 24, 2022 13:49
@jonashaag jonashaag marked this pull request as ready for review February 24, 2022 15:21
@jonashaag
Copy link
Contributor Author

Awesome, this also works for Windows now. Not yet implemented for Azure pipelines though, so this only applies to the python-dev Windows builds. Would like to test this for a few days and then port it to Azure pipelines as well.

@jonashaag jonashaag requested a review from mroeschke February 24, 2022 15:22
@mroeschke
Copy link
Member

Looks pretty good! Have you observed pushing a commit a day later and seeing the ccache being rebuilt?

@jonashaag
Copy link
Contributor Author

I have observed that numerous times while developing this but we can also retrigger this specific version of the PR tomorrow and have a look.

@jonashaag
Copy link
Contributor Author

There is one important caveat with this PR that I forgot to mention, which is that it currently uses an unmerged version of ccache-action (hendrikmuhs/ccache-action#47)

@jreback
Copy link
Contributor

jreback commented Feb 26, 2022

can you rebase again (as merged the mamba change).

@jonashaag
Add ccache to GHA builds
43e3af0 
- Refactor duplicated code from GHA workflows to the 'build-pandas' and
  'setup' actions.
- Use Mamba over Conda everywhere.
- Add sccache using 'hendrikmuhs/ccache-action'.
- General cleanup of GHA workflows.
@jonashaag
Switch order of name: to be less confusing
4488135
This was referenced Mar 10, 2022
Closed
Closed
@jonashaag
Copy link
Contributor Author

I'm working on an extension of this that also uses Micromamba for env setup (where I recently implemented caching of entire Conda envs). It removes another 5 minutes from the builds. #46346

@jonashaag
Copy link
Contributor Author

jonashaag commented Mar 12, 2022 •
edited
Loading

Example before, almost 10 minutes for Conda setup + Pandas build
Screenshot 2022-03-12 at 22 46 43

Example after, less than 2.5 minutes for Conda setup and Pandas build (both w/ hot cache)
Screenshot 2022-03-12 at 22 46 50

With hot cache most of the time of the Pandas build is spent during cythonization, maybe we can cache that too.

@jonashaag jonashaag closed this Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke left review comments

Assignees
No one assigned
Labels
CI Continuous Integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jonashaag @pep8speaks @jbrockmendel @mroeschke @jreback