Skip to content

Base device_memory_resource on cuda::stream_ordered_memory_resource#883

Closed
harrism wants to merge 33 commits intorapidsai:branch-21.12from
harrism:fea-libcudacxx-mr-stage-1
Closed

Base device_memory_resource on cuda::stream_ordered_memory_resource#883
harrism wants to merge 33 commits intorapidsai:branch-21.12from
harrism:fea-libcudacxx-mr-stage-1

Conversation

@harrism
Copy link
Member

@harrism harrism commented Oct 7, 2021

This PR is a stepping stone toward basing RMM memory resources on libcu++ cuda::memory_resource. This PR is intended to make it possible to transition dependent libraries (e.g. libcudf) to using cuda::resource_view instead of device_memory_resource pointers. Doing that will make it easier to change RMM over with less disruption.

  • Makes libcudacxx available for cuda::memory_resource
  • Makes device_memory_resource inherit cuda::stream_ordered_memory_resource<cuda::memory_kind::device>

Note that cuda::memory_resource currently only exists in a branch of libcu++ that has not been merged/released yet, so this should not be merged yet.

@harrism harrism added 3 - Ready for review Ready for review by team DO NOT MERGE Hold off on merging; see PR for details labels Oct 7, 2021
@harrism harrism requested a review from jrhemstad October 7, 2021 01:44
@harrism harrism self-assigned this Oct 7, 2021
@harrism harrism requested review from a team as code owners October 7, 2021 01:44
@harrism harrism requested a review from rongou October 7, 2021 01:44
@github-actions github-actions bot added CMake cpp Pertains to C++ code labels Oct 7, 2021
@harrism harrism added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 7, 2021
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

libcudacxx headers will need to be marked as non-system for users of rmm.
This is necessary so that the version that rmm installs is used instead of the older version providied by the CUDA Toolkit.

We can re-use cudf's logic for this: https://github.com/rapidsai/cudf/blob/branch-21.12/cpp/

Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

The big thing I am noticing as far as build-system impacts it the usage of 1.5 instead of 1.4.

This means projects that use cuDF or cuCollections ( or any other consumer of libcudacxx ) and RMM will have two versions of libcudacxx on the include path. This occurs because instead of having a find_package(libcudacxx) each project side-steps and injects the internally packaged cudacxx headers on the include line.

Long term we need libcudacxx to provide proper CMake module support so projects don't install a copy in a custom 'internal' location.

harrism and others added 2 commits October 8, 2021 06:49
Co-authored-by: Robert Maynard <robertjmaynard@gmail.com>
Co-authored-by: Robert Maynard <robertjmaynard@gmail.com>
@harrism
Copy link
Member Author

harrism commented Oct 12, 2021

Long term we need libcudacxx to provide proper CMake module support so projects don't install a copy in a custom 'internal' location.

@robertmaynard is that something you would be able / willing to contribute to libcudacxx?

@harrism
Copy link
Member Author

harrism commented Oct 13, 2021

rerun tests

@harrism harrism requested a review from a team as a code owner October 20, 2021 02:09
@github-actions github-actions bot added the Python Related to RMM Python API label Oct 20, 2021
GPUtester and others added 6 commits November 18, 2021 12:47
[gpuCI] Forward-merge branch-21.12 to branch-22.02 [skip gpuci]
This PR adds a script to find the cmake-format-rapids-cmake.json file in a standard location and run the cmake-format or cmake-lint programs with that config file. The script fails gracefully when the file cannot be found and is therefore suitable for use as a pre-commit hook in scenarios where no build directory (containing the config file) exists yet. A corresponding pre-commit configuration is added here as well, replacing the old cmake-format hook which did not use the rapids-cmake config file.

Resolves rapidsai#903.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Robert Maynard (https://github.com/robertmaynard)
  - Rong Ou (https://github.com/rongou)

URL: rapidsai#913
[gpuCI] Forward-merge branch-21.12 to branch-22.02 [skip gpuci]
@github-actions
Copy link

This PR has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates. This PR will be labeled inactive-90d if there is no activity in the next 60 days.

@github-actions
Copy link

This PR has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this PR if it is no longer required. Otherwise, please respond with a comment indicating any updates.

@harrism
Copy link
Member Author

harrism commented Sep 27, 2022

Superseded by #1095

@harrism harrism closed this Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Ready for review Ready for review by team CMake cpp Pertains to C++ code DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function inactive-30d inactive-90d non-breaking Non-breaking change Python Related to RMM Python API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants