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

[REVIEW] Remove deprecated APIs #396

Merged
merged 12 commits into from
Jun 11, 2020

Conversation

jrhemstad
Copy link
Contributor

@jrhemstad jrhemstad commented Jun 10, 2020

Depends on #401

Closes: #302 #301 #305

Furthermore, removes the rmmGetAllocationOffset function. This was simply a wrapper around cuMemGetAddressRange to get the head address of a pool for an IPC handle. However, Numba already provides a wrapper around this API. So there's no longer a need for RMM to provide it (which has the added benefit of RMM no longer needs to link against the driver API).

  • Move default resource functionality from a .cpp file to a header.

After this PR, the only thing preventing RMM from being header only is CNMeM.

@jrhemstad jrhemstad requested a review from a team as a code owner June 10, 2020 02:21
@jrhemstad jrhemstad added the 2 - In Progress Currently a work in progress label Jun 10, 2020
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

I guess this must feel pretty good. :)

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Keith Kraus <kkraus@nvidia.com>
@jrhemstad
Copy link
Contributor Author

I guess this must feel pretty good. :)

Deleting code is my favorite!

@jrhemstad
Copy link
Contributor Author

This shouldn't be merged until @shwina's PR updates the IPC machinery to use Numba instead of rmmGetAllocationOffset.

@shwina
Copy link
Contributor

shwina commented Jun 10, 2020

This shouldn't be merged until @shwina's PR updates the IPC machinery to use Numba instead of rmmGetAllocationOffset.

That PR is #401

@jrhemstad jrhemstad changed the title [WIP] Remove deprecated APIs [REVIEW] Remove deprecated APIs Jun 10, 2020
@jrhemstad jrhemstad added 3 - Ready for review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Jun 10, 2020
@jrhemstad jrhemstad requested a review from a team as a code owner June 11, 2020 19:24
@jrhemstad jrhemstad force-pushed the remove-deprecated-apis branch from b278418 to a4579d7 Compare June 11, 2020 19:26
@jrhemstad jrhemstad requested a review from a team as a code owner June 11, 2020 19:26
@jrhemstad
Copy link
Contributor Author

@shwina can you look at the Cython changes I made in 13df503?

@shwina
Copy link
Contributor

shwina commented Jun 11, 2020

@jrhemstad - yup, LGTM

@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for review Ready for review by team labels Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Deprecate and remove rmm::alloc/free APIs
5 participants