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

Remove build isolation in wheel builds #108

Open
4 of 19 tasks
jameslamb opened this issue Oct 14, 2024 · 2 comments
Open
4 of 19 tasks

Remove build isolation in wheel builds #108

jameslamb opened this issue Oct 14, 2024 · 2 comments
Assignees

Comments

@jameslamb
Copy link
Member

jameslamb commented Oct 14, 2024

Description

Currently, RAPIDS projects build wheels with build isolation, like pip wheel ..

With build isolation, the build tool (in our case, pip wheel), creates a virtual environment and installs of the package's build-time dependencies into it. That virtual environment is created at a random location on each build, which means source files included from other wheels (like the rmm headers from the librmm wheel) are found at a different path on each build in CI.

Path changes lead to cache misses with sccache, and therefore longer-running and more memory-intensive builds.

As more of RAPIDS build-time dependencies are provided wheels (e.g. #33), this problem is going to get worse (see #33 (comment)).

This issue tracks the work of removing build isolation in RAPIDS wheel-building CI jobs.

Benefits of this work

Better sccache hit rate = faster, cheaper CI (contributes to #95).

Stronger guarantees that packages built in CI are actually installed at test time (i.e. would close #79).

Acceptance Criteria

  • all RAPIDS C/C++ wheel builds in CI build with --no-build-isolation

Approach

Follow the approach from rapidsai/cuspatial#1473, and documented in #112.

  • pure Python? build isolation
  • Python + Cython? build isolation
  • mostly C/C++? no build isolation

Notes

These can be done in any order.

Updates

Do the GNN repos last, as some of the cugraph repos are currently being shuffled around. See e.g. rapidsai/cugraph-gnn#58 (comment).

GNN repos

@jameslamb jameslamb self-assigned this Oct 14, 2024
rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue Oct 22, 2024
Contributes to rapidsai/build-planning#108

Contributes to rapidsai/build-planning#111

Proposes building `libcuspatial` wheels with `--no-build-isolation`, to improve the rate of `sccache` cache hits and therefore reduce CI times.

Also proposes printing `sccache` stats to CI logs after wheel and conda builds.

## Notes for Reviewers

#

Authors:
  - James Lamb (https://github.com/jameslamb)
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Mike Sarahan (https://github.com/msarahan)
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

URL: #1473
@jameslamb
Copy link
Member Author

I've updated the description to reflect our decision from an offline conversation:

  • pure Python? build isolation
  • just Python + Cython? build isolation
  • mostly C/C++? no build isolation

And proposed documenting that in the build-planning docs: #112

rapids-bot bot pushed a commit to rapidsai/dask-cuda that referenced this issue Oct 23, 2024
Contributes to rapidsai/build-planning#108

This is a pure Python project, so it doesn't need configuration about CMake or `sccache`.

This proposes removing them to simplify build scripts a bit.

It also proposes updating the `rapids-dependency-file-generator` pre-commit hook to it's latest version, something I'm trying to roll out across RAPIDS as part of rapidsai/build-planning#108.

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Jake Awe (https://github.com/AyodeAwe)

URL: #1400
@vyasr
Copy link
Contributor

vyasr commented Oct 25, 2024

We shouldn't need to do this for any of the packages that haven't switched to using C++ wheels from other libraries yet, i.e. anything that is still statically linking everything. cugraph, cuml, cuvs, and raft (that may be exhaustive, or I may have missed some) don't have relevant changes yet.

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

No branches or pull requests

2 participants