Fix loading of C++ libraries in wheels #118

vyasr · 2024-11-08T17:40:36Z

Description

Currently the wheels in which we are shipping native libraries have a conditional load logic that first searches for a library on the system and loads that before loading one in the wheel. As I've discussed in other channels, there is no way to support this kind of switching safely in general. If two different copies of a library exist and different packages make different assumptions when loading, the first one loaded wins and will be used in all cases. I'm working on more general solutions to this problem, but in the meantime we at minimum need to reconsider our default behavior, which is causing users confusion now. If we assume that for rapids native libraries the only consumers of those wheels are other rapids wheels that we control and are therefore consistently behaved, we can make some more sane default choices for the user. In general, we should assume that our packages are being installed by end users unfamiliar with packaging. The default behavior should support this, meaning that by default we probably want the cudf pip wheel to use libcudf from a wheel even if a system libcudf library exists by default. Conversely, more advanced users may install cudf into an environment (like a container) where the library already exists and should be reused. We should enable that use case using an environment variable. A global configuration is another option, but I'd like to reserve that for when I have a more general solution available.

We should also switch all our library loads to use RTLD_LOCAL instead of RTLD_GLOBAL to be safe. Global loads can easily lead to symbol clashes.

Benefits of this work

adds flexibility for different preferred methods of library-loading
makes library-loading safer (i.e. less likely to cause conflicts)

Acceptance Criteria

For all relevant RAPIDS libraries:

preference for system vs. wheel shared library is configurable via an environment
shared library loading defaults to preferring the library from the wheel itself
all library loads are done with RTLD_LOCAL

Approach

N/A

Notes

N/A

Task Lists

Updates

Give feedback

cudf (prefer wheel-provided libcudf.so in load_library(), use RTLD_LOCAL cudf#17316, fix library-loading issues in editable installs cudf#17338)
cuspatial (prefer wheel-provided libcuspatial.so in load_library(), use RTLD_LOCAL cuspatial#1483, fix library-loading issues in editable installs cuspatial#1484)
kvikio (prefer wheel-provided libkvikio.so, use RTLD_LOCAL kvikio#551, fix library-loading issues in editable installs kvikio#553)
libucx (prefer wheel-provided libraries, use RTLD_LOCAL ucx-wheels#13, and all previously-published versions)
ucx-py (document changes in library-loading, update build-UCX-from-source docs ucx-py#1099)
ucxx (prefer wheel-provided libucxx.so in load_library, use RTLD_LOCAL ucxx#322)
update descriptions on still-open "Dynamic RAPIDS Deps" issues on wheel-planning board (https://github.com/orgs/rapidsai/projects/136)
update https://github.com/rapidsai/build-planning/tree/main/docs with note about this (let's skip this: Fix loading of C++ libraries in wheels #118 (comment))
Options

devcontainers changes

Give feedback

update devcontainers (prefer system installs of UCX libraries when using RAPIDS wheels devcontainers#421)
cugraph (pip devcontainers: use UCX 1.17, prefer system installation cugraph#4792)
cugraph-gnn (add devcontainers cugraph-gnn#79)
cuml (prefer system install of UCX in pip devcontainers, update outdated RAPIDS references cuml#6149)
cuvs (prefer system install of UCX in devcontainers cuvs#501)
nx-cugraph (prefer system install of UCX in devcontainers, update outdated RAPIDS references nx-cugraph#53)
raft (prefer system install of UCX in devcontainers, update outdated RAPIDS references raft#2514)
Options

NOTE: individual-repo devcontainers changes are only needed for the ucx wheels, see rapidsai/devcontainers#421 (comment).

The text was updated successfully, but these errors were encountered:

jameslamb · 2024-11-08T19:43:02Z

Linking some related conversations:

…AL (#1483) Contributes to rapidsai/build-planning#118 Modifies `libcuspatial.load_library()` in the following ways: * prefer wheel-provided `libcuspatial.so` to system installation * expose environment variable `RAPIDS_LIBCUSPATIAL_PREFER_SYSTEM_LIBRARY` for switching that preference * load `libcuspatial.so` with `RTLD_LOCAL`, to prevent adding symbols to the global namespace ([dlopen docs](https://linux.die.net/man/3/dlopen)) ## Notes for Reviewers ### How I tested this See "How I tested this" in rapidsai/cudf#17316 Also opened this PR originally pulling in built packages from rapidsai/cudf#17316 # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Bradley Dice (https://github.com/bdice) URL: #1483

…17316) Contributes to rapidsai/build-planning#118 Modifies `libcudf.load_library()` in the following ways: * prefer wheel-provided `libcudf.so` to system installation * expose environment variable `RAPIDS_LIBCUDF_PREFER_SYSTEM_LIBRARY` for switching that preference * load `libcudf.so` with `RTLD_LOCAL`, to prevent adding symbols to the global namespace ([dlopen docs](https://linux.die.net/man/3/dlopen)) ## Notes for Reviewers ### How I tested this Locally (x86_64, CUDA 12, Python 3.12), built `libcudf`, `pylibcudf`, and `cudf` wheels from this branch, then `libcuspatial` and `cuspatial` from the corresponding cuspatial branch. Ran `cuspatial`'s unit tests, and tried setting the environment variable and inspecting `ld`'s logs to confirm that the environment variable changed the loading and search behavior. e.g. ```shell # clear ld cache to avoid cheating rm -f /etc/ld.so.cache ldconfig # try using an env variable to say "prefer the system-installed version" LD_DEBUG=libs \ LD_DEBUG_OUTPUT=/tmp/out.txt \ RAPIDS_LIBCUDF_PREFER_SYSTEM_LIBRARY=true \ python -c "import cuspatial; print(cuspatial.__version__)" cat /tmp/out.txt.* > prefer-system.txt # (then manually looked through those logs to confirm it searched places like /usr/lib64 and /lib64) ``` # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Bradley Dice (https://github.com/bdice) URL: #17316

Contributes to rapidsai/build-planning#118 Modifies `libkvikio.load_library()` in the following ways: * prefer wheel-provided `libkvikio.so` to system installation * expose environment variable `RAPIDS_LIBKVIKIO_PREFER_SYSTEM_LIBRARY` for switching that preference * load `libkvikio.so` with `RTLD_LOCAL`, to prevent adding symbols to the global namespace ([dlopen docs](https://linux.die.net/man/3/dlopen)) ## Notes for Reviewers ### How I tested this Tested the general approach on rapidsai/cudf#17316 and rapidsai/cuspatial#1483. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #551

jameslamb · 2024-11-15T00:22:13Z

update descriptions on still-open "Dynamic RAPIDS Deps" issues on wheel-planning board (https://github.com/orgs/rapidsai/projects/136)

done (see the links to the private issues in the event feed above)

Contributes to rapidsai/build-planning#118 The pattern introduced in #551 breaks editable installs in devcontainers. In that type of build, `libkvikio.so` is built outside of the wheel but **not installed**, so it can't be found by `ld`. Extension modules in `kvikio` are able to find it via RPATHs instead. This proposes: * try-catching the entire library-loading attempt, to silently do nothing in cases like that * adding an import of the `kvikio` Python library in the `devcontainers` CI job, as a smoke test to catch issues like this in the future ## Notes for Reviewers ### How I tested this Reproduced that with the CUDA 12.5 pip devcontainers today: ```shell build-all python -c "import kvikio" # OSError: libkvikio.so: cannot open shared object file: No such file or directory ``` Confirmed that the changes in this PR fix that. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Lawrence Mitchell (https://github.com/wence-) URL: #553

jameslamb · 2024-11-15T18:09:39Z

update https://github.com/rapidsai/build-planning/tree/main/docs with note about this

I'd added this to the task list for this issue but... let's skip it. The loading logic for C++ wheels is already not described in those docs, and it's still very much actively changing and being redesigned.

This issue and others linked to/from it are enough to document the current state, think it's ok to wait until that stabilizes a bit before updating those docs.

Contributes to rapidsai/build-planning#118 The pattern introduced in #1483 breaks editable installs in devcontainers. In that type of build, `libcuspatial.so` is built outside of the wheel but **not installed**, so it can't be found by `ld`. Extension modules in `cuspatial` are able to find it via RPATHs instead. This proposes: * try-catching the entire library-loading attempt, to silently do nothing in cases like that * ~adding an import of the `cuspatial` Python library in the `devcontainers` CI job, as a smoke test to catch issues like this in the future~ *(edit: removed those, [`devcontainer` builds run on CPU nodes](https://github.com/rapidsai/shared-workflows/blob/4e84062f333ce5649bc65029d3979569e2d0a045/.github/workflows/build-in-devcontainer.yaml#L19))* ## Notes for Reviewers ### How I tested this Tested this approach on rapidsai/kvikio#553 # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1484

Contributes to rapidsai/build-planning#118 Modifies `libucxx.load_library()` in the following ways: * prefer wheel-provided `libucxx.so` to system installation * expose environment variable `RAPIDS_LIBUCXX_PREFER_SYSTEM_LIBRARY` for switching that preference * load `libucxx.so` with `RTLD_LOCAL`, to prevent adding symbols to the global namespace ([dlopen docs](https://linux.die.net/man/3/dlopen)) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Peter Andreas Entschev (https://github.com/pentschev) URL: #322

Contributes to rapidsai/build-planning#118 The pattern introduced in #17316 breaks editable installs in devcontainers. In that type of build, `libcudf.so` is built outside of the wheel but **not installed**, so it can't be found by `ld`. Extension modules in `cudf` and `pylibcudf` are able to find it via RPATHs instead. This proposes: * try-catching the entire library-loading attempt, to silently do nothing in cases like that * ~adding imports of the `cudf` and `pylibcudf` libraries in the `devcontainers` CI job, as a smoke test to catch issues like this in the future~ *(edit: removed those, [`devcontainer` builds run on CPU nodes](https://github.com/rapidsai/shared-workflows/blob/4e84062f333ce5649bc65029d3979569e2d0a045/.github/workflows/build-in-devcontainer.yaml#L19))* ## Notes for Reviewers ### How I tested this Tested this approach on rapidsai/kvikio#553 # Authors: - James Lamb (https://github.com/jameslamb) - Matthew Murray (https://github.com/Matt711) Approvers: - Bradley Dice (https://github.com/bdice) - Matthew Murray (https://github.com/Matt711) URL: #17338

## Description Contributes to rapidsai/build-planning#118 Modifies `libucx.load_library()` in the following ways: * prefer wheel-provided shared libraries to system installation * expose environment variable `RAPIDS_LIBUCX_PREFER_SYSTEM_LIBRARY` for switching that preference * load libraries with `RTLD_LOCAL`, to prevent adding symbols to the global namespace ([dlopen docs](https://linux.die.net/man/3/dlopen)) Also updates all the `pre-commit` hook versions while I'm touching this repo. That was harmless, and I especially wanted to get up to the latest version of the RAPIDS copyright hook. ## Notes for Reviewers ### Version changes? Proposing starting with `1.15.0.post2` because it's the oldest version supported at build and runtime by `ucxx` ([code link](https://github.com/rapidsai/ucxx/blob/73e2102406a78527b1f4c1ca4bde29158bee06a1/dependencies.yaml#L482)). And doing the following, in this order: 1. merge this PR 2. in a `ucxx` PR, test with these packages 3. publish 1.15.0.post2 packages (via manually triggering CI run here) 4. put up PRs to publish each of the other versions (https://pypi.org/project/libucx-cu12/#history) * 1.14.0.post2 * 1.16.0.post2 * 1.17.0.post1 (there was never a 1.17.0.post1) --------- Co-authored-by: Vyas Ramasubramani <[email protected]>

Contributes to rapidsai/build-planning#118 Should only be merged if / after we merge rapidsai/ucx-wheels#13. That PR switches `libucx` wheels' loading behavior, such that `libucx.load_library()` (which `ucx-py` and `libucxx` call at import time) prefers the `libuc{m,p,s}.so` provided by the `libucx` wheels. rapidsai/ucx-wheels#13 introduces an environment variable to control that... this proposes setting that, to continue preferring system-installed UCX libraries in devcontainers. --------- Co-authored-by: Vyas Ramasubramani <[email protected]>

… references (#53) Contributes to rapidsai/build-planning#118 Proposes the following changes for pip devcontainers: * use UCX 1.17 (ref: rapidsai/cugraph-gnn#79 (comment)) * prefer system installation of ucx to the one provided by the `libucx-cu{11,12}` wheels (ref: rapidsai/devcontainers#421 (comment)) And some other related changes noticed while doing that: * update lingering `24.*` references to `25.02` * fix `update-version.sh` so those will be correctly updated in future releases Similar to rapidsai/cugraph#4792 ## Notes for Reviewers ### How I tested this Relying on CI for most things. But for `update-version.sh`, tested like this: ```shell ./ci/release/update-version.sh '25.02.00' git grep -E '24\.' ``` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #53

Contributes to rapidsai/build-planning#118 Proposes the following changes for pip devcontainers: * prefer system installation of ucx to the one provided by the `libucx-cu{11,12}` wheels (ref: rapidsai/devcontainers#421 (comment)) And some other related changes noticed while doing that: * update lingering `24.*` references to `25.02` ## Notes for Reviewers ### How I tested this Relying on CI for most things. Double-checked that `update-version.sh` would have caught the one lingering `24.12` reference like this: ```shell ./ci/release/update-version.sh '25.02.00' git grep -E '24\.' ``` Similar to rapidsai/cuml#6149 Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #501

Contributes to rapidsai/build-planning#118 Proposes the following changes for pip devcontainers: * use UCX 1.17 (ref: rapidsai/cugraph-gnn#79 (comment)) * prefer system installation of ucx to the one provided by the `libucx-cu{11,12}` wheels (ref: rapidsai/devcontainers#421 (comment)) And one other related change: * skip most CI when a PR only changes files in the `.devcontainer/` directory (this was incorrectly spelled `.devcontainers/` before) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Jake Awe (https://github.com/AyodeAwe) URL: #4792

… references (#2514) Contributes to rapidsai/build-planning#118 Proposes the following changes for pip devcontainers: * prefer system installation of ucx to the one provided by the `libucx-cu{11,12}` wheels (ref: rapidsai/devcontainers#421 (comment)) And some other related changes noticed while doing that: * update lingering `24.*` references to `25.02` ## Notes for Reviewers ### How I tested this Relying on CI for most things. Double-checked that `update-version.sh` would have caught the one lingering `24.12` reference like this: ```shell ./ci/release/update-version.sh '25.02.00' git grep -E '24\.' ``` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #2514

…PIDS references (#6149) Contributes to rapidsai/build-planning#118 Proposes the following changes for pip devcontainers: * prefer system installation of ucx to the one provided by the `libucx-cu{11,12}` wheels (ref: rapidsai/devcontainers#421 (comment)) And some other related changes noticed while doing that: * update lingering `24.*` references to `25.02` ## Notes for Reviewers ### How I tested this Relying on CI for most things. Double-checked that `update-version.sh` would have caught the one lingering `24.12` reference like this: ```shell ./ci/release/update-version.sh '25.02.00' git grep -E '24\.' ``` Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) Approvers: - Tim Head (https://github.com/betatim) - Bradley Dice (https://github.com/bdice) URL: #6149

#1099) Contributes to rapidsai/build-planning#118 Caused by rapidsai/ucx-wheels#13 I originally came here to document the implications of rapidsai/ucx-wheels#13 in the docs, namely: * if you have a `libucx-cu{11,12}` wheel installed, then by default `ucx-py` will use UCX libraries from that wheel * environment variable `RAPIDS_LIBUCX_PREFER_SYTEM_LIBRARY=true` can be set to opt out of this and use a system installation instead While doing that, I noticed some other opportunities for improvement in the installation docs: * updating build-UCX-from-source instructions to UCX 1.15 ([the oldest version this project now supports](https://github.com/rapidsai/ucx-py/blob/9efacc6069226de8e207177a359189f8880203a8/dependencies.yaml#L159)) * clarifying and simplifying some language ## Notes for Reviewers ### How I tested this Followed these instructions in a Docker container running on an x86_64 machine with 8 V100s. ```shell docker run \ --rm \ --gpus 0 \ -v $(pwd):/opt/work \ -w /opt/work \ -it rapidsai/ci-conda:latest \ bash ``` Used `conda` to set up the build environment: ```shell conda create -n ucx -c conda-forge \ automake make libtool pkg-config \ "python=3.12" "setuptools>=64.0" "cython>=3.0.0" \ cuda-nvcc \ cuda-cudart-dev \ cuda-nvml-dev \ cuda-nvtx-dev \ cuda-version=12.5 ``` Ran variations of this code snippet to test my install: ```shell python -c "import ucp; print(ucp.get_ucx_version())" ``` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Peter Andreas Entschev (https://github.com/pentschev) URL: #1099

jameslamb · 2024-12-16T17:52:43Z

This is complete, thanks yall.

jameslamb assigned jameslamb and vyasr Nov 13, 2024

jameslamb mentioned this issue Nov 15, 2024

fix library-loading issues in editable installs rapidsai/kvikio#553

Merged

This was referenced Nov 15, 2024

fix library-loading issues in editable installs rapidsai/cudf#17338

Merged

fix library-loading issues in editable installs rapidsai/cuspatial#1484

Merged

jameslamb mentioned this issue Nov 15, 2024

prefer system installs of UCX libraries when using RAPIDS wheels rapidsai/devcontainers#421

Merged

jameslamb mentioned this issue Dec 13, 2024

document changes in library-loading, update build-UCX-from-source docs rapidsai/ucx-py#1099

Merged

jameslamb closed this as completed Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix loading of C++ libraries in wheels #118

Fix loading of C++ libraries in wheels #118

vyasr commented Nov 8, 2024 •

edited by jameslamb

Loading

Updates

devcontainers changes

jameslamb commented Nov 8, 2024

jameslamb commented Nov 15, 2024

jameslamb commented Nov 15, 2024

jameslamb commented Dec 16, 2024

Fix loading of C++ libraries in wheels #118

Fix loading of C++ libraries in wheels #118

Comments

vyasr commented Nov 8, 2024 • edited by jameslamb Loading

Description

Benefits of this work

Acceptance Criteria

Approach

Notes

Task Lists

Updates

devcontainers changes

jameslamb commented Nov 8, 2024

jameslamb commented Nov 15, 2024

jameslamb commented Nov 15, 2024

jameslamb commented Dec 16, 2024

vyasr commented Nov 8, 2024 •

edited by jameslamb

Loading