Skip to content

[WIP] Remove RAFT memory management#400

Merged
rapids-bot[bot] merged 12 commits into
NVIDIA:branch-22.04from
viclafargue:remove-raft-memory-management
Mar 18, 2022
Merged

[WIP] Remove RAFT memory management#400
rapids-bot[bot] merged 12 commits into
NVIDIA:branch-22.04from
viclafargue:remove-raft-memory-management

Conversation

@viclafargue

Copy link
Copy Markdown
Contributor

Answers #308.
Requires the appropriate changes in cuML and cuGraph before merging.

@viclafargue viclafargue requested review from a team as code owners November 24, 2021 10:12
@cjnolet cjnolet added the 2 - In Progress Currenty a work in progress label Dec 6, 2021
rapids-bot Bot pushed a commit to rapidsai/cuml that referenced this pull request Jan 18, 2022
Completes NVIDIA/raft#400

Authors:
  - Victor Lafargue (https://github.com/viclafargue)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #4468
@cjnolet

cjnolet commented Jan 25, 2022

Copy link
Copy Markdown
Contributor

@viclafargue, it looks like there's just a couple includes for allocator remaining in the code and this is ready to go. Do we want to get this into 22.02?

@cjnolet cjnolet changed the base branch from branch-22.02 to branch-22.04 January 25, 2022 16:24
@viclafargue

Copy link
Copy Markdown
Contributor Author

Postponing to 22.04, as discussed offline.

@github-actions github-actions Bot added the CMake label Jan 25, 2022
@viclafargue viclafargue requested review from a team as code owners February 14, 2022 13:35
#include <raft/mr/buffer_base.hpp>
#include <raft/mr/device/buffer.hpp>

namespace raft {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this is going to require some updates to cugraph as well.

Comment thread cpp/test/spatial/fused_l2_knn.cu Outdated
rapids-bot Bot pushed a commit to rapidsai/cugraph that referenced this pull request Feb 15, 2022
@viclafargue viclafargue added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change breaking Breaking change and removed non-breaking Non-breaking change labels Feb 24, 2022
@cjnolet

cjnolet commented Feb 25, 2022

Copy link
Copy Markdown
Contributor

rerun tests

@viclafargue viclafargue force-pushed the remove-raft-memory-management branch from cf19910 to 0243630 Compare March 15, 2022 18:36
/** Default cudaMallocHost/cudaFreeHost based host allocator */
class default_allocator : public allocator {
public:
void* allocate(std::size_t n, cudaStream_t stream) override

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had tried to remove this at one point and realized cugraph is still using it. We should also open a PR in cugraph to move the use of host allocator/buffer to something equivalent.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just searched cugraph/cpp and could not find code calling this. We need to double check but we may have removed the code using this function.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@seunghwak I think you are right, I'm no longer seeing it in cugraph either.

@cjnolet

cjnolet commented Mar 18, 2022

Copy link
Copy Markdown
Contributor

@gpucibot merge

@rapids-bot rapids-bot Bot merged commit 613c722 into NVIDIA:branch-22.04 Mar 18, 2022
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
loulankxh pushed a commit to loulankxh/raft that referenced this pull request Oct 14, 2025
Answers NVIDIA#308.
Requires the appropriate changes in `cuML` and `cuGraph` before merging.

Authors:
  - Victor Lafargue (https://github.com/viclafargue)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: NVIDIA#400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 - In Progress Currenty a work in progress breaking Breaking change CMake cpp improvement Improvement / enhancement to an existing function python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants